Add MIN_DC_OUTPUT floor to keep B2500 inverter awake#431
Conversation
Some DC batteries (e.g. Marstek B2500) feed a separate inverter from their DC output. Under high PV surplus the balancer drives the target to 0 W, the inverter's DC input drops below its wake threshold, and it switches off and sleeps without recovering on its own. Add a configurable minimum net discharge floor: - Global MIN_DC_OUTPUT in [CT002]/[CT003] (default 0 = off), warned when set below the saturation min target. - Per-device override via MQTT Insights, surfaced as a Home Assistant "Min DC Output" number — only on batteries where it has an effect. The floor is applied at a single chokepoint in the auto path, holding an eligible battery at the floor whenever its commanded net output drops below it (recovering the consumer's full intended reading via the phase-split total), while leaving manual/inactive/weight-0 batteries untouched. Introduce a device-capabilities classifier (built-in inverter / AC input / DC input) as the single source of truth for device-type decisions. The floor applies only to batteries with no AC input and no built-in inverter (B2500 family); Venus and Jupiter are excluded. _is_ac_chargeable now derives from this classifier; unrecognized/empty types are treated as modern AC-coupled batteries. Mirrored 1:1 in the ESPHome C++ component (classifier, config, floor, and the per-device MQTT/discovery wiring) with parity-test coverage across device types.
|
Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine. 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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR introduces a Min DC Output keep-alive: global and per-consumer minimum DC discharge thresholds, a DeviceCapabilities-based device classification, enforcement in the load balancer (auto targets floored when applicable), propagation to snapshots/MQTT, conditional HA discovery/command handling, UI/generator/config wiring, and tests. ChangesMin DC Output Keep-Alive Feature
Sequence DiagramsequenceDiagram
participant Client as HA/MQTT Client
participant MqttSvc as MqttInsightsService
participant CT002 as CT002 Component
participant Balancer as LoadBalancer
Client->>MqttSvc: publish min_dc_output command
MqttSvc->>CT002: call set_consumer_min_dc_output(device_id, value)
CT002->>Balancer: include min_dc_output in ConsumerReport
Balancer->>Balancer: compute_auto_target -> apply_min_dc_output_ -> floored targets
Balancer-->>CT002: return phase targets
CT002->>MqttSvc: publish consumer state including min_dc_output
MqttSvc-->>Client: retained state message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)tests/components/ct002/fixtures/balancer_parity_harness.cpptests/components/ct002/fixtures/balancer_parity_harness.cpp:37:10: fatal error: 'esphome/components/ct002/balancer.h' file not found ... [truncated 1217 characters] ... internal-isystem" "/usr/local/include" "-internal-isystem" esphome/components/ct002/ct002.cppIn file included from esphome/components/ct002/ct002.cpp:1: ... [truncated 2200 characters] ... 6-141 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 |
|
MIN_DC_OUTPUT applies to each DC-only battery individually (including a single-battery setup) and is independent of multi-battery balancing. Move it out of the "Fair distribution / Multi-battery balancing" grouping in the docs and the web config generator into its own "DC battery keep-alive" section. No behavior change — the balancer already applies the floor per consumer. The ESPHome YAML key stays under the `balancer:` block.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/components/ct002/fixtures/balancer_parity_harness.cpp (1)
11-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
mdtruly optional intargetparsing (or make it explicitly required).The parser at Line 99 always reads
md, but the documented row format at Line 17 is still 4 fields. With multi-row inputs using 4-field rows, token alignment can shift and corrupt report parsing.Proposed fix (accept both 4-field and 5-field row formats deterministically)
@@ - } else if (cmd == "target") { + } else if (cmd == "target") { std::string cid, mode_str; float manual = 0.0f, grid = 0.0f; int n = 0; in >> cid >> mode_str >> manual >> grid >> n; + std::vector<std::string> tokens; + for (std::string tok; in >> tok;) tokens.push_back(tok); + const bool has_md = (tokens.size() == static_cast<size_t>(n) * 5U); + if (!has_md && tokens.size() != static_cast<size_t>(n) * 4U) { + continue; // malformed line + } ReportMap reports; - for (int i = 0; i < n; ++i) { - std::string rc, dev, phase; - float power = 0.0f, md = -1.0f; - in >> rc >> dev >> phase >> power >> md; + size_t idx = 0; + for (int i = 0; i < n; ++i) { + std::string rc = tokens[idx++]; + std::string dev = tokens[idx++]; + std::string phase = tokens[idx++]; + float power = std::stof(tokens[idx++]); + float md = -1.0f; + if (has_md) md = std::stof(tokens[idx++]); ConsumerReport r{dev, phase, power}; if (md >= 0.0f) r.min_dc_output = md; reports[rc] = r; }-// target <cid> <mode> <manual> <grid> <n> [<cid> <dev> <phase> <power>]xN +// target <cid> <mode> <manual> <grid> <n> +// [<cid> <dev> <phase> <power> [<md>]]xNAlso applies to: 96-103
🤖 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/components/ct002/fixtures/balancer_parity_harness.cpp` around lines 11 - 18, The target-row parser in balancer_parity_harness.cpp currently unconditionally reads the optional md token, which corrupts alignment when rows use the documented 4-field form; modify the parsing logic in the target handling code (the routine that tokenizes each target row and uses variables like md, mode, manual, grid, n and the subsequent per-device tuple parsing) to first count tokens for the row and deterministically branch: if token_count == 5 read md then shift subsequent reads, else if token_count == 4 set md to a default/empty value and parse the remaining fields normally; apply the same deterministic token-count check to the duplicate parsing block referenced in the nearby section (the other target-parsing lines) so both variants (4-field and 5-field) are supported without misalignment.
🤖 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 `@esphome/components/ct002/ct002.cpp`:
- Around line 655-660: The C++ setter CT002Component::set_consumer_min_dc_output
currently rejects values >1000 while the Python CT002 contract accepts any
finite value >= 0; update the validation in
CT002Component::set_consumer_min_dc_output to only reject non-finite or negative
values (remove the upper bound check against 1000) so behavior matches
src/astrameter/ct002/ct002.py, leaving assignment to
this->get_consumer_(consumer_id).min_dc_output unchanged.
In `@src/astrameter/main.py`:
- Around line 196-205: The code reads MIN_DC_OUTPUT with cfg.getint but
BalancerConfig.min_dc_output is a float and ESPHome accepts non-integer values,
so change the call to use cfg.getfloat (e.g., replace cfg.getint(...) with
cfg.getfloat(...)) and ensure the fallback is a float (0.0) so min_dc_output is
a float; keep the existing comparisons against min_target_for_saturation and the
logger.warning text (referencing min_dc_output, min_target_for_saturation, and
BalancerConfig.min_dc_output) unchanged.
---
Outside diff comments:
In `@tests/components/ct002/fixtures/balancer_parity_harness.cpp`:
- Around line 11-18: The target-row parser in balancer_parity_harness.cpp
currently unconditionally reads the optional md token, which corrupts alignment
when rows use the documented 4-field form; modify the parsing logic in the
target handling code (the routine that tokenizes each target row and uses
variables like md, mode, manual, grid, n and the subsequent per-device tuple
parsing) to first count tokens for the row and deterministically branch: if
token_count == 5 read md then shift subsequent reads, else if token_count == 4
set md to a default/empty value and parse the remaining fields normally; apply
the same deterministic token-count check to the duplicate parsing block
referenced in the nearby section (the other target-parsing lines) so both
variants (4-field and 5-field) are supported without misalignment.
🪄 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: 49c10205-c2d9-4819-8f65-db2ee9055444
📒 Files selected for processing (26)
CHANGELOG.mdREADME.mdconfig.ini.exampleesphome.example.yamlesphome/components/ct002/__init__.pyesphome/components/ct002/balancer.cppesphome/components/ct002/balancer.hesphome/components/ct002/ct002.cppesphome/components/ct002/ct002.hesphome/components/ct002/ha_discovery.cppesphome/components/ct002/mqtt_insights.cppesphome/components/ct002/test_hooks.cppsrc/astrameter/ct002/balancer.pysrc/astrameter/ct002/ct002.pysrc/astrameter/main.pysrc/astrameter/mqtt_insights/discovery.pysrc/astrameter/mqtt_insights/mqtt_insights_test.pysrc/astrameter/mqtt_insights/service.pytests/components/ct002/fixtures/balancer_parity_harness.cpptests/components/ct002/host_balancer_test.cpptests/components/ct002/test_balancer_parity.pytests/test_balancer_mixed_battery_charging.pytests/test_ct002_active_control.pyweb/ts/app.tsweb/ts/generate.tsweb/ts/schema.ts
- main.py: read MIN_DC_OUTPUT with getfloat (the field is a float and is exposed as float per-device/ESPHome) so e.g. 12.5 doesn't fail at startup; log with %g. - ct002.cpp: drop the >1000 upper bound in set_consumer_min_dc_output to match the Python setter (finite, >=0); the MQTT handler still enforces 0..1000 on both stacks. - balancer_parity_harness.cpp: update the stale command-format comment to document the optional global min_dc_output and the per-report <md> token.
2 participants
Summary
Implements a configurable minimum discharge floor (
MIN_DC_OUTPUT) to prevent DC batteries with external inverters (Marstek B2500 family) from sleeping at 0 W under high PV surplus. This solves issue #425 where a lone B2500 would deadlock the grid at full feed-in because its inverter would switch off.Key Changes
Device capabilities model: Replaces ad-hoc prefix checks with a unified
DeviceCapabilitiesdataclass that classifies batteries into three independent capabilities (built-in inverter, AC input, DC input). This is the single source of truth for all device-type decisions.AC-charge eligibility reclassified: Unknown/empty
device_typestrings are now assumed to be modern AC-coupled batteries (issue Feature Request: Mindest-Leistungslimit (Min Target) für Inverter bei hohem PV-Überschuss #425), intentionally dropping the former fail-closed-to-DC default (issue falsche Werte bei emulierten Smartmeter #338). This prevents deadlock when device types can't be identified.MIN_DC_OUTPUT floor: New global
min_dc_outputconfig option (default 0 = disabled) that holds B2500-family batteries at a minimum discharge to keep their external inverter awake. Also supports per-device overrides via MQTT.Floor eligibility: Only applies to batteries with no built-in inverter and no AC input (B2500 family:
HMA*,HMJ*,HMK*). Venus and Jupiter are unaffected because they have built-in inverters.Respects user intent: The floor does not override explicit user choices—batteries parked via
distribution_weight=0, manual mode, or inactive mode stay at 0.Home Assistant integration: New "Min DC Output" number entity in MQTT discovery, only shown for B2500-family batteries where it has an effect.
Implementation Details
src/astrameter/ct002/balancer.pyandesphome/components/ct002/balancer.cpp).MIN_DC_OUTPUTis set below the saturationmin_target, which would prevent saturation detection on floored units.https://claude.ai/code/session_01BE7uMJDKMJNRPZTquxuTyM
Summary by CodeRabbit
New Features
Bug Fixes
Documentation