lorenzodonini/ocpp-go

Parsing of local timestamps not supported - spec violation?

andig opened this issue · 9 comments

andig commented

In evcc-io/evcc#10399 we've noticed that timestamps in the form of are rejected:

parsing time "2023-10-19T15:28:07" as "2006-01-02T15:04:05Z07:00": cannot parse "" as "Z07:00"

evcc-io/evcc#10399 (comment) found:

In the specs of ocpp 1.6 I can read
..
3.15. Time notations
This section is normative.
Implementations MUST use ISO 8601 date time notation. Message receivers must be able to handle fractional
seconds and time zone offsets (another implementation might use them). Message senders MAY save data
usage by omitting insignificant fractions of seconds.
..
and as mentioned here
https://en.wikipedia.org/wiki/ISO_8601
..
If the time is in UTC, add a Z directly after the time without a space. Z is the zone designator for the zero UTC offset.
..
and some lines above
..
If no UTC relation information is given with a time representation, the time is assumed to be in local time.
..
Could this not also be understood as that the "Z" or the other time zone strings for example "+02:00" are optional?
Or is this a requirement of evcc to have all chargers and, may be, all other devices at UTC?

This seems to indicate that parsing local time zone should be possible, too?

andig commented

Update: noticed that DateTime supports custom format via DateTimeFormat. Unfortunately that is a global setting and cannot be adjusted per chargepoint.

RFC3339 indeed doesn't support a timestamp without explicit timezone and I oversimplified the ISO8601 support, due to it actually being a huge collection of different formats (see https://ijmacd.github.io/rfc3339-iso8601/).

I'd fix this by supporting all ISO8601 via an external lib. Shouldn't break anything and instead add more flexibility.

andig commented

That would imho be preferable over a fallback global format 👍🏻

Added the improved ISO8601 parsing.

The DateTimeFormat global variable is still used for sending messages: this allows to configure the lib to use a specific (if any) format for marshaling timestamps. This can be set to arbitrary format. If set to empty (""), it will use the internal Go default (RFC3339, which is ISO8601-compatible in that case).

The DateTimeFormat global variable is still used for sending messages: this allows to configure the lib to use a specific (if any) format for marshaling timestamps.

Problem remains that this approach does not allow to mix different charge points. The format should be per CP instead of global in order to be useful beyond the most basic case?

Problem remains that this approach does not allow to mix different charge points. The format should be per CP instead of global in order to be useful beyond the most basic case?

You expect sending timestamps in different formats to different clients? Why? By sending an ISO-formatted timestamp to all of them it wouldn't cause any issue, assuming the charge points implement the spec correctly.

Please don't get me wrong- I haven't seen this particular issue in the wild yet.

By sending an ISO-formatted timestamp to all of them it wouldn't cause any issue, assuming the charge points implement the spec correctly.

If that wasn't the case, the global fallback setting would not be needed at all, right?

If that wasn't the case, the global fallback setting would not be needed at all, right?

Totally true for servers. The fallback was originally meant for charge points, in case a vendor wishes to enforce a specific format of the ISO standard.

I'm really hoping to keep this as simple as possible tbh. If this is ever needed I can think of two ways to make this dynamic though:

  1. include a format override option in the DateTime struct itself. This would need to be passed at runtime for every message. It would offer the best flexibility but would require a bit more effort for the applications
  2. a global mapping that can be looked up during serialization. The type serialization is not linked to any state currently (pure object marhsaling operation), so the map needs to be "static". This one is uglier imo

Totally true for servers. The fallback was originally meant for charge points, in case a vendor wishes to enforce a specific format of the ISO standard.

Oh yeah, I've had our server use case in mind. Let's solve this when it really becomes a problem.

Thank you for all the efforts spend with maintaining this great library!