Damp latency-driven grid oscillation in active control (#473)#475
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis PR adds oscillation-damping capability to the CT002 power meter load balancer to suppress continuous hunting caused by laggy meter readings. A new residual-sign-reversal detection algorithm scales down computed residuals when the system repeatedly changes direction, while bypassing damping for genuinely large load steps. Configuration parameters control damping intensity, decay rate, and threshold. The feature is implemented across ESPHome (C++) and Python reference balancer, with full test coverage. ChangesOscillation Damping Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Steering evaluation (base vs head)Lower is better for every metric. See 📊 Interactive grid-power charts (zoom / hover / toggle series) are in the self-contained What do these metrics mean?
mixed_cadence/eff — settle 71.9→52.5s, overshoot 352.8→327.2W, RMS 25.0→21.5W
mixed_cadence/fair — settle 52.9→51.8s, overshoot 161.0→133.6W, RMS 15.1→15.2W
mixed_cadence_solar/eff — settle 58.3→93.4s, overshoot 289.2→293.9W, RMS 24.4→28.1W
mixed_cadence_solar/fair — settle 50.4→78.8s, overshoot 190.8→203.1W, RMS 19.4→23.9W
mixed_venus_b2500/eff — settle 130.6→104.7s, overshoot 262.0→258.0W, RMS 22.4→18.0W
mixed_venus_b2500/fair — settle 110.8→76.2s, overshoot 253.4→244.9W, RMS 23.2→19.0W
single_venus_solar — settle 12.0→11.8s, overshoot 45.6→28.5W, RMS 15.2→15.9W
single_venus_steps — settle 12.2→12.5s, overshoot 19.3→18.6W, RMS 14.4→14.5W
single_venus_washer — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 92.3→66.2W
two_venus/eff — settle 24.8→15.4s, overshoot 187.9→187.1W, RMS 15.8→14.5W
two_venus/fair — settle 15.3→15.1s, overshoot 182.3→182.3W, RMS 15.0→15.6W
two_venus_solar/eff — settle 28.1→18.4s, overshoot 202.9→205.6W, RMS 19.0→20.1W
two_venus_solar/fair — settle 46.9→31.1s, overshoot 189.3→185.8W, RMS 20.3→22.0W
📊 Open the interactive report — |
tomquist
force-pushed
the
claude/washer-oscillation-fix
branch
from
a3ab29b to
93ada34
Compare
Under a laggy meter the gain-1 grid-following residual limit-cycles: the controller keeps reacting to a stale reading that doesn't yet reflect its last command, so the grid hunts continuously instead of settling. Add oscillation-gated damping to the per-battery residual: an accumulating score tracks how often the residual reverses sign (the signature of hunting, not of a real load change) and bleeds the loop gain by up to osc_damp_max as it rises. Two guards keep genuine reactions at full speed: a magnitude threshold (a residual past osc_damp_threshold is a real demand step, never damped) and the accumulation itself (a one-off zero-crossing, e.g. a solar ramp, barely moves the score). Tuned on the steering-eval suite (max=0.8, alpha=0.15, decay=0.1, threshold=450). 3-seed means vs the undamped baseline: the washing-machine oscillation scenario's grid swing drops ~30% and its band-crossing rate ~40%, and several multi-battery scenarios also settle faster with less hunting and overshoot; the one notable trade is a small overshoot rise on the mixed-cadence fair-share scenario (slow + fast poller sharing one phase). Tune or disable with OSC_DAMP_MAX / OSC_DAMP_ALPHA / OSC_DAMP_DECAY / OSC_DAMP_THRESHOLD (osc_damp_* on ESPHome). Both balancer implementations (Python and the ESPHome C++ mirror) changed in lockstep; covered by new unit tests and the existing differential parity suite.
tomquist
force-pushed
the
claude/washer-oscillation-fix
branch
from
93ada34 to
0653acd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/test_balancer.py (1)
656-669: 💤 Low valueTest helper uses
osc_damp_alpha=0.25instead of production default0.15.The
_lbhelper setsosc_damp_alphato0.25, but the production default inBalancerConfigand the PR objectives both specify0.15. This appears intentional (the comment at line 705 explicitly calculates with alpha=0.25), likely to make damping effects more pronounced in tests. Consider adding a brief comment here explaining why the test uses a non-default alpha value to avoid confusion.📝 Suggested clarification
def _lb(self, **cfg): + # Use alpha=0.25 (higher than production 0.15) to make damping effects + # more visible in test assertions. cfg.setdefault("osc_damp_max", 0.8) cfg.setdefault("osc_damp_alpha", 0.25) cfg.setdefault("osc_damp_decay", 0.1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_balancer.py` around lines 656 - 669, The test helper _lb sets osc_damp_alpha to 0.25 which differs from the production/default 0.15; add a brief inline comment in the _lb function next to the cfg.setdefault("osc_damp_alpha", 0.25) line explaining that the higher alpha is intentional for tests (to amplify damping behavior and match the calculations in the nearby test that assume alpha=0.25), so future readers understand this deviation from BalancerConfig's default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 5: The new standalone bullet about "oscillation-damped" per-battery
corrections should not be added as a separate entry under the "## Next" section;
instead, edit the existing single summary bullet under the "## Next" heading in
CHANGELOG.md to incorporate this change (mentioning OSC_DAMP_MAX /
OSC_DAMP_ALPHA / OSC_DAMP_DECAY and the user-visible flags `osc_damp_*`) so
there remains only one summary entry. Locate the added bullet text (the
paragraph explaining continuous grid oscillation and the new options) and merge
its key outcome and knobs into the preexisting "## Next" summary sentence,
removing the extra bullet.
In `@src/astrameter/ct002/balancer.py`:
- Around line 344-348: The reset_consumer() implementations must clear the new
oscillation state: set osc_score = 0.0 and osc_last_sign = 0 (in addition to
existing pacing/saturation resets) so stale damping memory is not reused; update
both the Python reset_consumer() implementation (where osc_score and
osc_last_sign are defined/used) and the ESPHome reset_consumer() equivalent to
explicitly zero these two fields.
In `@src/astrameter/ct002/ct002.py`:
- Around line 182-185: The constructor now accepts osc_damp_threshold but the
instantiation in src/astrameter/main.py still only passes osc_damp_max,
osc_damp_alpha, and osc_damp_decay, so any configured threshold is ignored;
update the construction call in main.py to pass osc_damp_threshold (e.g.,
osc_damp_threshold=config.osc_damp_threshold or the local variable used for
other osc_* settings) as a keyword argument to match the CT002/constructor
signature, and keep a fallback to 450 if the config value is missing
(osc_damp_threshold=cfg.get('osc_damp_threshold', 450)).
In `@src/astrameter/main.py`:
- Around line 185-187: The CT002 config never reads or exposes
OSC_DAMP_THRESHOLD, so update the config reading in main.py alongside
osc_damp_max/osc_damp_alpha/osc_damp_decay by adding: read osc_damp_threshold
via cfg.getfloat(...) (fallback 450) and pass it into the CT002(...) constructor
as osc_damp_threshold=osc_damp_threshold (this ties into the BalancerConfig
behavior that bypasses damping when abs(residual) > osc_damp_threshold); also
update web_config.py's CT002 schema to include OSC_DAMP_THRESHOLD (matching the
pattern used for osc_damp_max/alpha/decay) so the value is editable from the web
UI.
In `@src/astrameter/web_config.py`:
- Around line 297-299: Add the missing OSC_DAMP_THRESHOLD entry to the CT002 web
config metadata block alongside the existing OSC_DAMP_MAX / OSC_DAMP_ALPHA /
OSC_DAMP_DECAY keys so the web editor will render and validate that knob; define
it as a float with a non-negative minimum (e.g. {"type":"float","min":0}) to
allow the runtime's 450 W default to be overridden via the web UI. Target the
same metadata dict where OSC_DAMP_MAX/OSC_DAMP_ALPHA/OSC_DAMP_DECAY are defined
and insert the OSC_DAMP_THRESHOLD key there.
---
Nitpick comments:
In `@tests/test_balancer.py`:
- Around line 656-669: The test helper _lb sets osc_damp_alpha to 0.25 which
differs from the production/default 0.15; add a brief inline comment in the _lb
function next to the cfg.setdefault("osc_damp_alpha", 0.25) line explaining that
the higher alpha is intentional for tests (to amplify damping behavior and match
the calculations in the nearby test that assume alpha=0.25), so future readers
understand this deviation from BalancerConfig's default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 936f8072-5129-4d26-b600-d07d89bbd3e2
📒 Files selected for processing (10)
CHANGELOG.mdesphome/components/ct002/__init__.pyesphome/components/ct002/balancer.cppesphome/components/ct002/balancer.hesphome/components/ct002/test_hooks.cppsrc/astrameter/ct002/balancer.pysrc/astrameter/ct002/ct002.pysrc/astrameter/main.pysrc/astrameter/web_config.pytests/test_balancer.py
- reset_consumer now zeroes osc_score / osc_last_sign (Python + ESPHome C++) so stale hunting memory isn't reused after a consumer returns to auto / resumes from inactive, matching the pacing/saturation resets beside it. - Wire OSC_DAMP_THRESHOLD through the config.ini path: main.py reads it (fallback 450) and passes it to CT002, and web_config.py exposes it next to OSC_DAMP_MAX/ALPHA/DECAY so it's editable from the web UI (it was already a CT002/BalancerConfig parameter but unreachable from config). - CHANGELOG: fold the oscillation-damping entry into the existing multi-battery "hunting" bullet (one summary bullet) and list the OSC_DAMP_* knobs. - test: note why the damping test helper uses alpha=0.25 vs the 0.15 default.
Add the oscillation-damping knobs (OSC_DAMP_MAX/ALPHA/DECAY/THRESHOLD, ESPHome osc_damp_*) to the CT_BALANCER group in web/ts/schema.ts so the config generator renders them alongside the other active-control tuning fields and emits them for both config.ini and the ESPHome balancer block. cd web && npm run check passes.
Contributor
|
This was referenced Jun 14, 2026
This was referenced Jun 24, 2026
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
2 participants
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.
Problem
When the meter the controller acts on lags reality (a Home Assistant push sensor with measurement delay, a slow P1 dongle), active control hunts continuously instead of settling — the grid never holds zero. The gain‑1 grid‑following residual keeps reacting to a stale reading that doesn't yet reflect its last command, so it overshoots and limit‑cycles. This is the field report in #473, reproduced by the
single_venus_washerevaluation scenario (added in #474).Fix
Oscillation‑gated damping on the per‑battery residual (
_damp_oscillation/damp_oscillation_, Python + ESPHome C++ in lockstep): an accumulating score tracks how often the residual reverses sign — the signature of hunting, not of a real load change — and bleeds the loop gain by up toosc_damp_maxas the score rises. Two guards keep genuine reactions at full speed:osc_damp_threshold(450 W) is a real demand step (kettle, solar ramp), never damped.A clean load/solar step holds one sign, so the score stays ~0 and reaction speed is unchanged. Defaults
osc_damp_max=0.8, osc_damp_alpha=0.15, osc_damp_decay=0.1, osc_damp_threshold=450, tuned on the steering‑eval suite; tune or disable viaOSC_DAMP_MAX/OSC_DAMP_ALPHA/OSC_DAMP_DECAY/OSC_DAMP_THRESHOLD(osc_damp_*on ESPHome).Results (steering‑eval, seed 1 — CI posts the full 3‑seed base‑vs‑head table)
Broad improvement across the matrix (settle, band‑crossings, grid swing, battery travel down; several overshoots down too). Honest trade‑offs: a few
…/fairsolar scenarios show a small settle/p2p rise (~+10–16%, e.g.mixed_cadence_solar/fairsettle 56→65) where the damper mildly slows a transition; these are minor and partly seed noise. Tuningalphatrades washer‑damping strength against this — 0.15 was chosen as the balance point (0.25 damps the washer harder but regressesmixed_cadence/fairovershoot).Tests
TestDampOscillationunit tests (steady step not damped, sustained hunt damped, large residual bypasses, single reversal barely damps, disable switch).ruff/mypy/pytestgreen.Parity: simulator/eval is Python‑only, but the balancer change is mirrored in
esphome/components/ct002/(config, codegen, test hooks) per the parity rule.Draft for review of the tuning trade‑off before finalizing.
https://claude.ai/code/session_01V6PWDgijfH7nmniL47A2qy
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Tests