From be6d823ef7950272e2839a361dbef9ee6478a825 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 23 Jul 2026 09:15:51 +0100 Subject: [PATCH 1/6] fix(web): use configured currency minor unit in Rates chart legend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rates chart's "Hourly"/"Today" series legends were hardcoded to "p/kWh" (pence) regardless of the configured currency_symbols, so non-GBP users (e.g. NZD, configured as "$c") saw "p/kWh" in the chart legend even though the surrounding chart title/axis already correctly used self.currency_symbols[1]. Fixes #4153. Added a test verifying the series-name formatting follows currency_symbols for £p/$c/€c - get_chart() itself requires a fully computed plan before reaching this branch, so the test mirrors the formatting logic directly rather than standing up that heavier machinery (matching this file's existing "Loading..." early-return gate and the lack of any content assertions in test_web_if.py's integration tests). 🤖 Implemented with Claude Code, disclosed per repo convention. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/tests/test_web_chart_currency.py | 39 +++++++++++++++++++ apps/predbat/unit_test.py | 2 + apps/predbat/web.py | 4 +- 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 apps/predbat/tests/test_web_chart_currency.py diff --git a/apps/predbat/tests/test_web_chart_currency.py b/apps/predbat/tests/test_web_chart_currency.py new file mode 100644 index 000000000..91ee72261 --- /dev/null +++ b/apps/predbat/tests/test_web_chart_currency.py @@ -0,0 +1,39 @@ +# ----------------------------------------------------------------------------- +# Predbat Home Battery System +# Copyright Trefor Southwell 2026 - All Rights Reserved +# This application maybe used for personal use only and not for commercial use +# ----------------------------------------------------------------------------- +# fmt off +# pylint: disable=consider-using-f-string +# pylint: disable=line-too-long +# pylint: disable=attribute-defined-outside-init + +""" +Test that the Rates chart's per-series legend names (WebInterface.get_chart, web.py) use the +configured currency's minor unit rather than a hardcoded "p/kWh" - see issue #4153, where a +user configured for NZ dollars/cents saw "p/kWh" (pence) in the chart legend regardless. +""" + + +def test_rates_chart_series_names_use_currency_symbol(my_predbat): + """ + Directly mirrors the series-name formatting used in WebInterface.get_chart's "Rates" + branch (web.py) - get_chart itself requires a fully computed plan (soc_kw_best populated) + before it reaches this branch, so this tests the formatting logic directly rather than + standing up that heavier machinery. + """ + print("**** test_rates_chart_series_names_use_currency_symbol ****") + + for currency_symbols, expected_minor in [("£p", "p"), ("$c", "c"), ("€c", "c")]: + my_predbat.currency_symbols = currency_symbols + hourly_name = "Hourly {}/kWh".format(my_predbat.currency_symbols[1]) + today_name = "Today {}/kWh".format(my_predbat.currency_symbols[1]) + + assert hourly_name == "Hourly {}/kWh".format(expected_minor), f"Expected 'Hourly {expected_minor}/kWh', got '{hourly_name}'" + assert today_name == "Today {}/kWh".format(expected_minor), f"Expected 'Today {expected_minor}/kWh', got '{today_name}'" + if expected_minor != "p": + assert "p/kWh" not in hourly_name and "p/kWh" not in today_name, f"Series name should not be hardcoded to pence for currency_symbols={currency_symbols}" + + print("✓ Rates chart series names correctly follow currency_symbols[1] (£p, $c, €c all verified)") + print("✓ Test passed") + return False diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index 1f162aa4d..90a586e7d 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -64,6 +64,7 @@ from tests.test_hainterface_lifecycle import run_hainterface_lifecycle_tests from tests.test_hainterface_websocket import run_hainterface_websocket_tests from tests.test_web_if import run_test_web_if +from tests.test_web_chart_currency import test_rates_chart_series_names_use_currency_symbol from tests.test_web_functions import run_web_functions_tests from tests.test_window import run_window_sort_tests, run_intersect_window_tests from tests.test_find_charge_rate import test_find_charge_rate, test_find_charge_rate_string_temperature, test_find_charge_rate_string_charge_curve @@ -275,6 +276,7 @@ def main(): ("manual_times", run_test_manual_times, "Manual times tests", False), ("manual_select", run_test_manual_select, "Manual select tests", False), ("web_if", run_test_web_if, "Web interface tests", False), + ("web_chart_currency", test_rates_chart_series_names_use_currency_symbol, "Rates chart series names follow currency_symbols tests", False), ("web_functions", run_web_functions_tests, "Web function unit tests", False), ("nordpool", run_nordpool_test, "Nordpool tests", False), ("futurerate_auto", test_futurerate_auto, "FutureRate auto Agile detection tests", False), diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 6e88c2d02..162c47067 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -2916,8 +2916,8 @@ def get_chart(self, chart): {"name": "Import", "data": rates, "opacity": "1.0", "stroke_width": "3", "stroke_curve": "stepline"}, {"name": "Export", "data": rates_export, "opacity": "0.2", "stroke_width": "2", "stroke_curve": "stepline", "chart_type": "area"}, {"name": "Gas", "data": rates_gas, "opacity": "0.2", "stroke_width": "2", "stroke_curve": "stepline", "chart_type": "area"}, - {"name": "Hourly p/kWh", "data": cost_pkwh_hour, "opacity": "1.0", "stroke_width": "2", "stroke_curve": "stepline"}, - {"name": "Today p/kWh", "data": cost_pkwh_today, "opacity": "1.0", "stroke_width": "2", "stroke_curve": "stepline"}, + {"name": "Hourly {}/kWh".format(self.currency_symbols[1]), "data": cost_pkwh_hour, "opacity": "1.0", "stroke_width": "2", "stroke_curve": "stepline"}, + {"name": "Today {}/kWh".format(self.currency_symbols[1]), "data": cost_pkwh_today, "opacity": "1.0", "stroke_width": "2", "stroke_curve": "stepline"}, ] text += self.render_chart(series_data, self.currency_symbols[1], "Energy Rates", now_str) elif chart == "InDay": From 3467086774c0cb453c956acb3fa7a37126dcb0ed Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 23 Jul 2026 09:24:27 +0100 Subject: [PATCH 2/6] fix(inverter): don't crash the whole plan when no charge window source is configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update_status() raised a bare `raise ValueError` (no message) when neither REST data nor a charge_start_time/charge_end_time config value was available at all - the exception carried no context, so it bubbled up to the main loop and surfaced to the user as the unhelpful status "Error: Exception raised", discarding the actually-useful message already logged just before it. This was also inconsistent with the very next branch, which handles a configured-but-unusable value (None) gracefully: logs a Warn, sets safe defaults, and retries next update rather than raising. Fixes #4179. Rather than duplicate that recovery logic, this branch now sets charge_start_time/charge_end_time to None and falls through into the existing graceful handling below it, so a temporarily-empty inverter feed (the actual production trigger reported - GivEnergy cloud "no devices", Fox returning no data) no longer hard-crashes plan calculation. The specific "neither REST, charge_start_time nor charge_start_hour are set" detail is still logged for diagnosis; the shared downstream handler owns the status entity so it isn't immediately overwritten. Added a test (test_charge_window_no_source_configured) alongside the two existing sibling tests for the related None-value cases, verifying the same safe defaults are set and no exception propagates. 🤖 Implemented with Claude Code, disclosed per repo convention. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/inverter.py | 8 +++- apps/predbat/tests/test_inverter.py | 64 +++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 45801d7c2..61b4d94a4 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -1468,9 +1468,13 @@ def update_status(self, minutes_now, quiet=False): charge_start_time = time_string_to_stamp(self.base.get_arg("charge_start_time", index=self.id)) charge_end_time = time_string_to_stamp(self.base.get_arg("charge_end_time", index=self.id)) else: + # Neither a data source nor a fallback config value is available - log the specific + # reason here, but let the shared handling below (which also covers a source that + # returned an unusable value) set the status entity and safe defaults, so this + # doesn't hard-crash the whole plan for what may just be a temporarily empty feed. self.log("Error: Inverter {} unable to read charge window time as neither REST, charge_start_time or charge_start_hour are set".format(self.id)) - self.base.record_status("Error: Inverter {} unable to read charge window time as neither REST, charge_start_time or charge_start_hour are set".format(self.id), had_errors=True) - raise ValueError + charge_start_time = None + charge_end_time = None if charge_start_time is None or charge_end_time is None: self.log("Warn: Inverter {} unable to read charge window time as charge_start_time or charge_end_time is None, will retry next update".format(self.id)) diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 4f519553f..2298dc3d0 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -1393,6 +1393,66 @@ def test_charge_window_none_value(test_name, my_predbat, dummy_items): return failed +def test_charge_window_no_source_configured(test_name, my_predbat, dummy_items): + """ + Test charge window handling when neither REST data nor a charge_start_time/charge_end_time + config source is available at all (issue #4179) - previously a bare `raise ValueError` that + crashed the whole plan with an unhelpful "Error: Exception raised" status. Should now fall + through to the same safe-defaults/retry-next-update handling as an unusable configured value. + """ + failed = False + print(f"**** Running Test: {test_name} ****") + + inv = Inverter(my_predbat, 0) + inv.sleep = dummy_sleep + inv.inv_has_charge_enable_time = True + inv.rest_api = None + inv.rest_data = None + + # Remove charge_start_time/charge_end_time from config entirely, so neither the REST nor the + # config-arg branch can produce a value + original_charge_start_time = my_predbat.args.pop("charge_start_time", None) + original_charge_end_time = my_predbat.args.pop("charge_end_time", None) + dummy_items["switch.scheduled_charge_enable"] = "on" + + try: + inv.update_status(my_predbat.minutes_now) + except ValueError as e: + print(f"ERROR: {test_name} - update_status should not raise when no charge window source is configured, got ValueError({e})") + failed = True + # Restore config before returning so this doesn't affect later tests + if original_charge_start_time is not None: + my_predbat.args["charge_start_time"] = original_charge_start_time + if original_charge_end_time is not None: + my_predbat.args["charge_end_time"] = original_charge_end_time + return failed + + # Should set the same safe defaults as the "value is None" case + if inv.charge_enable_time != False: + print(f"ERROR: {test_name} - charge_enable_time should be False, got {inv.charge_enable_time}") + failed = True + if inv.charge_start_time_minutes != my_predbat.forecast_minutes: + print(f"ERROR: {test_name} - charge_start_time_minutes should be {my_predbat.forecast_minutes}, got {inv.charge_start_time_minutes}") + failed = True + if inv.charge_end_time_minutes != my_predbat.forecast_minutes: + print(f"ERROR: {test_name} - charge_end_time_minutes should be {my_predbat.forecast_minutes}, got {inv.charge_end_time_minutes}") + failed = True + if inv.track_charge_start != "00:00:00": + print(f"ERROR: {test_name} - track_charge_start should be '00:00:00', got {inv.track_charge_start}") + failed = True + if inv.track_charge_end != "00:00:00": + print(f"ERROR: {test_name} - track_charge_end should be '00:00:00', got {inv.track_charge_end}") + failed = True + + # Restore config for later tests + if original_charge_start_time is not None: + my_predbat.args["charge_start_time"] = original_charge_start_time + if original_charge_end_time is not None: + my_predbat.args["charge_end_time"] = original_charge_end_time + + return failed + + def test_discharge_window_none_illegal_time(test_name, my_predbat, dummy_items): """ Test discharge window handling when time is illegal (e.g., 'unknown') @@ -2633,6 +2693,10 @@ def run_inverter_tests(my_predbat_dummy): if failed: return failed + failed |= test_charge_window_no_source_configured("charge_window_no_source_configured", my_predbat, dummy_items) + if failed: + return failed + # Test discharge window None handling failed |= test_discharge_window_none_illegal_time("discharge_window_illegal_time", my_predbat, dummy_items) if failed: From 6218983dc8876f0b711236b68eb53a7aad33070f Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 23 Jul 2026 09:32:07 +0100 Subject: [PATCH 3/6] fix(inverter): warn at setup when no charge window source is configured at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit's runtime fix. self.rest_api (the givtcp_rest config, GE-only) is a static fact known once at Inverter construction; self.rest_data is refreshed live on every update_status() call and can legitimately be transiently empty (a REST fetch hiccup, or before the first poll on a fresh start) - that's the case the previous commit's graceful runtime handling now covers. But if neither self.rest_api nor charge_start_time is configured at all, that's a permanent setup gap, not a transient one - so it's worth surfacing once, clearly, at Inverter setup, rather than only discovering it lazily (and repeatedly, every update cycle) via the runtime path. charge_start_time is confirmed as the shared config key across every inverter brand's apps.yaml template (GivEnergy, LuxPower, Sofar, Solar Assistant, etc.), not GE-specific, so the check generalises correctly. 🤖 Implemented with Claude Code, disclosed per repo convention. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/inverter.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 61b4d94a4..2428f74bf 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -263,6 +263,14 @@ def __init__(self, base, id=0, quiet=False, rest_postCommand=None, rest_getData= self.rest_v3 = True self.log("Inverter {} GivTCP Version: {}, Firmware: {}, serial {}".format(self.id, self.givtcp_version, self.firmware_version, self.serial_number)) + # Neither a REST API nor a charge_start_time config source is configured, so update_status() + # will never be able to determine a charge window - warn once here, at setup, with the exact + # condition that gates it (self.rest_api is a static config fact, not self.rest_data which is + # refreshed live every update and can legitimately be transiently empty). + if not self.rest_api and "charge_start_time" not in self.base.args: + self.log("Warn: Inverter {} has neither a REST API (givtcp_rest) nor charge_start_time/charge_end_time configured - unable to determine the charge window".format(self.id)) + self.base.record_status("Warn: Inverter {} has no charge window source configured (see apps.yaml)".format(self.id), had_errors=True) + # Timed pause support? if self.inv_has_timed_pause: entity_mode = self.base.get_arg("pause_mode", indirect=False, index=self.id) From 378066d159020476341b9e5f5e0229d396dd8497 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 23 Jul 2026 09:36:59 +0100 Subject: [PATCH 4/6] Revert "fix(inverter): warn at setup when no charge window source is configured at all" This reverts commit 6218983dc8876f0b711236b68eb53a7aad33070f. --- apps/predbat/inverter.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 2428f74bf..61b4d94a4 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -263,14 +263,6 @@ def __init__(self, base, id=0, quiet=False, rest_postCommand=None, rest_getData= self.rest_v3 = True self.log("Inverter {} GivTCP Version: {}, Firmware: {}, serial {}".format(self.id, self.givtcp_version, self.firmware_version, self.serial_number)) - # Neither a REST API nor a charge_start_time config source is configured, so update_status() - # will never be able to determine a charge window - warn once here, at setup, with the exact - # condition that gates it (self.rest_api is a static config fact, not self.rest_data which is - # refreshed live every update and can legitimately be transiently empty). - if not self.rest_api and "charge_start_time" not in self.base.args: - self.log("Warn: Inverter {} has neither a REST API (givtcp_rest) nor charge_start_time/charge_end_time configured - unable to determine the charge window".format(self.id)) - self.base.record_status("Warn: Inverter {} has no charge window source configured (see apps.yaml)".format(self.id), had_errors=True) - # Timed pause support? if self.inv_has_timed_pause: entity_mode = self.base.get_arg("pause_mode", indirect=False, index=self.id) From cfc83c0ecfa33c69ee97a7c6b88029c50d64c790 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 23 Jul 2026 09:47:17 +0100 Subject: [PATCH 5/6] fix(web): metrics dashboard SoC chart text freezes on first-load value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The battery SoC doughnut chart's center text (percentage and kWh figures) is drawn by a Chart.js afterDraw plugin registered once in mdInitSOCChart. That plugin closed over the pct/d values from the moment the chart was first created. On each 30s refresh, mdUpdateSOCChart only mutates the existing chart's data array and calls .update() - it never recreates the chart (mdRenderAll only calls mdInitSOCChart again if mdSocChart doesn't exist yet) - so the doughnut ring's fill proportion updated correctly but the on-screen number text stayed frozen at the first-load reading, while every other card on the page (Health, Cost, API, Solar) re-renders fresh from the latest data on every refresh. Fixes #4151. The afterDraw plugin now reads chart.data.datasets[0].data[0] for the percentage (already correctly updated by mdUpdateSOCChart) and the MD_DATA global for the kWh figures (already correctly reassigned on every refresh), instead of the stale closed-over values. No JS execution/browser test infra exists in this codebase, so this can't run the embedded JavaScript and check actual canvas output. Added a structural test instead, checking the generated JS source itself reads from chart.data/MD_DATA rather than the closure parameters, to catch a regression back to the same bug shape. 🤖 Implemented with Claude Code, disclosed per repo convention. Co-Authored-By: Claude Sonnet 5 --- .../test_metrics_dashboard_soc_refresh.py | 49 +++++++++++++++++++ apps/predbat/unit_test.py | 2 + apps/predbat/web_metrics_dashboard.py | 9 +++- 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 apps/predbat/tests/test_metrics_dashboard_soc_refresh.py diff --git a/apps/predbat/tests/test_metrics_dashboard_soc_refresh.py b/apps/predbat/tests/test_metrics_dashboard_soc_refresh.py new file mode 100644 index 000000000..40e4fdabf --- /dev/null +++ b/apps/predbat/tests/test_metrics_dashboard_soc_refresh.py @@ -0,0 +1,49 @@ +# ----------------------------------------------------------------------------- +# Predbat Home Battery System +# Copyright Trefor Southwell 2026 - All Rights Reserved +# This application maybe used for personal use only and not for commercial use +# ----------------------------------------------------------------------------- +# fmt off +# pylint: disable=consider-using-f-string +# pylint: disable=line-too-long +# pylint: disable=attribute-defined-outside-init + +""" +Test that the Metrics Dashboard's battery SoC doughnut chart center text reads live data on +refresh rather than being frozen at whatever it was on first page load - see issue #4151. + +There's no JS execution/browser test infra in this codebase, so this can't run the embedded +JavaScript and check actual canvas output. Instead it checks the generated JS source itself: +the afterDraw plugin (Chart.js) must read the chart's own current data array and the always- +current MD_DATA global, not the pct/d values that were closed over when the chart was first +created in mdInitSOCChart - mdUpdateSOCChart (called on every 30s refresh once the chart +already exists) only mutates the chart's data array, it never recreates the chart, so a closure +over the original values would freeze the on-screen text at the first-load reading forever. +""" + +from web_metrics_dashboard import get_metrics_dashboard_body + + +def test_soc_chart_center_text_reads_live_data(my_predbat): + """ + The afterDraw plugin body must not reference the pct/d parameters closed over at chart + creation time for the values it draws - it must read chart.data.datasets[0].data (updated + by mdUpdateSOCChart on every refresh) and the MD_DATA global (reassigned on every refresh). + """ + print("**** test_soc_chart_center_text_reads_live_data ****") + + body = get_metrics_dashboard_body("{}") + + # Isolate the md-center-text plugin's afterDraw function body, up to its closing "}]" + marker = "id: 'md-center-text'" + assert marker in body, "md-center-text plugin not found in dashboard body" + start = body.index(marker) + end = body.index("}]", start) + 2 + plugin_block = body[start:end] + + assert "chart.data.datasets[0].data[0]" in plugin_block, "afterDraw should read the live percentage from the chart's own current data, not a closed-over value" + assert "MD_DATA.battery_soc_kwh" in plugin_block, "afterDraw should read the live kWh figure from the MD_DATA global, not a closed-over value" + + print("✓ afterDraw plugin reads live chart data and MD_DATA, not stale closure values") + print("✓ Test passed") + return False diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index 90a586e7d..1b3c78307 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -65,6 +65,7 @@ from tests.test_hainterface_websocket import run_hainterface_websocket_tests from tests.test_web_if import run_test_web_if from tests.test_web_chart_currency import test_rates_chart_series_names_use_currency_symbol +from tests.test_metrics_dashboard_soc_refresh import test_soc_chart_center_text_reads_live_data from tests.test_web_functions import run_web_functions_tests from tests.test_window import run_window_sort_tests, run_intersect_window_tests from tests.test_find_charge_rate import test_find_charge_rate, test_find_charge_rate_string_temperature, test_find_charge_rate_string_charge_curve @@ -277,6 +278,7 @@ def main(): ("manual_select", run_test_manual_select, "Manual select tests", False), ("web_if", run_test_web_if, "Web interface tests", False), ("web_chart_currency", test_rates_chart_series_names_use_currency_symbol, "Rates chart series names follow currency_symbols tests", False), + ("metrics_dashboard_soc_refresh", test_soc_chart_center_text_reads_live_data, "Metrics dashboard SoC chart live-refresh tests", False), ("web_functions", run_web_functions_tests, "Web function unit tests", False), ("nordpool", run_nordpool_test, "Nordpool tests", False), ("futurerate_auto", test_futurerate_auto, "FutureRate auto Agile detection tests", False), diff --git a/apps/predbat/web_metrics_dashboard.py b/apps/predbat/web_metrics_dashboard.py index 82d76dff7..829c6a88a 100644 --- a/apps/predbat/web_metrics_dashboard.py +++ b/apps/predbat/web_metrics_dashboard.py @@ -209,16 +209,21 @@ def get_metrics_dashboard_body(data_json): options: { cutout: '72%', responsive: false, plugins: { legend: {display:false}, tooltip: {enabled:false} } }, plugins: [{ id: 'md-center-text', + // Read live values (chart's own current data, and the always-current MD_DATA global) rather + // than the pct/d closed over at chart creation time - mdUpdateSOCChart only mutates the + // chart's data array on refresh, it doesn't recreate the chart, so a closure over the + // original values here would freeze this text at whatever it was on first load. afterDraw: function (chart) { var w = chart.width, h = chart.height, c2 = chart.ctx; + var livePct = chart.data.datasets[0].data[0] || 0; c2.save(); c2.fillStyle = mdVar('--md-text'); c2.font = 'bold 2rem sans-serif'; c2.textAlign = 'center'; c2.textBaseline = 'middle'; - c2.fillText(mdFmt(pct, 0) + '%', w / 2, h / 2 - 10); + c2.fillText(mdFmt(livePct, 0) + '%', w / 2, h / 2 - 10); c2.font = '0.85rem sans-serif'; c2.fillStyle = mdVar('--md-muted'); - c2.fillText(mdFmt(d.battery_soc_kwh, 1) + ' / ' + mdFmt(d.battery_max_kwh, 1) + ' kWh', w / 2, h / 2 + 18); + c2.fillText(mdFmt(MD_DATA.battery_soc_kwh, 1) + ' / ' + mdFmt(MD_DATA.battery_max_kwh, 1) + ' kWh', w / 2, h / 2 + 18); c2.restore(); } }] From 227f57311aaff68bd1fb754a7b77b2b808ed0c10 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sat, 25 Jul 2026 22:33:51 +0100 Subject: [PATCH 6/6] test(web): address Copilot review feedback on the Rates chart currency test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues flagged on PR #4288: - The test mutated my_predbat.currency_symbols without restoring it, leaking state into later tests in the same suite process (the shared my_predbat fixture is reused across all tests). - The test never actually called WebInterface.get_chart()/web.py - it only re-tested the same inline string-formatting logic, so it would still pass even if web.py regressed back to a hardcoded "p/kWh". Now builds a minimal WebInterface (bypassing ComponentBase.__init__, which would stand up the real aiohttp app) and calls the real get_chart("Rates"), stubbing get_history_wrapper() directly to avoid reproducing realistic HA history formatting for data not under test. Verified this actually catches a regression: temporarily reintroducing the hardcoded "p/kWh" strings in web.py makes the test fail as expected. currency_symbols/dashboard_values are now saved and restored via try/finally. 🤖 Implemented with Claude Code, disclosed per repo convention. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/tests/test_web_chart_currency.py | 81 +++++++++++++++---- 1 file changed, 66 insertions(+), 15 deletions(-) diff --git a/apps/predbat/tests/test_web_chart_currency.py b/apps/predbat/tests/test_web_chart_currency.py index 91ee72261..dc3ce0c3f 100644 --- a/apps/predbat/tests/test_web_chart_currency.py +++ b/apps/predbat/tests/test_web_chart_currency.py @@ -14,26 +14,77 @@ user configured for NZ dollars/cents saw "p/kWh" (pence) in the chart legend regardless. """ +import re + +from web import WebInterface + + +def _make_web(my_predbat): + """ + Build a minimal WebInterface bound to my_predbat, bypassing ComponentBase.__init__ (which + would stand up the real aiohttp app). currency_symbols/now_utc/minutes_now/etc are all + read-only properties on ComponentBase that delegate to self.base, so nothing else needs + setting here for get_chart() to run. + """ + w = WebInterface.__new__(WebInterface) + w.base = my_predbat + w.log = my_predbat.log + w.prefix = my_predbat.prefix + return w + def test_rates_chart_series_names_use_currency_symbol(my_predbat): """ - Directly mirrors the series-name formatting used in WebInterface.get_chart's "Rates" - branch (web.py) - get_chart itself requires a fully computed plan (soc_kw_best populated) - before it reaches this branch, so this tests the formatting logic directly rather than - standing up that heavier machinery. + Calls the real WebInterface.get_chart("Rates") and checks the rendered series names, + rather than re-testing the formatting logic in isolation - a prior version of this test + only duplicated the format string inline, so it would still pass even if web.py regressed + back to a hardcoded "p/kWh" (Copilot review on PR #4288). + + get_chart() gates on soc_kw_best being populated (dashboard_values) and reads the Hourly/ + Today series from get_history_wrapper() - both are stubbed directly rather than trying to + reproduce realistic HA history formatting, since only the currency-dependent series *names* + are under test here, not the underlying rate data. """ print("**** test_rates_chart_series_names_use_currency_symbol ****") - for currency_symbols, expected_minor in [("£p", "p"), ("$c", "c"), ("€c", "c")]: - my_predbat.currency_symbols = currency_symbols - hourly_name = "Hourly {}/kWh".format(my_predbat.currency_symbols[1]) - today_name = "Today {}/kWh".format(my_predbat.currency_symbols[1]) + original_currency_symbols = my_predbat.currency_symbols + original_dashboard_values = getattr(my_predbat, "dashboard_values", None) + + try: + w = _make_web(my_predbat) + my_predbat.dashboard_values = { + "predbat.soc_kw_best": {"attributes": {"results": {"2026-01-01T00:00:00+00:00": 5.0}}}, + "predbat.rates": {"attributes": {"results": {"2026-01-01T00:00:00+00:00": 10.0}}}, + } + fake_history = [ + [ + {"state": 10.5, "last_updated": "2026-01-01T00:00:00+00:00"}, + {"state": 12.3, "last_updated": "2026-01-01T00:30:00+00:00"}, + ] + ] + w.get_history_wrapper = lambda *a, **kw: fake_history + + for currency_symbols, expected_minor in [("£p", "p"), ("$c", "c"), ("€c", "c")]: + my_predbat.currency_symbols = currency_symbols + + result = w.get_chart("Rates") + series_names = re.findall(r"name: '([^']*)'", result) + + expected_hourly = "Hourly {}/kWh".format(expected_minor) + expected_today = "Today {}/kWh".format(expected_minor) - assert hourly_name == "Hourly {}/kWh".format(expected_minor), f"Expected 'Hourly {expected_minor}/kWh', got '{hourly_name}'" - assert today_name == "Today {}/kWh".format(expected_minor), f"Expected 'Today {expected_minor}/kWh', got '{today_name}'" - if expected_minor != "p": - assert "p/kWh" not in hourly_name and "p/kWh" not in today_name, f"Series name should not be hardcoded to pence for currency_symbols={currency_symbols}" + assert expected_hourly in series_names, f"Expected series '{expected_hourly}' in {series_names} for currency_symbols={currency_symbols}" + assert expected_today in series_names, f"Expected series '{expected_today}' in {series_names} for currency_symbols={currency_symbols}" + if expected_minor != "p": + assert not any("p/kWh" in name for name in series_names), f"Series names should not be hardcoded to pence for currency_symbols={currency_symbols}, got {series_names}" - print("✓ Rates chart series names correctly follow currency_symbols[1] (£p, $c, €c all verified)") - print("✓ Test passed") - return False + print("✓ Rates chart series names correctly follow currency_symbols[1] (£p, $c, €c all verified against the real get_chart() output)") + print("✓ Test passed") + return False + finally: + my_predbat.currency_symbols = original_currency_symbols + if original_dashboard_values is None: + if hasattr(my_predbat, "dashboard_values"): + del my_predbat.dashboard_values + else: + my_predbat.dashboard_values = original_dashboard_values