Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit for barometer is wrong (hPA instead of hPa) (rev. 13) #2

Closed
simon-budig opened this issue Jan 21, 2017 · 8 comments
Closed

Unit for barometer is wrong (hPA instead of hPa) (rev. 13) #2

simon-budig opened this issue Jan 21, 2017 · 8 comments
Labels
Milestone

Comments

@simon-budig
Copy link

The unit for the "barometer" device is wrongly capitalized, it should be "hPa" instead of "hPA" (see https://en.wikipedia.org/wiki/Pascal_%28unit%29 )

@dbrgn
Copy link
Contributor

dbrgn commented Jan 21, 2017

You're obviously right.

@rnestler @dns2utf8 @rorist would this be a breaking change in an already published spec if we change the capitalization of one of multiple valid values? Does this need to go into the v14 draft?

@dbrgn
Copy link
Contributor

dbrgn commented Jan 21, 2017

I just crawled the entire directory. Right now a single space is providing barometer data:

https://hskrk-spacemon.herokuapp.com/

@rorist
Copy link

rorist commented Jan 22, 2017

Hi, maybe we could simplify the sensors. Because now it's almost always an array of objet with value, unit, location, and desc. We could write the specs so its' backward compatible with the detailed sensors, and remove them all, so people can add their own sensors. And still provide examples of sensors.

@dbrgn
Copy link
Contributor

dbrgn commented Jan 22, 2017

Hm, I think some of the sensors should be standardized, e.g. people now present.

But simplification might be an option.

@simon-budig
Copy link
Author

I have no clue how the specification is handled.

For v13 it might be an option to just add "hPa" as enum value in addition to "hPA", and then drop "hPA" in v14. That way it is possible for hackspaces (our hackspace in Siegen actually provides a barometric value of "null" - grgh) to deliver the sensor value correctly without breaking the other hackspaces.

@dns2utf8
Copy link
Contributor

I think it would be a breaking change.
But since it would only apply to two spaces, we could also change both endpoints.

What do you think @HackerspaceKRK @simon-budig ?

@dbrgn
Copy link
Contributor

dbrgn commented Jan 23, 2017

We don't know whether there are other applications that will consider hPa as invalid. After all, it should be considered invalid according to the published spec. Since it doesn't hurt anyone, I'd actually stick with the wrong writing and fix it in v0.14.

See #3 for v0.14 discussion.

@simon-budig what do you think? :)

@dbrgn dbrgn added the bug label Jan 23, 2017
@dbrgn dbrgn modified the milestone: API v0.14 Jan 23, 2017
@dns2utf8
Copy link
Contributor

This issues is about to be resolved in #7. Please comment there.

@dbrgn dbrgn closed this as completed in ed00121 Oct 7, 2017
dbrgn pushed a commit that referenced this issue Jan 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants