Use maximum channel offset instead of set length for determining break length

[McLP2]

This should fix unexpected behavior when gdtf files are missing channels.
I've noticed this when using a GDTF for the Cameo HydraBeam RGBW in 56-channel mode where only 40 of the channels are described (as in most files for this fixture on gdtf-share). Previously, this would return a channel count of 40, which caused blender-dmx to ignore channels past 40 on this 56-channel fixture. With this fix, blender-dmx works fine now. Correct me if I'm wrong, but I'd expect the channel count to return the footprint including reserved/undescribed channels, that's why I submitted this patch here instead of blender-dmx.

[vanous]

Thanks for looking into this.

Can you point to the GDTF file you mean? There is an open issue about this: mvrdevelopment/spec#269 and a PR to clarify it: mvrdevelopment/spec#275 , which would make these files incorrect, thus the file authors should fix their files by describing the unused channels with the NoFeature attribute.

If this usage is correct (not described channels are allowed) technically then, the channel_count would be correct and a new footprint or something similar would be needed.

[McLP2]

The files in question are these https://gdtf-share.com/share.php?page=home&manu=Cameo&fix=Hydra%20Beam%204000%20RGBW&rev=66566
technically, these files are indeed incomplete, since the channels do have a function irl, they are just not present in the gdtf. This probably means that for complete files (that adhere to the spec clarification) there would be no difference between a true channel count and the footprint. I think channel count is in this case (absurdly) synonymous to footprint (and not the number of unique channels that are listed in a file), especially since that is also what shows as "channels" next to the mode names on gdtf-share.

[vanous]

Hi, thank you!

I have checked documentation for this fixture and have looked at the file. The file is very obviously broken and it took me less then three minutes to fix it Added Missing Channels to make this file more complient. That is what should be the correct way to address and fix the issue, rather then trying to allow people to distribute broken files. (As for the file: i have not tested in BlenderDMX and i will remove this file later, to not pollute the BlenderDMX Fixtures page.

If you want to fix it for BlenderDMX, better way to do this would be to add the missing channels during parsing in pygdtf - create the channel and assign NoFeature to it. I have done several similar workarounds in pygdtf in the past (missing DMX Modes, missing links to root geometries...). That way, the resulted parsed representation will be correct, there would be no need for extra fields and also no need for some kind of workaround in BlenderDMX (which this PR would still require). This is not optimal (as per my comment about fixing the files which would be the preferred way), but it would would provide more pleasant behavior of the library/BlenderDMX.

[McLP2]

Thanks for clarifying! Would something like self.dmx_channels.append(DmxChannel(dmx_break, [missing_channel])) suffice for adding the missing channels during parsing? I don't think BlenderDMX would require additional changes with this PR to function properly with defects of this kind.

[vanous]

I have just pushed some small changes to allow DmxChannel creation. More work might be needed.

Currently, the DmxMode post-processing logic is in the _read_xml of the DmxMode. This is the place where this needs to be done - check if channels are missing, if yes, create them and so on. I have not checked this further, but probably it will be needed to separate this post-processing logic out into a new function, so it can be called independently, as the values might be dependent on each other.

[vanous]

Hello @McLP2 , are you planning to implement the necessary features as described above? If not and if re-editing GDTF files to make them compliant is working for you, we should close this PR. Thank you!

[vanous]

I have implemented the above mentioned logic here #18. I will close this PR without merging, thank you for trying to help, appreciated!