Skip to content

Don't collapse pure-DC pool on brief negative-grid transients (#359)#360

Merged
tomquist merged 3 commits into
developfrom
claude/investigate-issue-359-yFddw
May 15, 2026
Merged

Don't collapse pure-DC pool on brief negative-grid transients (#359)#360
tomquist merged 3 commits into
developfrom
claude/investigate-issue-359-yFddw

Conversation

@tomquist
Copy link
Copy Markdown
Owner

@tomquist tomquist commented May 15, 2026

The charge-blind/steer-to-zero path in _compute_auto_target exists to
keep a B2500 from stealing the surplus a Venus needs to wake up (see
issue #338). It fired on any grid_total < 0 reading, even when no
AC-chargeable battery was reporting — so a pure DC-only pool would
slam every inverter to 0 W on a brief negative-grid transient (load
drop, ramp overshoot) and then have to re-ramp from scratch.

Gate the whole charge-territory check on at least one AC-chargeable
battery actually being present. Pure-DC setups now ride the transient
through the regular fair-share path; the B2500s' own AC-charge clamp
still keeps them at 0 W under a sustained surplus, and the existing
"no AC-chargeable battery reporting" diagnostic notice is decoupled
from charge_blind so it still fires for the all-DC case.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved stability of DC-only battery systems during brief grid power fluctuations.
    • Enhanced handling of mixed DC/AC battery configurations to prevent unintended load shifting.
  • Tests

    • Added regression tests for DC-only battery pool stability scenarios.

Review Change Stack

claude added 2 commits May 15, 2026 12:23
The charge-blind/steer-to-zero path in `_compute_auto_target` exists to
keep a B2500 from stealing the surplus a Venus needs to wake up (see
issue #338). It fired on any `grid_total < 0` reading, even when no
AC-chargeable battery was reporting — so a pure DC-only pool would
slam every inverter to 0 W on a brief negative-grid transient (load
drop, ramp overshoot) and then have to re-ramp from scratch.

Gate the whole charge-territory check on at least one AC-chargeable
battery actually being present. Pure-DC setups now ride the transient
through the regular fair-share path; the B2500s' own AC-charge clamp
still keeps them at 0 W under a sustained surplus, and the existing
"no AC-chargeable battery reporting" diagnostic notice is decoupled
from `charge_blind` so it still fires for the all-DC case.
Adds a balancer-only assertion to complement the existing closed-loop
all-DC-surplus test.  Previously ``_steer_to_zero`` guaranteed every
DC target was 0 under surplus; with the #359 fix that path falls
through to fair-share, so the safety property now rests on the
``grid_total < 0 and target > 0`` sign-clamp plus fair-share producing
a non-positive share of a negative grid.  Pin the batteries at 0 W and
verify the emitted target stays ``<= 0`` every tick.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b42413c8-9008-40ee-bfa1-d672c7d6ef0f

📥 Commits

Reviewing files that changed from the base of the PR and between a9abeae and 8c79284.

📒 Files selected for processing (2)
  • src/astrameter/ct002/balancer.py
  • tests/test_balancer_mixed_battery_charging.py

Walkthrough

LoadBalancer's _compute_auto_target method is modified to compute AC-chargeable battery presence separately from charging state, gating charge territory logic on actual presence rather than charging activity. The all-DC degenerate case now explicitly checks for absence of AC-chargeable batteries. Two new regression tests validate that DC-only pools remain stable during brief negative-grid transients and never receive discharge commands under sustained surplus.

Changes

DC-only pool transient protection

Layer / File(s) Summary
Charge territory gating on AC-chargeable presence
src/astrameter/ct002/balancer.py
_compute_auto_target now computes any_ac_chargeable separately from ac_charging and gates charge territory logic on battery presence. All-DC-under-surplus degenerate case explicitly checks not any_ac_chargeable to avoid suppressing DC-only pools during brief negative-grid transients.
Transient and sustained surplus regression tests
tests/test_balancer_mixed_battery_charging.py
Module docstring updated with issue #359 coverage. Two new closed-loop tests: one simulating transient grid-total sign change asserting DC-only batteries settle to nonzero discharge, another verifying sustained surplus never produces positive target deltas for DC-only batteries at 0W.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • tomquist/AstraMeter#359: This PR directly implements the fix for issue #359 by adding any_ac_chargeable presence detection and gating in_charge_territory on it in LoadBalancer._compute_auto_target to prevent DC-only pool suppression during brief transients.

Possibly related PRs

  • tomquist/AstraMeter#342: Both PRs modify LoadBalancer._compute_auto_target and degenerate-case handling for all-DC scenarios using AC-chargeable detection; this PR's adjusted gating and not any_ac_chargeable logic directly builds on the foundation introduced in #342.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: preventing pure-DC battery pools from collapsing to zero during brief negative-grid transients, directly addressing issue #359.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/investigate-issue-359-yFddw

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tomquist tomquist marked this pull request as ready for review May 15, 2026 12:32
@tomquist tomquist merged commit 8ff7c8d into develop May 15, 2026
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants