Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions apps/predbat/tests/test_integer_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,83 @@ def is_integer_valued(value):

print("✓ Test passed: all integer-step input_number items have integer-valued min/max")
return False


def test_get_ha_config_normalises_int_default_for_fractional_step(my_predbat):
"""
Mechanism-level regression test for #4296: get_ha_config() must hand back a float default for
any input_number item with a fractional step, even if CONFIG_ITEMS happens to declare its
"default" as a bare Python int (e.g. 0 instead of 0.0).

This default is not merely used when a value is missing - get_arg() (the only caller that
reaches get_ha_config with default=None) applies a further type coercion to whatever value
get_ha_config returns, keyed on the *type* of the resolved default, regardless of whether the
returned value actually came from that default or from the item's real, present, configured
value. So a call site reading the item with no explicit default (e.g. fetch.py's plain
`self.metric_battery_cycle = self.get_arg("metric_battery_cycle")`) can have its correctly
resolved value coerced by a default it never fell back to: confirmed live for
metric_battery_cycle, whose CONFIG_ITEMS "default" of 0 (an int) caused a genuinely
user-configured 0.5 to still be truncated to 0 via get_arg's `int(float(value))`, on every
read, every ~5 minutes - not because 0.5 was missing, but because the default's type alone
decided how the real value got coerced.

Fixed at the source in get_ha_config() (userinterface.py) rather than by hand-editing each
affected item's "default" to a float literal - a future item added with an int default and a
fractional step is protected automatically, regardless of which literal its author happens to
write. This test proves that directly: it deliberately restores a real CONFIG_ITEMS entry's
"default" to an int (undoing whatever it's currently declared as) to simulate "a new setting
with the same mistake", and confirms get_ha_config() still normalises it.
"""
print("**** test_get_ha_config_normalises_int_default_for_fractional_step ****")

item = my_predbat.config_index.get("metric_battery_cycle")
assert item is not None, "metric_battery_cycle config item not found"
assert item.get("step") == 0.1, f"metric_battery_cycle step should be fractional (0.1), got {item.get('step')}"

original_default = item.get("default")
original_value = item.get("value")
try:
# Simulate a future item authored with an int default despite a fractional step -
# regardless of what config.py currently declares, get_ha_config must still normalise it.
item["default"] = 0
item["value"] = None

value, resolved_default = my_predbat.get_ha_config("metric_battery_cycle", None)
assert isinstance(resolved_default, float), f"get_ha_config should normalise an int default to float for a fractional-step item, got {type(resolved_default)}"
assert value == 0.0 and isinstance(value, float), f"Expected float 0.0, got {value!r} ({type(value)})"
finally:
item["default"] = original_default
item["value"] = original_value

print("✓ Test passed: get_ha_config normalises an int default to float for a fractional-step item")
return False


def test_metric_battery_cycle_fractional_value_not_truncated(my_predbat):
"""
Regression test for #4296: a fractional metric_battery_cycle (e.g. 0.5p/kWh) must survive
get_arg(), not get silently truncated to an integer. This is the concrete runtime symptom the
mechanism-level test above (test_get_ha_config_normalises_int_default_for_fractional_step)
exists to prevent.
"""
print("**** test_metric_battery_cycle_fractional_value_not_truncated ****")

# metric_battery_cycle is gated on expert_mode - enable it so get_ha_config doesn't just
# null the value out and mask the truncation this test is checking for.
original_expert_mode = my_predbat.config_index["expert_mode"].get("value")
original_value = my_predbat.config_index["metric_battery_cycle"].get("value")
my_predbat.expose_config("expert_mode", True, force_ha=True)
my_predbat.expose_config("metric_battery_cycle", 0.5, force_ha=True)

try:
value = my_predbat.get_arg("metric_battery_cycle")
assert value == 0.5, "get_arg('metric_battery_cycle') should return 0.5, got {} ({})".format(value, type(value))

my_predbat.fetch_config_options()
assert my_predbat.metric_battery_cycle == 0.5, "self.metric_battery_cycle after fetch_config_options() should be 0.5, got {} ({})".format(my_predbat.metric_battery_cycle, type(my_predbat.metric_battery_cycle))
finally:
my_predbat.expose_config("metric_battery_cycle", original_value, force_ha=True)
my_predbat.expose_config("expert_mode", original_expert_mode, force_ha=True)

print("✓ Test passed: a fractional metric_battery_cycle value is not truncated")
return False
11 changes: 10 additions & 1 deletion apps/predbat/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,14 @@
from tests.test_battery_curve_keys import run_battery_curve_keys_tests
from tests.test_balance_inverters import run_balance_inverters_tests
from tests.test_octopus_download_rates import test_octopus_download_rates_wrapper
from tests.test_integer_config import test_integer_config_entities, test_expose_config_preserves_integer, test_config_item_range_clamp, test_config_item_step_min_max_types_consistent
from tests.test_integer_config import (
test_integer_config_entities,
test_expose_config_preserves_integer,
test_config_item_range_clamp,
test_config_item_step_min_max_types_consistent,
test_get_ha_config_normalises_int_default_for_fractional_step,
test_metric_battery_cycle_fractional_value_not_truncated,
)
from tests.test_predbat_metrics_data_age import test_data_age_metrics_round_trip
from tests.test_validate_config import test_validate_config, test_validate_config_retry
from tests.test_plan_json_rate_adjust import run_test_plan_json_rate_adjust
Expand Down Expand Up @@ -401,6 +408,8 @@ def main():
("expose_config_integer", test_expose_config_preserves_integer, "Expose config preserves integer tests", False),
("config_item_range_clamp", test_config_item_range_clamp, "Config item min/max range clamp tests", False),
("config_item_step_min_max_types", test_config_item_step_min_max_types_consistent, "Config item step/min/max type consistency tests", False),
("get_ha_config_fractional_default", test_get_ha_config_normalises_int_default_for_fractional_step, "get_ha_config normalises int default to float for fractional-step items (#4296)", False),
("metric_battery_cycle_fractional", test_metric_battery_cycle_fractional_value_not_truncated, "metric_battery_cycle fractional value not truncated by get_arg (#4296)", False),
("data_age_metrics", test_data_age_metrics_round_trip, "Metrics dashboard data_age_days/data_age_required_days tests", False),
("plan_json_rate_adjust", run_test_plan_json_rate_adjust, "Plan JSON rate adjust type field tests", False),
("plan_why_reason", run_test_plan_why_reason, "Plan JSON per-slot 'why' reason text tests", False),
Expand Down
15 changes: 15 additions & 0 deletions apps/predbat/userinterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,21 @@ def get_ha_config(self, name, default):
value = None
if default is None:
default = item.get("default", None)
if item.get("type") == "input_number" and isinstance(default, int) and not isinstance(default, bool):
# This default is not just a fallback for a missing value - get_arg() (the only
# caller that reaches here with default=None) applies a further type coercion to
# whatever value this function returns, keyed on the *type* of this default,
# regardless of whether that returned value is this default or the item's real
# configured value. So an int default doesn't just risk supplying an int when
# unset - it forces every read of this item back to an int even when the user has
# genuinely configured a fractional one, via get_arg's int(float(value)) (#4296:
# metric_battery_cycle's real, present, correctly-resolved 0.5 was still coerced
# to 0 downstream, purely because its default happened to be the int 0). Normalise
# here, at the source, so it can't matter which literal a future item's "default"
# happens to be written as.
step = item.get("step", 1)
if isinstance(step, float) and step != int(step):
default = float(default)
if value is None:
value = default
return value, default
Expand Down
Loading