mvrdevelopment / spec

DIN SPEC 15800 General Device Type Format (GDTF) and My Virtual Rig (MVR) File Format description DIN SPEC 15801
64 stars 13 forks source link

Virtual DMX Channels not accepted in the offset pattern #71

Closed fbeasse closed 3 years ago

fbeasse commented 3 years ago

We received a ticket for the builder with this log:

Robe_Lighting@pixelPATT@15012021v3.gdtf

/dev/stdin:2762: element DMXChannel: Schemas validity error : Element 'DMXChannel', attribute 'Offset': [facet 'pattern'] The value '' is not accepted by the pattern 'None|([0-9]+(,[0-9]+)*)'.

After checking the file, it corresponds to every virtual channel of this fixture, which has an empty string for an offset.

Robe_Lighting@pixelPATT@15012021v3.gdtf.zip

moritzstaffel commented 3 years ago

@Firionus Virtual Channels are allowed to have an empty array here.

Firionus commented 3 years ago

@Firionus Virtual Channels are allowed to have an empty array here.

Sorry to hold your own standard against you, but DIN 15800:2020-07, page 33, Table 46 describes the XML attribute Offset of value type Array of Int as:

Relative addresses of the current DMX channel from highest to least significant; Seperator of values is ","; Special value: “None” – does not have any addresses; Default value: “None”; Size per int: 4 bytes

Therefore, this field must either be some Ints (an empty string is not a valid Int) or be "None". Empty String is not allowed here and should be replaced by "None".

-> I can confirm this as a bug in GDTF Builder.

Edit: Another argument: Why have a special value "None" if it has the same meaning as empty string?

Edit2: Come to think about it, I guess an empty array of Int's would be an empty string in that notation and an empty array is generally an accepted concept in a programming context. So I guess when the standard is interpreted like that, empty string and "None" would both be valid. But the standard explicitly mentions the "None" value, so I think others could come to the same conclusion as me that the "None" value must be used instead of the empty string. Maybe keep the schema like it is, fix it in the builder and allow the empty string in a future standard? That would be restrictive (which is good for compatibility) and would allow flexibility in the future without breaking GDTF backwards compatibility.

moritzstaffel commented 3 years ago

Your're right with the None value. I did not had this on my mind. It maked sense when we still named the Offset Node different.

I think we should allow booth the empty string and the None Value. I will check if the builder handles the None correctly. The empty string will be the default way to print this. But we need to clarify this in the examples of the spec.

@Firionus Thanks for the Feedback.