ssilverman / rdm-schema

The schema for the Parameter Metadata Language from Section 5 of E1.37-5.
10 stars 3 forks source link

DISPLAY_LEVEL's restrictToLabeled should be false? #18

Closed Bartel-C8 closed 2 years ago

Bartel-C8 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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