SpaceApi / schema

SpaceAPI JSON schema files.
24 stars 14 forks source link

Added lastchange property to all sensors. Fixes #95 #97

Closed the-metalgamer closed 1 year ago

the-metalgamer commented 1 year ago

The lastchange property represents a unix timestamp indicating the time the sensor value was last updated.

For the wind sensor, I added it at the top object, but it could also be added to the nested object properties.

s3lph commented 1 year ago

Thanks for the pull request, this is something I've had had on my personal roadmap for a longer time now, especially since I myself have been using ext_lastchange for all of the sensor values in my space for ages now.

Also did a short survey checking for the most used ext_... keys, and after ext_ccc, ext_lastchange seems to be the most used among all endpoints in the directory (only listing those with more than 1 occurence):

     25 ext_ccc
     21 ext_lastchange
      5 ext_matrix
      3 ext_show_on_page
      2 ext_mastodon

I think it makes sense to have the lastchange at the top level like you're suggesting. As I understand it, the single properties of multivalued sensors such as wind or network traffic generally belong together, so it makes sense to have the lastchange value for the whole sensor and not just individual properties.

I'm in favor of merging this as-is, unless @gidsi @dbrgn @rnestler disagree?

gidsi commented 1 year ago

Sounds good to me, i would like to change the description and also maybe define the field once and reference it instead of copy & paste (but since this would be something we haven't done before in the schema i would create a PR afterwards).

the-metalgamer commented 1 year ago

I can change the text. At the moment it is adapted from the other lastchange fields in the schema.

Any suggestions?

dbrgn commented 1 year ago

Yeah, makes sense, as long as it's an optional field.

Nitpick from my side: Descriptions should end with a period (.).

The text is fine for me. @gidsi do you have a concrete proposal for an improved version?

maybe define the field once and reference it instead of copy & paste

Does JSON Schema support that? In any case, I think copy&paste is fine, since it's easier to read and understand, even if it's not totally DRY.

dbrgn commented 1 year ago

Ah, another suggestion: Please indicate the unit of the timestamp: "Unix timestamp (in seconds)". This should usually be clear, but a lot of people also use milliseconds for timestamps. (There are two other occasions of timestamps in the schema, feel free to update those as well.)

the-metalgamer commented 1 year ago

Ah, another suggestion: Please indicate the unit of the timestamp: "Unix timestamp (in seconds)". This should usually be clear, but a lot of people also use milliseconds for timestamps. (There are two other occasions of timestamps in the schema, feel free to update those as well.)

I will do a separate pull requests for the two other occasions.

rnestler commented 1 year ago

Ah, another suggestion: Please indicate the unit of the timestamp: "Unix timestamp (in seconds)". This should usually be clear, but a lot of people also use milliseconds for timestamps. (There are two other occasions of timestamps in the schema, feel free to update those as well.)

With double precision people can still get millisecond resolution with the timestamp in seconds, but not much more.