Robustness, correctness and documentation fixes#14
Merged
Merged
Conversation
Incoming sACN payloads omit channels whose value is 0, so overlaying them onto the retained state left channels that faded to 0 stuck at their old value. This corrupted the full-universe output, the keepalive re-emit and change detection alike. Treat every received data packet as authoritative for all 512 slots: rebuild the full state from a zeroed base each frame and derive changes by diffing against the previous state, so a channel dropping to 0 is now detected and emitted correctly.
sendData now clones the message payload before sending, so a downstream node can never mutate the receiver's internal universe cache by holding on to the emitted object. emitFull no longer needs its own defensive copy.
Both nodes create an sACN socket but never listened for its "error" event. An emitted error with no listener is an uncaught exception in Node.js and could bring down the entire Node-RED runtime (e.g. a busy port or a bad interface address). Attach an error listener to each that logs the error and shows it on the node, so the failure is visible but the runtime and the remaining flows keep going.
The output node gave no feedback at all about what it was doing. Display a node status with the configured universe and refresh rate so it is obvious at a glance that the sender is configured and active.
The send promise was discarded with void, so a rejected send became an unhandled promise rejection which can terminate the process. Handle the result instead: reaffirm the status on success and log the error plus show a red status on failure.
The scene controller accepted universes up to 65279 while the receiver node uses the E1.31 usable range of 1 to 63999. Align the scene controller on 1 to 63999 so the whole package validates universes consistently.
Both editor definitions defaulted the node name to "Scene-Controller", so a
freshly dropped sACN in or sACN out node showed the wrong label on the
canvas. Clear the default name and fall back to a short type-and-universe
label ("sACN in · U1") when the user has not set a custom name.
The universe field was a text input carrying numeric min/max/step attributes that a text input ignores; make it a number input like the out node. The mode dropdown pre-selected passthrough while the node default is htp; mark htp as the selected option so the form matches the actual default.
The code-style and TypeScript workflows used npm install, which can resolve different versions than the lockfile. Switch them to npm ci for reproducible installs from package-lock.json.
Add type-check, lint and build steps before publishing so a broken build can never be released, and pin the third-party github-action-json action to a specific commit instead of a moving main ref to remove that supply-chain risk.
Correct "Paremeter", "wether" and "send by sACN" in the readme and the "IP-addresse" label of the out node.
Add the output trigger, keepalive interval and blank-on-universe-change options to the sACN in parameter table, describe switching the observed universe at runtime via a message, note the Node-RED and Node.js version requirements, and correct the scene controller sections (empty parameter table and the universe input, which is only required for a single universe).
Add a section spelling out what the underlying library does not provide: no universe discovery, no synchronization, no stream termination on stop, and DMX values represented as percentages rather than raw 8-bit values, so integrators can judge compatibility up front.
Spell out that this is not a certified safety system and must not be used for emergency lighting or the functional safety of machinery, and add notices for strobe/photosensitivity and laser control, with the relevant DE/AT/CH frameworks so operators know where responsibility lies.
sACN has no stream-termination handling here, so on stop or redeploy receivers held the last look until their own signal-loss timeout. Add an opt-in setting that sends an all-zero frame before the socket is closed, so the output can be defined to go dark on shutdown.
The zeroed-universe builder, the DMX value type and the interface/port resolution were copied across the receiver, sender and scene controller. Move them into a small shared backend library and use it from all three nodes. Editor (frontend) code stays separate as it cannot share runtime modules.
All three nodes now offer a "Values" setting choosing between percentage (0–100) and raw DMX values (0–255). The receiver converts the library's percentages to the chosen scale, the sender switches the library between scaled and raw output, and the scene controller validates against the matching range and preserves fractional percentages. Editor forms, locales and help texts are updated accordingly; the default stays percent so existing flows keep working.
Add an admin endpoint that lists the host's IPv4 interfaces and offer them as suggestions on the interface field of both nodes, so the interface can be picked instead of typed. Replace the length-based interface guess with real IPv4 validation when binding, and validate the universe, port and interface fields in the editor. Binding to a chosen interface also avoids multicast-routing ambiguity on hosts with several gateways.
Scenes were kept in the in-memory context and lost on every Node-RED restart. Store them as a JSON file per node in a sacn-scenes directory under the Node-RED user directory, loaded on start and written on every save and reset, so recorded scenes survive restarts. This also replaces the context-key handling and resolves the leftover reset placeholder.
Replace the shared Font Awesome lightbulb (used for both the in and out node) and the scene controller's list icon with three purpose-drawn SVGs: an inbound and outbound signal arrow for the receiver and sender and a set of faders for the scene controller. The node colour is unchanged; the icons ship in an icons directory that Node-RED picks up.
The configuration switched off every no-unsafe rule and no-explicit-any. The code no longer needs that: it uses explicit types and guards rather than any, so turning the rules back on passes cleanly and keeps future unsafe access from slipping in. no-non-null-assertion stays off, as the scene parser still relies on a few asserted values.
List the value scale for all three nodes, the blank-on-stop option for the sender, and note that recorded scenes are now persisted to disk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A batch of self-contained fixes from a full code and QM review. Each change is its own commit.
Correctness (sACN in)
Stability / fault tolerance
UI / editor
Validation
CI / release
npm ciin the CI workflows.Documentation
Not included (deliberate follow-ups)
Percent/absolute value mode, network-interface picker with real IP validation, custom icons, null-on-close option, persistent scenes, and the shared-utils/type-guard refactor — each is a feature better done deliberately (ideally with tests).