feat: Move from JodaTime to java.time #16
Conversation
src/main/java/de/rwth/idsg/ocpp/jaxb/JavaDateTimeConverter.java
Outdated
Show resolved
Hide resolved
juherr
force-pushed
the
feature/java-time
branch
2 times, most recently
from
dfd7513 to
5822b29
Compare
|
@goekay Updated with the new test cases. |
juherr
force-pushed
the
feature/java-time
branch
5 times, most recently
from
b18f2bb to
fc91344
Compare
juherr
force-pushed
the
feature/java-time
branch
from
fc91344 to
bf0233d
Compare
|
@goekay According to the specification: 
Wouldn’t you agree that all date-time values should be in UTC? |
|
You can have a look at the potential change: juherr@b0f54a5 |
i would, but this is not the reality. you can find some steve issues with timestamp examples where timezone is missing. moreover, UTC is a strong recommendation but not a must for ocpp. in any case, this UTC enforcement should not come from this library. it is not the job of it. at best, we should remove the timezone parameter in steve and set it internally to UTC which would make this lib also work in UTC. |
|
The purpose of the library is to read timestamps provided by the station and write timestamps that the station can handle. From experience, we know that some stations omit the time zone in the timestamps they send. The library must therefore handle such cases gracefully. On the other hand, since |
agreed. no objections here. semantically, they represent the same point in time. |
| private final boolean marchallToUtc; | ||
|
|
||
| public JavaDateTimeConverter() { | ||
| this(ZoneId.systemDefault(), System.getProperty("steve.ocpp.marshall-to-utc", "true").equals("true")); |
There was a problem hiding this comment.
i dont get why this property is necessary.
steve has a UTC setting, which has this consequence. finally, the library actually should just respect the zone id the java app runs in and that is it.
There was a problem hiding this comment.
The property configures the zone used by the output format: utc as expected by the specification or the default java zone.
It helps when default zone is not UTC but you want to respect the specification.
As Steve configures utc by default both configuration will have the same effect.
UTC could always be used (my 1st implementation) but your test suite checks cases with not utc as default zone.
There was a problem hiding this comment.
i would find it a little bit weird that a subsystem (this lib) behaves differently than the main java system (steve). from my point of view, there should be only one timezone configuration that applies globally to the java app/process.
for example, if someone changes the default UTC timezone to something with UTC+2 hours such that we start adding the offset of 2h, then the date/times should reflect that new reality. we should not translate this back to UTC on OCPP messaging level.
Close #12