Repository navigation
fix(loadpoint): mark a one-phase car only from an order rise - #1894
Merged
Merged
Conversation
The steady check had no baseline. A 3 kW load on L1 and solar on L2/L3 read [17, 4.5, 4.5] A next to a three-phase car at 6 A, which it took for a one-phase car. The order then tripled from 4140 W to 11040 W and swung back on the next rise test. Delete the steady check and keep the rise test, which compares against the meter before the rise. 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. |
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
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.
Item D5 of the stable plan.
Problem
The steady one-phase check (
steadyVerdict, one_phase_car.go:119-136) had no baseline. It marked a car one-phase when one phase carried the charger's whole draw and the other two carried less than a third.Take a three-phase car at 6 A (4140 W, or 18 A over one phase's voltage), a 3 kW oven on L1 and some solar on every phase. The meter reads [17, 4.5, 4.5] A, and the check marks the car one-phase within 60 s.
computeCommandForBudgetthen triples the order (controller.go:2647-2651). A planned 4140 W slot went out as 11040 W. In a closed loop, with a house load of [12, -1.5, -1.5] A, the mark set and cleared 12 times in 15 minutes. 84 of 181 orders went above 6 kW on a 4140 W plan.Change
steadyVerdict,observeSteadyPhaseand their state. A car is now marked one-phase only by the rise test, which measures from the meter just before FTW's own order rose.TestOnePhaseCarMarkedAfterARestartbecomesTestOnePhaseCarMarkedOnTheRiseAfterARestart. No car is marked without a rise, and a one-phase car is marked on the first rise after the restart.Tests
one_phase_house_load_test.go:TestHouseLoadDoesNotMarkAThreePhaseCar,TestHouseLoadKeepsAPlannedThreePhaseOrderandTestHouseLoadDoesNotSwingAThreePhaseOrder. All three fail on master with the numbers above and pass on this branch. The rewritten restart test also fails on master.go test -race -count=1 ./internal/loadpoint/... ./internal/api/... ./internal/control/... ./cmd/ftw/... ./internal/mpc/...: all ok.go vet ./...andgo build ./...: clean.Risk
Low. The change deletes code and never raises an order. #1865 also touches one_phase_car.go, but only
chargerPhaseMaxA, so the two should not conflict.🤖 Generated with Claude Code