In order to test the robustness of my new code, I decided to create some very silly fixtures in the QLC Fixture Editor. I created a fixture with two heads, but one head only has 8-bit pan/tilt, while the second head has 16-bit. So there is a LSB pan/tilt channel for head 1, but not head 0.
I discovered that fixture->channelNumber(...) was giving me a valid channel for pan LSB on head 0, even though it didn't have one (only head 1 did). At first I thought this was a bug, but eventually I found the section of code that was doing it, clearly on purpose. The code is in qlcfixturehead.cpp in the cacheChannels(...) method:
Code: Select all
// if this head doesn't include any Pan/Tilt channel
// try to retrieve them from the fixture Mode
if (channelNumber(QLCChannel::Pan, QLCChannel::MSB) == QLCChannel::invalid())
setMapIndex(QLCChannel::Pan, QLCChannel::MSB, mode->channelNumber(QLCChannel::Pan, QLCChannel::MSB));
if (channelNumber(QLCChannel::Pan, QLCChannel::LSB) == QLCChannel::invalid())
setMapIndex(QLCChannel::Pan, QLCChannel::LSB, mode->channelNumber(QLCChannel::Pan, QLCChannel::LSB));
if (channelNumber(QLCChannel::Tilt, QLCChannel::MSB) == QLCChannel::invalid())
setMapIndex(QLCChannel::Tilt, QLCChannel::MSB, mode->channelNumber(QLCChannel::Tilt, QLCChannel::MSB));
if (channelNumber(QLCChannel::Tilt, QLCChannel::LSB) == QLCChannel::invalid())
setMapIndex(QLCChannel::Tilt, QLCChannel::LSB, mode->channelNumber(QLCChannel::Tilt, QLCChannel::LSB));
Anyway, I suggest one of a few possible changes:
1. do the above code only if ALL the pan/tilt channels are missing, instead of one-by-one independently. If they're ALL missing, then fix them all according to the above code.
2. remove the above code altogether and trust / force people to create correct fixture definitions.
3. option 1, but only head 0, so that different heads' channels don't get mixed together
I am going to go for option 3 in my fork, because it seems to make the most sense to me, but I'm starting this thread to get other opinions (especially massimo's) since I easily could be overlooking some case. Option 2 might break some existing fixture definitions.
PS: I've attached the fixture definition I made which exposes this, because I might not be describing it very well. The mode in the definition is called "Buggy" and has one 8-bit head and one 16-bit head.