Repository navigation
fix(control): a site meter that repeats one reading counts as stale - #1921
Conversation
A meter that answers every poll with the same values passed the dispatch gate, which checks only UpdatedAt. In seed 207537 it repeated one sample for 39.9 s; the battery wound from +4 kW to -5 kW on it and an EV rise went out. Track the meter's total and phase values against FTW's own batteries and chargers. A device's move counts as the smaller of its order's move and its own reading's move, so only orders a device followed count. Once they move more than 500 W and the meter repeats the same values for 3 more polls and 15 s, the gate reports site_meter_stale. Any changed value reopens it. 500 W is far above meter resolution and inverter dead bands; 15 s covers a 10 s HAN or DSMR 4 refresh plus a poll. Item C5 of the random-rig triage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3f148c7cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| then, ok := a[k] | ||
| if !ok || !then.read || !now.read { | ||
| continue |
There was a problem hiding this comment.
Establish baselines for devices first commanded during a freeze
If the meter starts repeating before the first battery dispatch, its baseline contains no batteries: NewState initializes PrevTargets empty, and ObserveSiteMeterFreeze runs before ComputeDispatch populates it. This branch then skips that battery forever because f.devices is refreshed only when the meter values change. Even subsequent confirmed multi-kilowatt battery movements never mark the meter frozen, leaving dispatch enabled indefinitely. Establish a baseline when a newly commanded device first appears rather than requiring a changed meter reading.
AGENTS.md reference: AGENTS.md:L66-L66
Useful? React with 👍 / 👎.
| if !ok || !then.read || !now.read { | ||
| continue | ||
| } | ||
| moved += min(math.Abs(now.orderW-then.orderW), math.Abs(now.readW-then.readW)) |
There was a problem hiding this comment.
Account for cancellation between managed device movements
For a supported total-only meter, a battery increasing discharge by 2 kW while an EV increases consumption by 2 kW legitimately leaves the site reading unchanged. Summing absolute device movements reports 4 kW of movement instead of zero, so after 15 seconds this healthy meter is declared stale and all dispatch, including EV charging, is stopped. Account for the signed net effect at the site boundary before concluding that the meter must have changed.
AGENTS.md reference: AGENTS.md:L63-L63
Useful? React with 👍 / 👎.
| f.repeats++ | ||
| if !f.frozen && f.repeats >= meterFreezeSamples && now.Sub(f.movedAt) >= meterFreezeAfter { | ||
| f.frozen = true |
There was a problem hiding this comment.
Reset the detection window when device movement disappears
If a battery moves above the threshold and returns to its baseline before the next legitimate 10-second meter telegram, the meter can correctly keep reporting the same values. However, once movedAt is set, this branch never checks movement again: it declares the meter frozen 15 seconds later even when devicesMovedW is now zero. This shuts down healthy dispatch for a transient the meter never sampled. Clear or restart the pending verdict when the qualifying movement no longer exists.
Useful? React with 👍 / 👎.
…ery device Review of #1921: - Wait 30 s, not 15 s, after the move. A 10 s HAN or DSMR 4 meter that loses one telegram repeats its values for 20 s; 15 s tripped it. - Count every battery and charger in telemetry, with a missing order as 0 W. A meter frozen since start, or a car plugged in during a freeze, was never caught. - Sum the moves with their sign, as the meter sees them. Two batteries trading 1.5 kW no longer count as 3 kW. A move that comes back before the meter changes no longer counts. - One gate function, siteDispatchGateLocked, for the tick and operator setpoints, with a test that drives it to site_meter_stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A site meter that repeats its values after FTW's devices moved more than 500 W now counts as a held sample for battery dispatch from the first repeat, as #1879 does for a sample with an old timestamp. Seed 207537 wound the battery from +5 kW to -5 kW in the 30 s before the frozen verdict; the PI now holds it after its first followed move. The freeze check reads the meter and every battery and charger through health-gated telemetry accessors, so an offline driver's cached reading never counts. Setup's check before it enables control applies the same frozen verdict as the tick. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s for Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Item C5 of the random-rig triage (27 Oct). A site meter that answers every poll with the same total and phase values counts as fresh, because the dispatch gate checks only
UpdatedAt, and each repeated answer is a new sample.Seed 207537: the rig's meter repeated one sample (2667 W, L1/L2/L3 2.4/5.5/-0.2 A) for 39.9 s. Dispatch acted on it the whole time. The battery PI wound from +5 kW to -5 kW, and an EV rise from 0 to 6 A went out 38 s into the freeze ("no raise on a stale meter" failed). #1879 (merged) covers only a meter whose
UpdatedAtstops moving.Change
Owner rule: total and phases unchanged over N polls while FTW's own orders moved by more than a set amount means stale. Stop dispatch and raise no EV.
control.State.ObserveSiteMeterFreeze(store, now)(newgo/internal/control/meter_freeze.go) runs every tick under the control lock. It compares each new meter sample (total W, plusl1..l3A, W and V when present) with the one before. When the values change, it records where every battery and charger stands: FTW's last order (PrevTargets,evCommanded; none counts as 0 W) and the device's own reading.telemetry.Store.OnlineMeterSample,OnlineRawWByType). A watchdog-offline driver drops out, so its cached reading never counts.siteMeterHeld, as in fix(control): act only on new site-meter samples in battery dispatch #1879): the PI holds its targets instead of acting on the same error again, and other paths may only move a battery toward 0 W.cmd/ftw:siteDispatchGateLockedis the one gate the tick andsiteDispatchNow(operator setpoints) share. Setup's check before it enables control now uses the same gate (siteMeterGateLocked) with the candidate's fuse. A frozen meter givessite_meter_stale: drivers return to their default mode,TickWithDispatch(false)stands the chargers down, and the battery gets no new orders.Why these numbers
command_response_s: 40,startup_delay_s: 30). Real cars take as long to wake. Until then a live meter with no noise repeats itself. Counting the accepted offer would mark that live meter stale and send the chargers to 0 W.For seed 207537 the battery reading shows a 924 W move about 1.6 s into the freeze. From then the PI holds the battery, and the meter goes stale about 31.6 s into the freeze, before the 38 s EV rise. The control test that replays the seed with a one-tick battery lag holds the battery between 3208 and 4075 W and goes stale at 34 s. The runner should confirm this on the rig.
Second review changes (77bcd3d)
Tests
go/internal/control/meter_freeze_test.go:TestFrozenMeterWithMovingOrdersIsStale(triage section 3, item 5): the 207537 sample repeats whileComputeDispatchwinds the battery, which follows one tick later. Battery orders stay within 3208..4075 W; stale at 34 s, before the 38 s EV rise. One changed value reopens the gate.TestFrozenMeterHoldLetsLiveMeterLoopSettle: a closed loop withComputeDispatch. With a 10 s meter polled every 2 s, the battery reaches the target without overshooting more than 1 kW into export; with a meter that sends a new reading every tick, it settles too. Never stale.TestFrozenMeterSlowPollerNeedsThreeRepeats: one answer every 20 s after a 3 kW move. Not stale after 1 or 2 repeats, stale after 3.TestFrozenMeterAcceptedOfferWaitsForTheCar: the rig's timing, offer accepted at 1 s, car draws at 32 s, noise-free live meter. Never stale.TestFrozenMeterQuietSiteStaysFresh,TestFrozenMeterTenSecondTelegramsDoNotFlap,TestFrozenMeterLostTelegramDoesNotTrip,TestFrozenMeterOrdersNotFollowedDoNotCount,TestFrozenMeterOppositeMovesCancel,TestFrozenMeterMoveAndBackDoesNotCount,TestFrozenMeterFirstOrderMatchingAutonomousBatteryDoesNotCount: never stale.TestFrozenMeterNewlyPluggedEVCounts,TestFrozenMeterFromStartIsCaught,TestFrozenMeterWaitsForTheMoveToLand(not stale at 29 s, stale at 30 s): stale.go/internal/telemetry/telemetry_test.go:TestOnlineMeterSampleSkipsOfflineMeter,TestOnlineRawWByTypeSkipsOfflineDrivers.go/cmd/ftw/site_meter_stale_test.go:TestSiteDispatchGateClosesOnFrozenMeterdrivessiteDispatchNowtosite_meter_stalebefore 38 s, checks that setup's gate with a candidate fuse gives the same verdict, and that a changed value reopens it.TestFrozenSiteMeterClosesDispatchGate: a frozen verdict does not replace an existing reason.Fail before, pass after, by undoing each change on its own:
TestFrozenMeterWithMovingOrdersIsStalefails (orders swing -5000..4075 W) andTestFrozenMeterHoldLetsLiveMeterLoopSettlefails (10 s meter overshoots to 2000 W export).meterFreezeSamples = 0:TestFrozenMeterSlowPollerNeedsThreeRepeatsfails.TestFrozenMeterAcceptedOfferWaitsForTheCarfails at 31 s.go test -race -count=1passes on./internal/control,./internal/telemetryand./cmd/ftw.make verifypasses..github/scripts/check-telemetry-reads.sh origin/master: "none added". TestSite/siterig was not run here; the runner does that.Risk
clearBatteryManualHoldForDispatchBlock).TestFrozenMeterAcceptedOfferWaitsForTheCar).Stack and merge order
Based on
master(merged up to 7840c33). Stands alone. It touchesgo/cmd/ftw/main.go(the freshness block in the tick and setup's control check),go/internal/control/dispatch.go(oneStatefield, one line where dispatch reads the meter),go/internal/telemetry/store.go(two accessors) andgo/cmd/ftw/site_dispatch_safety.go.Open PRs that touch the same files:
meter_freeze.goortelemetry/store.go.site_dispatch_safety.go, including the return line ofsiteDispatchNowthat this PR changes. Whichever lands second needs a small hand merge.main.goordispatch.go.Question for the owner
At a site with no battery, should an EV offer the charger accepted count as a move before the car draws it? Yes catches a frozen meter within 30 s of the first raise, but marks a live meter with no noise stale while the car wakes, which on the rig sends the chargers to 0 W. This PR answers no.
🤖 Generated with Claude Code