Refresh cstate.endTime when active schedule window advances (closes #1192)#1193
Open
pawmmm wants to merge 1 commit into
Open
Refresh cstate.endTime when active schedule window advances (closes #1192)#1193pawmmm wants to merge 1 commit into
pawmmm wants to merge 1 commit into
Conversation
Two related fixes to the cstate.endTime refresh path so that checkCircuitEggTimerExpirationAsync does not force-turn-off a circuit whose schedule is still active. SystemBoard.syncScheduleStates previously called setEndTime only when schedIsOn != ssched.isOn. For a continuously-active schedule (e.g. a 12:00 AM -> 12:00 AM window) that transition never happens at the local midnight rollover, so cstate.endTime froze at the previous day's value while ssched.scheduleTime.endTime advanced. Within ~1 minute after midnight the circuit was being turned off in error. Now we also refresh when the schedule is still on, the circuit is still on, and the schedule's endTime has advanced past the cached cstate.endTime. IntelliCenterBoard.syncScheduleStates had `circs.push;` (a reference to the method, not a call) at line 4817, so the array stayed empty and the subsequent refresh loop never ran. Same intent, same bug class. Fixes tagyoureit#1192
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.
Summary
SystemBoard.syncScheduleStates: also refreshcstate.endTimewhen an active schedule'sscheduleTime.endTimehas advanced past the cachedcstate.endTime(previously only happened onschedIsOntransitions, which never occur for continuous 24-hour schedules).IntelliCenterBoard.syncScheduleStates: fixcircs.push;(method-reference, no-op) →circs.push(c);so the existing refresh loop actually runs.Both edits are about the same class of bug — a stale
cstate.endTimeafter the schedule's window rolls forward — observed in production on Nixie and dead-coded on IntelliCenter.Reference issue
Closes #1192 — full trace and a real-world journal excerpt are in the issue.
Short version: a
12:00 AM → 12:00 AMcontinuous schedule on a Nixie filter pump causedcheckCircuitEggTimerExpirationAsyncto fire the morning after midnight (~00:01:02), with the pump then staying off ~6.5 hours until the user manually re-armed it. The schedule's ownshouldBeOnwas correctlytruethroughout; the off-event came from comparing the live time against the previous day's stalecstate.endTime.Test plan
tsc --noEmitpasses (baseline clean before, still clean after).12:00 AM → 12:00 AMschedule on a filter pump circuit (all 7 days), confirm the circuit stays on across multiple midnight rollovers and noNCP: Setting Circuit ... to falselines appear in the journal at00:0X.cstate.endTimeupdates within ~10s of a schedule edit that changes the endTime (previously this path was a no-op due to thecircs.push;typo).Risk
The new branch in
SystemBoardonly fires when:schedIsOn === true), ANDscirc.isOn === true), ANDscirc.endTimeandssched.scheduleTime.endTimeare defined, ANDscirc.endTime(i.e. the cached value is genuinely stale).It calls the same
setEndTimepath that the existing transition branch uses, just under a different precondition. The IntelliCenter change is a one-character fix to a loop that was previously dead — so its behavior change is just "the loop runs as originally intended."Out of scope (documented in #1192)
Once this lands, the secondary "stuck
ssched.triggered" behavior incontroller/nixie/schedules/Schedule.ts:120-188becomes moot for the continuous-schedule case — the egg-timer expiration no longer fires, so the schedule never needs to re-arm itself after a spurious off. Whether the trigger loop should also re-arm after an external off-event is tangled with Manual OP semantics; happy to file as a follow-up if maintainers want it addressed separately.