Repository navigation
fix(inverter): name the entity at fault in the charge/discharge curve warnings (#4009) - #5646
Conversation
… warnings (#4009) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
springfall2008
left a comment
There was a problem hiding this comment.
Automated comment from the triage bot.
Reviewed ea6e0e0 (fix(inverter): name the entity at fault in the charge/discharge curve warnings). This PR changes log text only, inside the two warning branches of find_charge_curve() — the branch gates themselves (and the return value) are untouched. I checked the new history_checks/no_history/missing_settings logic, the new len(data[0]) if data else 0 record-count expression against both production history paths (HAHistory.get_history in ha.py:223 returns [records] or None, so data[0] is always a list when data is truthy), the new test's red/green claim against main's current wording (main's warning contains soc_percent, which the new test asserts against — it does fail without the fix), the fixture restore (args popped and restored in finally, and the get_history stub convention matches the rest of the module — each test installs its own stub), and docs/ for pages quoting the old warning. Two findings: 1 inline, 1 in the body below. No correctness bugs found; the approach of naming the entity at fault is the right one.
apps/predbat/inverter.py:1361-1364 — four local initialisers are now dead code
soc_kwh = [], charge_rate = [], battery_power = [] and predbat_status = [] are assigned but never read before the reassignments inside the all-history-present branch (the minute_data/minute_data_state calls at lines 1387/1404/1418/1432/1438). Their only reader was the removed all-zero "Sensor history data lengths" line; the new warning branches read the *_data fetch results instead. The PR body concedes they are "safe to delete" — deleting them is the cleaner diff, and it stops the next reader from assuming the parsed lists have a pre-branch meaning. Anchorable only in the body: these lines sit outside both hunks of the diff.
|
Automated reply from the triage bot (PR cleanup run). Not pushed. The quality gate failed, but the failing test is outside this PR. Both review items are fixed locally and ready to push once the gate can pass. Review items (fixed locally, not pushed)
Why it was not pushed:
A fix would build each day's local anchor from the date ( Next step: re-run cleanup on 26 Oct or later, or after a separate fix for the octopus anchor merges. |
|
Automated cleanup failed to complete for this PR. Reason: The run exited cleanly but left 3 file(s) with uncommitted changes, so its fixes were never pushed. They are stashed in the bot's clone before the next flow runs. Failed at: |
…-name-entity-4009
… new charge-curve warnings (#4009) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated reply from the triage bot (PR cleanup run). Pushed 7128762, which supersedes the "not pushed" note above. The branch now has Review items:
|
…-name-entity-4009
This is an automated draft PR generated from issue #4009. A maintainer should review it before merging.
Fixes #4009
Summary
The exact warning quoted in #4009 (all history present, but no full-rate cycle found) was already turned into an Info in v8.40.0 (6784a3b). Two
find_charge_curve()warnings on main still list every possible setting instead of the one that is wrong. This PR changes both:no history found for battery_power (sensor.battery_power) - check the entity name in apps.yaml and that Home Assistant is recording its history.0, 0, 0, 0in this branch. It read the parsed lists, which only get filled in the all-history-present branch. It now reports the record count of each fetched history (Sensor history record counts: soc_percent 576, charge_rate 576, battery_power 0, predbat.status 576).battery_power not set in apps.yaml).soc_kwis used instead ofsoc_percent, the label is nowsoc_kw(the real apps.yaml key) instead ofsoc_kwh.This changes log text only. The control flow and return value of
find_charge_curve()are unchanged.Testing
test_find_charge_curve_names_missing_entityinapps/predbat/tests/test_find_charge_curve.py. It removes the history ofbattery_poweronly (checking both the charge and discharge curves), then removes thebattery_powersetting. It asserts that each warning namesbattery_powerand none of the other entities, and that the record counts show 0 only forbattery_power.tools/triage_test.sh find_charge_curvewithinverter.pystashed failed with 5 errors (old wording, all-zero lengths). With the fix it passes, and it passes again after black reformatting../run_pre_commitpasses. Black re-wrapped one line ininverter.pyand the second run was clean.Notes
impact()onInverter.find_charge_curvereports HIGH risk only because it sits on the main update loop (1 direct caller, 3 processes). The change is limited to the two warning branches and does not change the signature or return value.detect_changes()shows no affected processes.soc_kwh = []/charge_rate = []/battery_power = []/predbat_status = []initialisers near the top offind_charge_curve()are now unused, because the fixed log line was the only thing that read them outside the branch that assigns them. I left them in to keep the diff small. They are safe to delete.[[]]) instead ofNone/[]for an entity, the history gate still treats it as present. That is existing behaviour and is out of scope here.🤖 Generated with Claude Code