From 8f9150e1601d68c01a0c76d697e72e3fe0063833 Mon Sep 17 00:00:00 2001 From: Rene De Ren Date: Sat, 9 May 2026 18:17:45 +0200 Subject: [PATCH] fix: shutdown clears delayedMove so abort+autoPickup can't re-engage pump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When PS commanded turnOffAllMachines, executeSequence's interruptible abort path triggered transitionToState('operational'), which auto-picked up the queued delayedMove and re-started the pump. Pump bounced accelerating ↔ decelerating forever and never reached idle. Clear state.delayedMove at the top of shutdown/emergencystop sequences so a user-commanded stop cancels any pending move. Observed live: in pumpingstation-complete-example the basin drained past stopLevel and equilibrated at ~0.3 m with one pump stuck at min flow. With this fix pumps shut down cleanly at stopLevel. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/specificClass.js | 11 ++- .../shutdown-sequence.integration.test.js | 74 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/specificClass.js b/src/specificClass.js index d394cc4..258c388 100644 --- a/src/specificClass.js +++ b/src/specificClass.js @@ -885,13 +885,22 @@ _callMeasurementHandler(measurementType, value, position, context) { return; } + // A shutdown/emergency-stop must cancel any pending move. Without this, + // the abort path below (returnToOperational=true) lets state.transitionToState + // auto-pick up state.delayedMove as soon as it lands in 'operational', + // which re-engages the pump on every shutdown attempt — pump bounces + // forever between accelerating and decelerating and never reaches idle. + const interruptible = new Set(["shutdown", "emergencystop"]); + if (interruptible.has(sequenceName)) { + this.state.delayedMove = null; + } + // Interruptible movement: if a shutdown or emergency-stop is requested // while a setpoint move is mid-flight (accelerating/decelerating), abort // the move first and wait briefly for the FSM to return to 'operational'. // Without this, transitions like accelerating->stopping are rejected by // stateManager.isValidTransition, leaving the machine running. const currentState = this.state.getCurrentState(); - const interruptible = new Set(["shutdown", "emergencystop"]); if (interruptible.has(sequenceName) && (currentState === "accelerating" || currentState === "decelerating")) { this.logger.warn(`Sequence '${sequenceName}' requested during '${currentState}'. Aborting active movement.`); diff --git a/test/integration/shutdown-sequence.integration.test.js b/test/integration/shutdown-sequence.integration.test.js index 5ec07ad..c08f0e9 100644 --- a/test/integration/shutdown-sequence.integration.test.js +++ b/test/integration/shutdown-sequence.integration.test.js @@ -70,3 +70,77 @@ test('exitmaintenance requires mode with exitmaintenance action allowed', async await machine.handleInput('fysical', 'exitMaintenance', 'exitmaintenance'); assert.equal(machine.state.getCurrentState(), 'idle'); }); + +test('shutdown clears delayedMove synchronously, before the abort/await path runs', async () => { + // Regression: when MGC parks a setpoint in state.delayedMove during a + // dead-zone keep-alive, then PS commands shutdown via turnOffAllMachines, + // the shutdown's interruptible-abort path triggers transitionToState + // ('operational'), which auto-picks up delayedMove and re-starts the + // pump. Pump bounces accelerating ↔ decelerating forever and the + // shutdown sequence never reaches idle. Observed live in the + // pumpingstation-complete-example demo: basin drained past stopLevel + // with one pump stuck at minimum flow. + // + // Fix: executeSequence clears state.delayedMove for shutdown/emergencystop + // BEFORE the abort+await path. Asserting synchronously (race the first + // microtask) is the precise behavioural check — without the fix, the + // auto-pickup could still re-engage the pump on the way to idle even if + // the value is null after the call returns. + + const slowMove = makeStateConfig({ + movement: { mode: 'staticspeed', speed: 50, maxSpeed: 100, interval: 10 }, + }); + const machine = new Machine(makeMachineConfig(), slowMove); + + await machine.handleInput('parent', 'execSequence', 'startup'); + assert.equal(machine.state.getCurrentState(), 'operational'); + machine.setpoint(80); + await new Promise((r) => setTimeout(r, 50)); + assert.equal(machine.state.getCurrentState(), 'accelerating'); + + machine.state.delayedMove = 75; + + // Kick off the shutdown but do not await — capture state before the + // abort path's await yields. + const shutdownPromise = machine.handleInput('parent', 'execSequence', 'shutdown'); + // Yield once to allow the synchronous prelude of executeSequence to run + // (lookup, lowercase, the new delayedMove=null assignment) without + // letting any await resolve. + await Promise.resolve(); + assert.equal(machine.state.delayedMove, null, + 'delayedMove must be cleared synchronously by the shutdown prelude — otherwise the abort path will auto-pick it up'); + + await shutdownPromise; + assert.equal(machine.state.getCurrentState(), 'idle'); +}); + +test('emergencystop also clears queued delayedMove', async () => { + const machine = new Machine(makeMachineConfig(), makeStateConfig()); + + await machine.handleInput('parent', 'execSequence', 'startup'); + await machine.handleInput('parent', 'execMovement', 30); + machine.state.delayedMove = 60; + + await machine.handleInput('parent', 'execSequence', 'emergencystop'); + + assert.equal(machine.state.delayedMove, null, + 'emergency-stop must clear delayedMove'); +}); + +test('startup does NOT clear delayedMove (only shutdown/emergencystop does)', async () => { + // delayedMove serves a legitimate purpose for non-stop sequences — e.g. + // setpoints arriving while the pump is in 'starting' get queued and + // auto-picked-up when state lands in 'operational'. The fix must be + // narrowly scoped to interruptible (stop) sequences. + const machine = new Machine(makeMachineConfig(), makeStateConfig()); + + await machine.handleInput('parent', 'execSequence', 'startup'); + machine.state.delayedMove = 42; + + // Re-running startup from operational is a no-op for state, but the + // delayedMove must still be there afterwards for the auto-pickup to fire. + await machine.handleInput('parent', 'execSequence', 'startup'); + + assert.equal(machine.state.delayedMove, 42, + 'non-stop sequences must preserve delayedMove for the auto-pickup'); +});