fix: shutdown clears delayedMove so abort+autoPickup can't re-engage pump
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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.`);
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user