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

DISPLAY_LEVEL's restrictToLabeled should be false? #18

Closed
Bartel-C8 opened this issue May 5, 2022 · 2 comments · Fixed by #19
Closed

DISPLAY_LEVEL's restrictToLabeled should be false? #18

Bartel-C8 opened this issue May 5, 2022 · 2 comments · Fixed by #19
Assignees

Comments

@Bartel-C8
Copy link

https://github.com/ssilverman/rdm-schema/blob/master/examples/e1.20/DISPLAY_LEVEL.json

The spec states:
To turn the display off, Display Level shall be set to 0. To turn the display on full, Display Level shall be set to 0xFF. Any value in between represents a relative intensity setting. If the device does not support relative intensity settings, any non-zero value shall be interpreted as on.

So, it should not be (by default) restricted to the current: ?
{ name: "Off", value: 0 }, { name: "Full", value: 255 },

@peternewman
Copy link
Contributor

peternewman commented May 10, 2022

OLA/OLP instead currently goes the other way, not even mentioning the labels at all:
https://github.com/OpenLightingProject/rdm-app/blob/220fdfeaa32b7b9ed745caebde6d2abe0ee09eb9/data/pid_data.py#L4516-L4524

I think this does nicely prove that the "examples" are actually nearly as important as the schema. Once you've got a schema, people are going to want to be able to download all the standardised PIDs in the format of that schema. A bit like the existing header file Scott makes, but given the schema is standardised I'd say it can be even more official.

Or to put it another way, if you provide "examples" of the standardised PIDs people will use them/take them as gospel without checking, so they need to be perfect.

@ssilverman
Copy link
Owner

@Bartel-C8 you’re quite correct. This should be changed.
@peternewman I strongly agree with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants