Conversation
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 5 to 6. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@v5...v6) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
The module's logic lives inside the callbacks passed to DiffCamEngine.init(), which previously made it unreachable without a browser and a webcam. The test helper loads MMM-MotionDetector.js in a vm context with stubs for Module, Log, moment, document and DiffCamEngine, and keeps the options object the engine receives. That exposes captureCallback, so tests can drive the module frame by frame with synthetic scores. Covers the timeout state machine (including timeout: -1 never powering off and DEACTIVATE_MONITOR firing only once), the powered-off time accounting, template data, and socket notification forwarding. Also runs shellcheck over the monitor-commands scripts in CI. No new dependencies, node:test and node:vm are built in. Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: release camera stream tracks in DiffCamEngine.stop() stop() cleared the capture interval and canvas but never stopped the underlying MediaStream tracks, leaving the camera active (and its indicator light on) after stop() was called. * Add engine tests for releasing the camera on stop The engine grabs window, document and navigator at load time, so the mock runs it in a vm context with a fake webcam and hand-driven capture ticks instead of a real interval. The camera release test was verified to fail against the old stop(), which left the tracks live and the camera indicator on. Note: tests/diff-cam-engine-mock.js is also added by the motion box threshold and coords refactor branches. The copies are identical, so whichever merges second can take either side. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: validate platform config before building command script path this.config.platform was interpolated directly into a script filename and shell-executed without validation, so a typo'd or unknown platform value silently failed at exec time with a confusing 'command not found'-style error instead of a clear one. Now unknown platforms are rejected up front with a message listing the valid options. * Add node helper tests covering platform validation The helper requires "node_helper" and "../../js/logger", neither of which resolves when this repository is checked out on its own, and it shells out via child_process. The mock swaps all three through a temporary require hook, so the tests run standalone in CI and never execute a real command. Both rejection tests were verified to fail against the unvalidated helper, including the one covering a platform that traverses out of the module directory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* refactor: remove unnecessary module-level coords variable calculateMotionBox(currentMotionBox, x, y) was reaching back into the coords closure for its box-init case even though it always receives the same x/y values as arguments (assigned from calculateCoordinates right before the call). Using the parameters directly removes the implicit global and makes the data flow explicit; behavior is unchanged. * Add engine tests pinning motion box coordinates This branch is a behaviour preserving refactor, so the tests pass against the engine both before and after it. That is the point: they pin the coordinates the shared coords variable used to produce, including that nothing leaks between consecutive captures. Kept in its own file so it does not collide with the other engine test suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Supersedes the seven open dependabot pull requests and goes past several of them, since the proposals had aged: eslint 10.7.0 rather than 10.0.2, cspell 10.0.1 rather than 9.7.0, lint-staged 17.1.0 rather than 16.3.0. The two transitive bumps are picked up by the regenerated lockfile, flatted at 3.4.2 and picomatch at 4.0.5. Six of these are major bumps: eslint 9 to 10, @eslint/js 9 to 10, @eslint/json 0.14 to 2, @eslint/markdown 7 to 8, cspell 9 to 10, lint-staged 16 to 17 and globals 16 to 17. The flat config needed no changes, and lint, formatting, spelling and the unit tests all pass after a clean npm ci. Note that lint-staged now requires node >= 22.22.1 and cspell >= 22.18.0. CI resolves node-version 22 to the newest 22.x so it is unaffected, but anyone running an older 22.x locally will need to update. Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: remove dead MOTION_DETECTED socket notification node_helper.js never handled the MOTION_DETECTED socket notification, so sending it was a no-op. The sendNotification broadcast to other modules is what actually matters and is kept. * Update motion notification test for the removed socket call The suite landed after this branch was cut and asserted that motion is announced on both buses. The node helper has no MOTION_DETECTED handler, so the socket call this branch removes was dead. Assert the module bus only, and add a regression test pinning that the socket call stays gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: guard against null lastTimeMotionDetected in getTemplateData lastTimeMotionDetected is always set today before any render, but getTemplateData() unconditionally called .toLocaleTimeString() on it, which would throw if it were ever null/unset (e.g. rendered before start() finishes, or on init error). Guard it so the template gets null instead of crashing the module. * Add regression test for the null lastTimeMotionDetected guard Verified to fail against the unguarded getTemplateData and pass with the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: use consistent >= comparison for motionBox threshold hasMotion used score >= scoreThreshold while motionBox was only set for score > scoreThreshold, so a score exactly at the threshold was reported as motion but had no motion box. * Add engine tests for the motion box threshold boundary Pins the case the fix addresses: at a score exactly equal to scoreThreshold the frame already counts as motion, but the old strict comparison withheld the motion box. Both boundary tests were verified to fail before the fix. Kept in its own file so it does not collide with the other engine test suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
init() read its options with ||, so a configured zero was indistinguishable from an omitted option and silently became the default. scoreThreshold is a documented config option, so setting it to 0 in config.js to catch the slightest motion quietly behaved as 16 instead. Switch the two thresholds and jpegQuality to ??, which falls back only when nothing was passed. The remaining || defaults are left alone: zero is not a meaningful capture interval or canvas dimension, so defaulting there still guards against a broken config rather than discarding intent. Four of the new tests were verified to fail against the old defaults. Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* refactor: skip redundant isMonitorOn status check before toggling activateMonitor/deactivateMonitor each spawned an extra 'status' subprocess before running the on/off command. All monitor-commands-*.sh scripts are idempotent (turning an already-on/off display on/off again is a no-op), so the pre-check added a subprocess spawn per toggle for no behavioral benefit. isMonitorOn() had no other callers and is removed along with it. * Add node helper tests for the removed status check Pins that activating and deactivating each issue exactly one command and that the status script is never invoked. Also covers activating while the monitor already reports ON, which the old status check skipped entirely. Kept in its own file so it does not collide with the platform validation suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Documented behaviour that did not match the code: - the platform option listed mac-intel twice and never mentioned mac-arm, although the helper accepts it and monitor-commands-mac-arm.sh exists - the notifications table listed DEACTIVATE_MONITOR as if other modules could observe it. It is only ever sent over the socket to this module's own node helper, and its documented percentageOff payload does not exist anywhere in the code. MOTION_DETECTED sends an object rather than a bare number - scoreThreshold now honours an explicit 0, so say so The requirements section contradicted itself: it suggested setting the address to 0.0.0.0 to fix camera problems, while the proxy section further down exists precisely because 0.0.0.0 stops the browser granting camera access. Reworded to match, and linked the two together. The Raspberry Pi OS section is restored in shortened form. The old workaround relied on the legacy camera stack, which is gone on Bookworm, but issue #56 is still open so the limitation is worth naming rather than dropping silently. Also: the proxy example could not run as written, since http-proxy-middleware has exported createProxyMiddleware rather than a callable default since v1 and the loop variable was undeclared. The PI-camera steps were an indented code block rather than the list they were meant to be, and Tested devices sat between Configuration options and its own deviceId subsection, which left that subsection nested under the wrong heading. Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The module forwarded USER_PRESENCE from its node helper to the module bus, but the helper has never sent it. git log -S "USER_PRESENCE" finds the string in exactly two commits across the whole history: the initial import in 2016, which carried it over from the upstream alexyak/motiondetector project, and the test suite added last week. There has never been a sender in any file. socketNotificationReceived only ever hears from a module's own node helper, and this helper sends nothing back at all, so the branch has been unreachable for the entire life of the repository. The handler existed solely for this one notification, so it is removed rather than emptied. The two tests covering it go with it. They passed, but pinned behaviour no user could trigger, so keeping them would have implied the path worked. The node helper keeps its own socketNotificationReceived, which handles the live module to helper direction for INIT_MONITOR, ACTIVATE_MONITOR and DEACTIVATE_MONITOR. Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Bump dependencies into the new year (#81) * Bump @eslint/js from 9.39.1 to 9.39.2 Bumps [@eslint/js](https://github.com/eslint/eslint/tree/HEAD/packages/js) from 9.39.1 to 9.39.2. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](https://github.com/eslint/eslint/commits/v9.39.2/packages/js) --- updated-dependencies: - dependency-name: "@eslint/js" dependency-version: 9.39.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump prettier from 3.7.3 to 3.7.4 Bumps [prettier](https://github.com/prettier/prettier) from 3.7.3 to 3.7.4. - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](prettier/prettier@3.7.3...3.7.4) --- updated-dependencies: - dependency-name: prettier dependency-version: 3.7.4 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump cspell from 9.3.2 to 9.4.0 Bumps [cspell](https://github.com/streetsidesoftware/cspell/tree/HEAD/packages/cspell) from 9.3.2 to 9.4.0. - [Release notes](https://github.com/streetsidesoftware/cspell/releases) - [Changelog](https://github.com/streetsidesoftware/cspell/blob/main/packages/cspell/CHANGELOG.md) - [Commits](https://github.com/streetsidesoftware/cspell/commits/v9.4.0/packages/cspell) --- updated-dependencies: - dependency-name: cspell dependency-version: 9.4.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump eslint from 9.39.1 to 9.39.2 Bumps [eslint](https://github.com/eslint/eslint) from 9.39.1 to 9.39.2. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v9.39.1...v9.39.2) --- updated-dependencies: - dependency-name: eslint dependency-version: 9.39.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Bump globals --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: veeck <gitkraken@veeck.de> * Bump dependencies into february (#86) * Bump cspell from 9.4.0 to 9.6.2 Bumps [cspell](https://github.com/streetsidesoftware/cspell/tree/HEAD/packages/cspell) from 9.4.0 to 9.6.2. - [Release notes](https://github.com/streetsidesoftware/cspell/releases) - [Changelog](https://github.com/streetsidesoftware/cspell/blob/main/packages/cspell/CHANGELOG.md) - [Commits](https://github.com/streetsidesoftware/cspell/commits/v9.6.2/packages/cspell) --- updated-dependencies: - dependency-name: cspell dependency-version: 9.6.2 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump prettier from 3.7.4 to 3.8.1 Bumps [prettier](https://github.com/prettier/prettier) from 3.7.4 to 3.8.1. - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](prettier/prettier@3.7.4...3.8.1) --- updated-dependencies: - dependency-name: prettier dependency-version: 3.8.1 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump globals from 17.0.0 to 17.3.0 Bumps [globals](https://github.com/sindresorhus/globals) from 17.0.0 to 17.3.0. - [Release notes](https://github.com/sindresorhus/globals/releases) - [Commits](sindresorhus/globals@v17.0.0...v17.3.0) --- updated-dependencies: - dependency-name: globals dependency-version: 17.3.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Bump @eslint/json from 0.14.0 to 1.0.0 Bumps [@eslint/json](https://github.com/eslint/json) from 0.14.0 to 1.0.0. - [Release notes](https://github.com/eslint/json/releases) - [Changelog](https://github.com/eslint/json/blob/main/CHANGELOG.md) - [Commits](eslint/json@json-v0.14.0...json-v1.0.0) --- updated-dependencies: - dependency-name: "@eslint/json" dependency-version: 1.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump minimatch (#87) Bumps and [minimatch](https://github.com/isaacs/minimatch). These dependencies needed to be updated together. Updates `minimatch` from 3.1.2 to 3.1.5 - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](isaacs/minimatch@v3.1.2...v3.1.5) Updates `minimatch` from 10.0.3 to 10.2.4 - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](isaacs/minimatch@v3.1.2...v3.1.5) --- updated-dependencies: - dependency-name: minimatch dependency-version: 3.1.5 dependency-type: indirect - dependency-name: minimatch dependency-version: 10.2.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump version * Refuse to toggle the monitor without a valid platform Addresses the Copilot review on #108. initMonitor rejected an unknown platform, but getCommandScript fell back to x11 and nothing else checked, so the rejection only stopped the initial activation. Every later ACTIVATE_MONITOR and DEACTIVATE_MONITOR still ran the x11 script. Confirmed by test: configuring platform "wayland" and letting the timeout elapse executed monitor-commands-x11.sh off on a machine the user never said was x11. Both toggles now refuse and log when no valid platform was accepted, and the x11 fallback is gone rather than left as an unreachable default. The stale platform case from the same review is deliberately left as is. The node helper is shared by every instance of this module, so clearing a platform that was already accepted would let a typo in a second instance break a first one that is working. A test now pins that. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Do not treat a still frame as motion, and log real exec failures Addresses the second round of Copilot review on #108. A score of zero means not a single pixel exceeded pixelDiffThreshold, but hasMotion used score >= scoreThreshold, so at scoreThreshold 0 a completely still frame reported motion. The monitor would then never power off. This only became reachable once an explicit zero started being honoured, so it arrived with that fix rather than before it. Both the hasMotion flag and the motionBox guard now go through one meetsScoreThreshold helper that also requires a non-zero score, so the two rules cannot drift apart again. The exec failure handlers read error.stderr, which is only populated when a script actually ran and failed. A failure to spawn logged "error activating monitor: undefined", which says nothing useful in a deployment. They now fall back to the error message. The child_process.exec stub called back with a single object rather than Node's (error, stdout, stderr). The promisified result happened to come out the same shape, so nothing was broken, but only by accident. It now uses the real callback signature and carries the promisify.custom symbol the way exec does. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Run the monitor scripts without a shell, and declare the node engine Addresses the third round of Copilot review on #108. The monitor scripts were run through exec with the script path interpolated into a shell string. A module directory containing a space would split the path and the command would fail. Switched to execFile, which takes the arguments as a list, so no shell parses the path at all. The mock follows the real execFile signature and now also records the raw argument list, so a test can assert the path stays a single argument. package.json declares no engines range while the toolchain does. The strictest requirements are lint-staged at >=22.22.1 and eslint, which does not support node 23, so the intersection is ^22.22.1 || >=24. Checked that CI's 22.23.1 satisfies it and that 22.18.0 and 23.x do not, which is the point: contributors now get a clear npm error instead of a confusing failure part way through a lint-staged run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Update comments left stale by the execFile switch Addresses the fourth round of Copilot review on #108. describeError still documented itself in terms of exec. Swept the rest of the same staleness while there: the mock's option was still called exec although it now stubs execFile, which would have been the next comment of the same kind. Renamed it to run and updated the surrounding docs. Comments only, no behaviour change, and the suite is unchanged at 61 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Drop the setter workaround from the motion box tests Addresses the fifth round of Copilot review on #108. These tests reached for setScoreThreshold(0) because init used to read the option with a || default, so a zero passed there became 16. That stopped being true once init switched to ??, which makes both the comment and the workaround wrong. They now pass scoreThreshold: 0 straight to init, which is also how a user would configure it. The setter path stays covered by its own tests in diff-cam-engine-options.test.js, so nothing is lost. Tests only, still 61 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Tear the stream down properly in DiffCamEngine.stop() Addresses the sixth round of Copilot review on #108. start() registers a canplay listener and only startComplete() removed it, so stopping between start() and the stream becoming ready left the listener in place. The browser then delivered canplay anyway, startComplete() ran, and the capture interval began ticking against a camera whose tracks stop() had just released. Confirmed by test before fixing. stop() now removes the listener, clears the interval handle, drops the stream reference and nulls srcObject. Releasing the stream also means start() after stop() throws rather than appearing to succeed with dead tracks, which is the honest outcome, a stopped engine has to go back through init. Not reachable from the module today, which never calls stop(), but stop() is part of the engine's public surface and releasing the tracks without detaching the listener made the inconsistency worse rather than better. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix the findings from an adversarial review of the release A hostile review pass over the whole release diff turned up seven issues that six rounds of automated review had missed. pixelDiffThreshold of 0 made a still scene score the entire frame. The same ?? change that made an explicit zero reachable for scoreThreshold made it reachable here too, but nothing downstream was checked: pixelDiff >= 0 is true for every pixel, so the score could never be zero and the "a score of zero is never motion" guard could never fire. The normalisation also divided by zero and wrote NaN across the motion canvas. An unchanged pixel is now never significant, mirroring the score rule, and the divisor falls back to one. The test that was supposed to cover that option asserted the broken behaviour as correct, so it is replaced by one that counts only changed pixels plus one that pins a still frame at zero. Monitor commands ran concurrently. The scripts are idempotent but not instant, so a slow off could land after a later on and leave the screen dark, and the module never retries because it has already recorded the monitor as awake. They are queued now, so the last requested state wins. Refusing to toggle without a valid platform resolved rather than rejected, so the helper logged that the monitor had been activated directly after logging that it could not be. It throws now and the existing catch handlers report it. The powered off percentage was computed before the current stretch was booked, so it read zero for as long as the monitor was off and reported the previous total at the moment of waking, which is the one moment it is logged. Also: the release shipped no changelog entry at all, the README described a score that can no longer occur, the engine mock's removeEventListener ignored the handler argument so a test could pass while removing the wrong listener, jpegQuality had no coverage at all, one toggling test only asserted the mock, and two files declared the same describe block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: veeck <gitkraken@veeck.de> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
rejas
marked this pull request as ready for review
July 21, 2026 17:55
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.
No description provided.