OpenLightingProject / open-fixture-library

A library and website for lighting technology's DMX fixture definition files.
https://open-fixture-library.org/
MIT License
194 stars 66 forks source link

Fixture valid: Detect out-of-range DMX values #966

Open fxedel opened 5 years ago

fxedel commented 5 years ago

Steps to reproduce

Create a capability that exceeds the maximum DMX value, like so:

    "Mode": {
      "defaultValue": 0,
      "capabilities": [
        {
          "dmxRange": [0, 239],
          "type": "NoFunction"
        },
        {
          "dmxRange": [240, 299],
          "type": "Effect",
          "effectName": "Sound-active",
          "soundControlled": true
        }
      ]
    }

The fixture valid test fails, but primarily because of an error thrown in scale-dmx-values module.

[FAIL] boomtonedj/silentpar-7x10w-5in1.json
└ Error: File could not be imported into model. Error: Given DMX value was outside the given resolution
    at getBytes (/home/felix/Development/open-fixture-library/lib/scale-dmx-values.mjs:105:11)
    at scaleDmxRangeIndividually (/home/felix/Development/open-fixture-library/lib/scale-dmx-values.mjs:44:20)
    at scaleDmxRange (/home/felix/Development/open-fixture-library/lib/scale-dmx-values.mjs:31:10)
    at Capability.getDmxRangeWithResolution (/home/felix/Development/open-fixture-library/lib/model/Capability.mjs:313:72)
    at Capability.get rawDmxRange [as rawDmxRange] (/home/felix/Development/open-fixture-library/lib/model/Capability.mjs:302:17)
    at checkRangeValid (/home/felix/Development/open-fixture-library/tests/fixture-valid.js:484:19)
    at checkDmxRange (/home/felix/Development/open-fixture-library/tests/fixture-valid.js:462:14)
    at checkCapabilities (/home/felix/Development/open-fixture-library/tests/fixture-valid.js:447:31)
    at checkChannel (/home/felix/Development/open-fixture-library/tests/fixture-valid.js:415:5)
    at checkChannels (/home/felix/Development/open-fixture-library/tests/fixture-valid.js:368:9)

Expected behavior

The error should be catched directly by the fixture-valid test, so that the error string includes information about which channel and which capability is affected.

Swiftb0y commented 5 years ago

I am looking into this but I am having some trouble: First of all, isn't this already covered in tests/fixture-valid.js#L462? Second of all, I am having trouble actually reproducing the Issue because it seems like there is no easy way to invoke checkFixture on a single file (in OFL JSON format) locally (without writing a quick wrapper) or am I missing something?

fxedel commented 5 years ago

Hello @Swiftb0y, thanks for having a look at this issue! Currently, there is no easy way to run checkFixture on a single fixture. A command line parameter would definitely be desirable for this (feel free to fix this if you want to).

Back to the topic of this issue: I think the problem was that an out-of-range DMX value caused an error in the scale-dmx-values module, just before the fixture-valid script could check the range and return a good error message. You'll possible need to change the position of the range check before the scale-dmx-values module is used.

FloEdelmann commented 5 years ago

You can run fixtures-valid.js to check all fixtures. For that script, a parameter -f / --fixture that restricts the set of fixtures would indeed be helpful.

Swiftb0y commented 5 years ago

Then I’ll look at implementing that first before working on this issue.

Sent with GitHawk