Repository navigation
fix(inverter): pad create_missing_arg's sibling slots with None, not the default (#5359) - #5491
Draft
chalfontchubby wants to merge 3 commits into
Draft
chalfontchubby wants to merge 3 commits into
chalfontchubby wants to merge 3 commits into
Conversation
… own entry (#5359) get_current_charge_rate()/get_current_discharge_rate() picked the percent-vs-watts path from whether "charge_rate_percent"/"discharge_rate_percent" existed ANYWHERE in args, not from this inverter's own entry. On a mixed fleet where one inverter uses percent control and another uses watts, the watts-only inverter's unset percent slot quietly resolved to the 100% default, so it always read back battery_rate_max_raw regardless of what was last written - the #3311 fault for a script-driven inverter. adjust_charge_rate()/adjust_discharge_rate() had the same bug on the write side: both registers were written whenever either key existed anywhere, so the watts-only inverter tried to write its own (unset) percent entity every cycle, logging a spurious "No entity_id for ..." warning and pinning had_errors onto the status sensor. Also fixes create_missing_arg(): when a key was entirely absent from args, it filled all four per-inverter slots with the same default value rather than padding every slot but the caller's own with None (as the already-existing short-list branch does). On a mixed fleet that left a plain number in a sibling inverter's slot before that inverter was even constructed, which the per-entry fix above would otherwise misread as a configured entity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…very create_missing_arg list (#5359) The rate-path tests set the percent arg to a bare value, which is not how a mixed fleet reaches args; they now use a list with a configured sibling slot. The create_missing_arg test now checks every arg constructing a GS inverter builds, since the None padding applies to all of them, not just the rates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's rate_controls() (#5542) already chooses the rate control per inverter entry, so this branch's own read/write gates are dropped in favour of it, along with the two tests main's test_rate_control_is_per_inverter covers. What remains is the create_missing_arg() padding: without it a sibling's leftover default number still reads as a bound watts entity to rate_controls(), now proven in the test (GS beside a percent-only GEC). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chalfontchubby
changed the title
This branch has not been deployed
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
1 participant
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.
Draft PR written by Claude on Rik's behalf, for maintainer review.
Fixes #5359
Summary
#5542 (
rate_controls()) already fixed the main part of #5359: each inverter now picks its charge/discharge rate control from its own entry. This PR now fixes only the gap that remains, increate_missing_arg().When an inverter type lacks a control,
create_missing_arg()builds the per-inverter list for it. If the key was missing fromargsentirely, it filled all four slots with the default value, then replaced only the calling inverter's own slot with its dummy entity. Any other inverter's slot was left holding a bare number such as2600.0, andrate_controls()reads that as a bound watts entity.Concrete case, covered by the test: a Solis GS (current-controlled, so it builds
charge_rate) as inverter 0, next to a percent-only GE Cloud unit as inverter 1. On main, inverter 1'srate_controls("charge_rate")returns(True, True), so it writes a rate to "entity"2600.0as well as to its percent entity. With this change it returns(False, True).The fix pads every slot except the caller's with
None, which is what the short-list branch of the same function already does.Reach: this is not rate-specific.
create_missing_arg()builds the dummy-entity list for every argument an inverter type lacks (scheduled_charge_enable,scheduled_discharge_enable,reserve,charge_limit,charge_rate,discharge_rate,inverter_mode, the four charge/discharge start/end times and the two idle times). For all of them, a sibling inverter's slot now readsNoneand falls back to that inverter's own default, instead of carrying inverter 0's default value as if it were an entity id.Testing
test_create_missing_arg_does_not_fill_sibling_entries(intest_inverter.py):None;On main's
inverter.pythe test fails for every created argument (for examplecharge_ratesiblings[2600.0, 2600.0, 2600.0],charge_limit[100, 100, 100]), and inverter 1 reports(True, True). With the change it passes../run_pre_commit(all hooks plus the quick suite) passes.History: this PR originally also changed the rate read/write gates. Those changes and their two tests were dropped in the merge with main, because #5542's
rate_controls()andtest_rate_control_is_per_invertercover them.🤖 Generated with Claude Code