Repository navigation
fix(easee_cloud): two review fixes to the 7 A start (1.3.8) - #191
Merged
Merged
Conversation
Findings L1 and L2 from the review of #190. L1: before the first poll last_op_mode is nil, so a reloaded driver offered 7 A to a car that may already charge at 6 A. A boost now needs a poll that saw the charger not charging. L2: a pause or the stand-down cleared the boost but left 7 A in last_amps_set, so the phaseMode re-assert on the next poll wrote 7 A without the cap or the 90 s window. Clearing a running boost now sets last_amps_set to what the charger should hold: the 6 A order after a pause, 0 A after the stand-down. A pause keeps 6 A, not 0 A, because Core's contactor cycle sends ev_resume after ev_pause with no new order between. Harness cases 15-17 fail on 1.3.7 and pass here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.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.
Fixes two low findings from the independent review of #190 (easee_cloud 1.3.7, the 7 A start for a 6 A order).
L1: no boost before the first poll
last_op_modeis nil until the first poll. After a driver reload with the car already charging at 6 A, the next 6 A order wrote 7 A until a poll saw op_mode 3. A boost now needs a successful poll that saw the charger not charging. A failed poll does not count. The cost is at most one control tick: if Core's first order comes before the first poll, that order gets 6 A and the next one gets 7 A.L2: a cleared boost no longer leaves 7 A to re-assert
ev_pauseanddriver_default_modecleared the boost but leftlast_amps_setat 7. When the boosting order also changedphaseMode, the re-assert on the next poll wrote 7 A without the cap check or the 90 s window. A newclear_start_boost(hold_a)clears the boost and, when one was running, setslast_amps_setto what the charger should hold:A pause keeps 6 A rather than 0 A because Core's contactor cycle (
cycleWallboxingo/internal/loadpoint/controller.go) sendsev_resume3 s afterev_pausewith no new order between. With 0 A, the resume would start into 0 A until the next periodic order.The boost's other ends already set
last_amps_setto 6 A (end_start_boost, and a 6 A order that ends the boost), or to 0 A (a 0 A order).Tests
drivers/tests/lua_harness/test_easee_cloud_start_boost.lua(lua55). Each fails on 1.3.7 (boost before the first poll,re-assert after a pause wrote the cleared boost's 7 A,re-assert after default mode wrote the cleared boost's 7 A) and passes here.make check: 4984 passed, 936 skipped.Version 1.3.8 through
make bump-driver(manifest, index, devices, support status) and a CHANGELOG entry.After merge
FTW's pin (
drivers/BUNDLED_SOURCE.json, now 5cc7bb1) has neither #190 nor this. Once this merges, a paired FTW PR moves the pin to the newmaincommit so the next beta carries 1.3.8 with Core'scap_amps_per_phase(srcfl/ftw#1809).🤖 Generated with Claude Code