Skip to content

fix(inverter): name the entity at fault in the charge/discharge curve warnings (#4009) - #5646

Merged
springfall2008 merged 4 commits into
mainfrom
fix/charge-curve-warn-name-entity-4009
Oct 24, 2026
Merged

springfall2008 merged 4 commits into
mainfrom
fix/charge-curve-warn-name-entity-4009

Conversation

@springfall2008

Copy link
Copy Markdown
Owner

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:

  • Missing history: the warning now names only the setting(s) and entity whose history fetch came back empty. For example: no history found for battery_power (sensor.battery_power) - check the entity name in apps.yaml and that Home Assistant is recording its history.
  • The companion "Sensor history data lengths" line was always 0, 0, 0, 0 in 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).
  • Settings missing: the warning now lists only the apps.yaml settings that are actually unset (battery_power not set in apps.yaml).
  • When soc_kw is used instead of soc_percent, the label is now soc_kw (the real apps.yaml key) instead of soc_kwh.

This changes log text only. The control flow and return value of find_charge_curve() are unchanged.

Testing

  • New test_find_charge_curve_names_missing_entity in apps/predbat/tests/test_find_charge_curve.py. It removes the history of battery_power only (checking both the charge and discharge curves), then removes the battery_power setting. It asserts that each warning names battery_power and none of the other entities, and that the record counts show 0 only for battery_power.
  • The test fails without the fix and passes with it: tools/triage_test.sh find_charge_curve with inverter.py stashed failed with 5 errors (old wording, all-zero lengths). With the fix it passes, and it passes again after black reformatting.
  • ./run_pre_commit passes. Black re-wrapped one line in inverter.py and the second run was clean.

Notes

  • impact() on Inverter.find_charge_curve reports 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.
  • The soc_kwh = [] / charge_rate = [] / battery_power = [] / predbat_status = [] initialisers near the top of find_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.
  • If HA returns an empty inner list ([[]]) instead of None/[] 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

… warnings (#4009)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@springfall2008 springfall2008 self-assigned this Oct 19, 2026
@springfall2008 springfall2008 added BOT_REVIEW Trigger an autotriage BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push labels Oct 19, 2026

@springfall2008 springfall2008 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread apps/predbat/inverter.py
@springfall2008 springfall2008 removed the BOT_REVIEW Trigger an autotriage label Oct 19, 2026
@springfall2008

Copy link
Copy Markdown
Owner Author

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)

  • Review body, inverter.py:1361-1364: I confirmed the four initialisers are dead. Every read of soc_kwh / charge_rate / battery_power / predbat_status happens inside the all-history-present branch, after that branch reassigns them. Deleted.
  • Inline, docs/faq.md: the "Cannot find battery charge curve" section quoted a message that no longer exists. It now quotes both new forms, (settings missing), <settings> not set in apps.yaml and (missing history), no history found for <setting> (<entity_id>) ..., plus the record-counts line, and explains each cause. The heading is now Warn: to match the log level, and the one link to its anchor (docs/inverter-setup.md:2715) is updated to #warn-cannot-find-battery-charge-curve.

tools/triage_test.sh find_charge_curve passes with these changes.

Why it was not pushed: ./run_pre_commit fails in octopus_url / day_night_rates, and main fails the same way today

apps/predbat/octopus.py and apps/predbat/tests/test_octopus_url.py are identical on this branch and on main. The test fails with Expected 16 rate entries (8 days), got 15 (Tests 1 and 7, the ones that don't pin now_utc_exact). I worked out the cause by reading the code; I could not run a standalone probe in this session:

  • async_get_day_night_rates builds local_base = local_midnight + timedelta(days=day) for day in range(-2, 7), then calls normalize(...).replace(hour=0, ...).
  • With pytz Europe/London and local date 20 Oct 2026, day 6 is 26 Oct 00:00+01:00. normalize() turns that into 25 Oct 23:00+00:00, and .replace(hour=0) turns it into 25 Oct 00:00+00:00. That is the same date as day 5.
  • The two 25 Oct night windows overlap, so no day gap is added between them, and the schedule comes out one entry short (15 instead of 16).
  • This happens on every run while the local date is between 20 and 25 Oct, because day 6 crosses the BST→GMT change on 25 Oct. Yesterday's CI run (local date 19 Oct) passed for that reason. It should clear by itself from 26 Oct. Until then, any PR's pre-commit CI that runs the quick suite will hit it, main included.
  • The production effect looks small. The day before the change is anchored twice, and the final day of the 8-day schedule (outside the 48 h plan horizon) is dropped. The test's count assertion is what breaks.

A fix would build each day's local anchor from the date (local_tz.localize(datetime.combine(local_midnight.date() + timedelta(days=day), time()))) instead of normalising a shifted datetime. It belongs in its own PR, not this one.

Next step: re-run cleanup on 26 Oct or later, or after a separate fix for the octopus anchor merges.

@springfall2008

Copy link
Copy Markdown
Owner Author

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: triage_daemon.py:2161 in process_bot_cleanup_pr() (daemon 063549db). Not retrying automatically; remove BOT_FAILED and re-add BOT_CLEANUP to try again.

@springfall2008 springfall2008 added BOT_FAILED An automated triage/PR attempt failed; see the comment for details and removed BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push labels Oct 19, 2026
@chalfontchubby chalfontchubby added BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push and removed BOT_FAILED An automated triage/PR attempt failed; see the comment for details labels Oct 20, 2026
CI and others added 2 commits October 20, 2026 10:18
… new charge-curve warnings (#4009)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@springfall2008

Copy link
Copy Markdown
Owner Author

Automated reply from the triage bot (PR cleanup run).

Pushed 7128762, which supersedes the "not pushed" note above. The branch now has origin/main merged in, which includes #5655. The octopus_url / day_night_rates failure that blocked the previous run was a clock-change problem, and #5655 fixed it, so the quality gate now passes: ./run_pre_commit (every hook plus the quick suite) and tools/triage_test.sh find_charge_curve.

Review items:

  • Review body, inverter.py:1361-1364: the four initialisers soc_kwh / charge_rate / battery_power / predbat_status were dead, so I removed them. Every read of them comes after the reassignment inside the branch where all the history is present.
  • Inline, docs/faq.md: updated to the new warning wording. Details are in the thread reply.

@springfall2008 springfall2008 removed the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Oct 20, 2026
@chalfontchubby
chalfontchubby marked this pull request as ready for review October 22, 2026 08:19
@springfall2008
springfall2008 merged commit 123aa4d into main Oct 24, 2026
3 checks passed
@springfall2008
springfall2008 deleted the fix/charge-curve-warn-name-entity-4009 branch October 24, 2026 10:49
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.

Make "Cannot find battery discharge curve"/"Cannot find battery charge curve" warnings more specific

2 participants