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

Validation suggestion: StrobeSpeed + ShutterStrobe #3981

Closed
kengruven opened this issue May 20, 2024 · 1 comment · Fixed by #4308
Closed

Validation suggestion: StrobeSpeed + ShutterStrobe #3981

kengruven opened this issue May 20, 2024 · 1 comment · Fixed by #4308
Labels
component-test Affects the automated tests.

Comments

@kengruven
Copy link
Contributor

There are many fixtures (and proposed fixtures) which are written with capabilities like this:

        {
          "dmxRange": [128, 250],
          "type": "StrobeSpeed",
          "speedStart": "1Hz",
          "speedEnd": "20Hz"
        },
        {
          "dmxRange": [251, 255],
          "type": "ShutterStrobe",
          "shutterEffect": "Open"
        }

which is almost certainly wrong: StrobeSpeed is a global setting, not an enabler of the strobe feature, so a sibling capability of type ShutterStrobe is indicative that this isn't what was intended.

In the above case, it should be:

        {
          "dmxRange": [128, 250],
          "type": "ShutterStrobe",
          "shutterEffect": "Strobe",
          "speedStart": "1Hz",
          "speedEnd": "20Hz"
        },
        {
          "dmxRange": [251, 255],
          "type": "ShutterStrobe",
          "shutterEffect": "Open"
        }

It would be great if we could catch this mistake earlier (in the editor, or the validator) so we didn't end up with a bunch of these to fix in review, or which accidentally pass review and end up in OFL.

@FloEdelmann FloEdelmann added the component-test Affects the automated tests. label Sep 30, 2024
@FloEdelmann
Copy link
Member

FloEdelmann commented Oct 8, 2024

I actually found some fixtures where this might be valid (or rather we don't have a better way to model this):

In futurelight/dmh-75-i-led-moving-head, prolights/diamond19 and showline/sl-nitro-510c, one channel defines the strobe type and another channel switches between closed/open shutter, or the selected strobe effect with a variable strobe speed.

I can make the check a little less eager though, so it only warns when there is no other Strobe channel. That seems to work fine and has also revealed the only other fixture with this capability combination (chauvet-professional/colorado-1-solo), where it is a true mistake:

"Strobe": {
"capabilities": [
{
"dmxRange": [0, 9],
"type": "NoFunction"
},
{
"dmxRange": [10, 99],
"type": "StrobeSpeed",
"speedStart": "0Hz",
"speedEnd": "25Hz"
},
{
"dmxRange": [100, 109],
"type": "NoFunction"
},
{
"dmxRange": [110, 179],
"type": "ShutterStrobe",
"shutterEffect": "Lightning",
"speedStart": "slow",
"speedEnd": "fast"
},
{
"dmxRange": [180, 189],
"type": "NoFunction"
},
{
"dmxRange": [190, 255],
"type": "ShutterStrobe",
"shutterEffect": "Strobe",
"randomTiming": true
}
]
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-test Affects the automated tests.
Projects
None yet
2 participants