ASCOMInitiative / ASCOMRemote

The ASCOM REST based Remote Driver Server and Remote Access Clients
GNU General Public License v3.0
57 stars 15 forks source link

Fix date-time format #60

Closed RReverser closed 1 week ago

RReverser commented 1 week ago

Fixing a regression from d8f241b17872e53976eed9cc9b24e78c715edb5f.

OpenAPI declares this standard format as specifically date-time constant, not date-time-string.

date-time-fits is a custom one, but I'm following the same precedent for consistency.

RReverser commented 1 week ago

@Peter-Simpson Related question: do you think there's any chance of unifying the date formats across those different APIs? For example, by making all of them accept time with or without the trailing Z (like DeviceStateDateTime does)? Or is it too late for such a change?

It just tends to be pretty awkward to have to deal with 2 date formats within the same spec / implementation depending which method they're coming from.

RReverser commented 1 week ago

Alternatively, it could be nice to sidestep the issue with date formats altogether and returns timestamps as plain integers (Unix timestamps). There are APIs in all languages to convert those to dates nowadays, and it would allow clients to avoid parsing dates altogether when they're used merely for comparisons / subtractions, but I guess that would be too much of a breaking change for Alpaca.

I am, however, hopeful about at least converging on a single datetime format in Platform 7 or Alpaca.

Peter-Simpson commented 1 week ago

The string formats have been established for many years so it's not possible to change them now. or to use the UNIX timestamp approach.

RReverser commented 1 week ago

The string formats have been established for many years so it's not possible to change them now. or to use the UNIX timestamp approach.

Do you mean in ASCOM or Alpaca? If only the former, I'd expect that it should be possible to use a different format in Alpaca and let ASCOM Remote convert between the data formats, like it does for e.g. image array representation.

Or, if it's both, could making Z optional in both formats work? It should be backward-compatible, right?

What is the down side of leaving the format names as is? Is this simply a style question?

Well, not only - while you can add custom ones when necessary, there is a set of standard predefined formats in OpenAPI that all tooling knows how to deal with.

That includes formats like int32, float, double, and, among various others, date-time. See this JSON Schema spec that OpenAPI is based on: https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-validation-00#section-7.3

Date and time format names are derived from RFC 3339, section 5.6 [RFC3339]. The duration format is from the ISO 8601 ABNF as given in Appendix A of RFC 3339.

Implementations supporting formats SHOULD implement support for the following attributes:

date-time: A string instance is valid against this attribute if it is a valid representation according to the "date-time" production.

I believe we should stick to standard, universally supported formats, where possible, and only define custom formats when it really is not part of the official spec (like it is for the FITS datetime format).

If you want, I can modify JS to show the string type even when a format is present.

Peter-Simpson commented 1 week ago

I accept these changes but due to further work on the YAML file since this PR was submitted, there is now a merge conflict to resolve. Since the changes are small I will add them manually to the update that I am preparing and so will not merge this PR.

RReverser commented 1 week ago

Sounds good, thanks.