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
10 changes: 5 additions & 5 deletions apps/predbat/components.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ def load_component_class(component_info):
"solcast_host": {"required": False, "config": "solcast_host", "default": "https://api.solcast.com.au/"},
"solcast_api_key": {"required": False, "secret": True, "config": "solcast_api_key"},
"solcast_sites": {"required": False, "config": "solcast_sites"},
"solcast_poll_hours": {"required": False, "config": "solcast_poll_hours", "default": 8},
"solcast_poll_hours": {"required": False, "config": "solcast_poll_hours", "default": 8.0},
"forecast_solar": {"required": False, "config": "forecast_solar", "default": False},
"forecast_solar_max_age": {"required": False, "config": "forecast_solar_max_age", "default": 8},
"forecast_solar_max_age": {"required": False, "config": "forecast_solar_max_age", "default": 8.0},
"forecast_solar_open_meteo_backup": {"required": False, "config": "forecast_solar_open_meteo_backup", "default": False},
"forecast_solar_open_meteo_first": {"required": False, "config": "forecast_solar_open_meteo_first", "default": False},
"pv_forecast_today": {"required": False, "config": "pv_forecast_today"},
Expand All @@ -134,7 +134,7 @@ def load_component_class(component_info):
"pv_forecast_d4": {"required": False, "config": "pv_forecast_d4"},
"pv_scaling": {"required": False, "config": "pv_scaling", "default": 1.0},
"open_meteo_forecast": {"required": False, "config": "open_meteo_forecast", "default": False},
"open_meteo_forecast_max_age": {"required": False, "config": "open_meteo_forecast_max_age", "default": 4},
"open_meteo_forecast_max_age": {"required": False, "config": "open_meteo_forecast_max_age", "default": 4.0},
},
"required_or": ["solcast_api_key", "forecast_solar", "pv_forecast_today", "open_meteo_forecast"],
"phase": 2, # Solar component moved to phase 2 so that any Predbat cloud components (such as GEcloud) have been started and initialised pv_today, etc
Expand Down Expand Up @@ -458,7 +458,7 @@ def load_component_class(component_info):
# one from, so it is estimated from poinv. This is the escape hatch for a user
# who knows their pack's real limit.
"battery_rate_max": {"required": False, "config": "alphaess_battery_rate_max"},
"api_delay": {"required": False, "default": 2, "config": "alphaess_api_delay"},
"api_delay": {"required": False, "default": 2.0, "config": "alphaess_api_delay"},
"min_write_interval": {"required": False, "default": 300, "config": "alphaess_min_write_interval"},
},
# Gate activation on having an AppID. Without this the component would start for
Expand Down Expand Up @@ -605,7 +605,7 @@ def load_component_class(component_info):
"event_filter": "predbat_axle_",
"args": {
"api_key": {"required": False, "secret": True, "config": "axle_api_key"},
"pence_per_kwh": {"required": False, "config": "axle_pence_per_kwh", "default": 100},
"pence_per_kwh": {"required": False, "config": "axle_pence_per_kwh", "default": 100.0},
"automatic": {"required": False, "config": "axle_automatic", "default": True},
"managed_mode": {"required": False, "config": "axle_managed_mode", "default": False},
"site_id": {"required": False, "secret": True, "config": "axle_site_id"},
Expand Down
6 changes: 3 additions & 3 deletions apps/predbat/octopus.py
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,7 @@ def get_saving_session_data(self):
# Default saving session rate in octopoints/kWh
# octopus_saving_session_rate is in p/kWh, convert to octopoints
octopoints_per_penny = self.get_arg("octopus_saving_session_octopoints_per_penny", 8)
default_rate_pence = self.get_arg("octopus_saving_session_rate", 100) # 100p/kWh default
default_rate_pence = self.get_arg("octopus_saving_session_rate", 100.0) # 100p/kWh default
default_octopoints = default_rate_pence * octopoints_per_penny

if not has_joined:
Expand Down Expand Up @@ -4212,7 +4212,7 @@ def fetch_octopus_sessions(self, axle_sessions=None):
if "octopus_saving_session" in self.args:
saving_rate = 200 # Default rate if not reported
octopoints_per_penny = self.get_arg("octopus_saving_session_octopoints_per_penny", 8) # Default 8 octopoints per penny
octopoints_min_threshold = self.get_arg("octopus_saving_session_min_octopoints_per_kwh", 0)
octopoints_min_threshold = self.get_arg("octopus_saving_session_min_octopoints_per_kwh", 0.0)
join_lead_hours = self.get_arg("octopus_saving_auto_join_lead_hours", 0)

joined_events = []
Expand Down Expand Up @@ -4327,7 +4327,7 @@ def fetch_octopus_sessions(self, axle_sessions=None):

# Default saving session rate for when octopoints_per_kwh is not available
# (e.g. new flexibility API events that don't report reward rates)
default_rate_pence = self.get_arg("octopus_saving_session_rate", 0)
default_rate_pence = self.get_arg("octopus_saving_session_rate", 0.0)

if joined_events:
for event in joined_events:
Expand Down
99 changes: 99 additions & 0 deletions apps/predbat/tests/test_integer_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,3 +478,102 @@ def test_metric_battery_value_scaling_step_resolves_export_margin(my_predbat):

print("✓ Test passed: metric_battery_value_scaling step {} keeps the first nudge ({:.4f}) clear of the {:.4f} flip point and keeps old 0.1-step values valid".format(step, first_nudge, flip_point))
return False


def _float_declared_keys():
"""The APPS_SCHEMA keys whose declared type includes "float"."""
from config import APPS_SCHEMA

return {key for key, spec in APPS_SCHEMA.items() if "float" in str(spec.get("type", "")).split("|")}


def test_float_declared_keys_never_read_with_int_default(my_predbat):
"""
Guard for #4925: no call site may read a key APPS_SCHEMA declares "float" with a bare int literal default.
get_arg() coerces its return value on the *type* of the default it is handed and applies that to
whatever value was resolved, real configured value or not. So `get_arg("solcast_poll_hours", 8)`
ran a configured 4.8 through int(float(value)) and returned 4 - shortening the Solcast poll TTL
to 4h and pushing a two-site hobbyist account past its 10 poll/day quota. Nothing warned:
validate_config() checks the raw apps.yaml value against the float schema and passes it.
The fix is a float literal at each call site. This scans the source rather than asserting on
today's eight sites, so a new `get_arg("<float key>", 8)` or a COMPONENT_LIST spec with
`"default": 8` fails here instead of truncating in production. It sees literals only - a
default passed through a variable, or a key read through a wrapper accessor, is not checked.
"""
print("**** test_float_declared_keys_never_read_with_int_default ****")
import ast
import glob
import os
import components

float_keys = _float_declared_keys()
source_dir = os.path.dirname(os.path.abspath(components.__file__))
offenders = []
scanned = 0
for path in sorted(glob.glob(os.path.join(source_dir, "*.py"))):
scanned += 1
with open(path, encoding="utf-8") as source:
tree = ast.parse(source.read())
for node in ast.walk(tree):
if isinstance(node, ast.Call) and getattr(node.func, "attr", getattr(node.func, "id", None)) == "get_arg" and node.args and isinstance(node.args[0], ast.Constant) and node.args[0].value in float_keys:
default = node.args[1] if len(node.args) > 1 else None
for keyword in node.keywords:
if keyword.arg == "default":
default = keyword.value
if isinstance(default, ast.Constant) and type(default.value) is int:
offenders.append("{}:{} get_arg({!r}, {})".format(os.path.basename(path), node.lineno, node.args[0].value, default.value))
elif isinstance(node, ast.Dict):
keys = [key.value if isinstance(key, ast.Constant) else None for key in node.keys]
if "config" in keys and "default" in keys:
config = node.values[keys.index("config")]
default = node.values[keys.index("default")]
if isinstance(config, ast.Constant) and config.value in float_keys and isinstance(default, ast.Constant) and type(default.value) is int:
offenders.append("{}:{} arg spec {!r} default {}".format(os.path.basename(path), node.lineno, config.value, default.value))

assert scanned > 10 and float_keys, "Guard found nothing to scan ({} files, {} float keys) - has the layout changed?".format(scanned, len(float_keys))
assert not offenders, "Float-declared keys read with an int default, which truncates a configured fraction (#4925) - write e.g. 8.0: {}".format("; ".join(offenders))

print("✓ Test passed: {} float-declared keys, no int defaults across {} source files".format(len(float_keys), scanned))
return False


def test_component_arg_specs_resolve_float_declared_keys_as_float(my_predbat):
"""
Behavioural companion to the source guard above (#4925): every COMPONENT_LIST arg spec with a
default whose "config" key APPS_SCHEMA declares "float" must resolve a configured fraction
intact, resolved exactly the way Components.initialize() does
(`arg_dict[arg] = self.base.get_arg(arg_info["config"], default, indirect=indirect)`).
"""
print("**** test_component_arg_specs_resolve_float_declared_keys_as_float ****")

from components import COMPONENT_LIST

float_keys = _float_declared_keys()
original_args = my_predbat.args.copy()
checked = []
try:
for component_name, component_info in COMPONENT_LIST.items():
for arg, arg_info in component_info.get("args", {}).items():
config_name = arg_info.get("config", None)
if not config_name or arg_info.get("config_late_resolve", False):
continue
if config_name not in float_keys:
continue
if arg_info.get("default", None) is None:
# No default means no coercion, so there is nothing that could truncate
continue

my_predbat.args[config_name] = 4.8
value = my_predbat.get_arg(config_name, arg_info.get("default", None), indirect=arg_info.get("indirect", False))
my_predbat.args.pop(config_name, None)
assert value == 4.8, "{}.{} ({}) resolved a configured 4.8 as {} ({}) - a float-declared key must not be truncated by its arg spec default".format(component_name, arg, config_name, value, type(value))
checked.append(config_name)
finally:
my_predbat.args = original_args

assert checked, "No float-declared component arg specs found to check - has APPS_SCHEMA or COMPONENT_LIST changed shape?"

print("✓ Test passed: {} float-declared component arg spec(s) resolve fractional values intact: {}".format(len(checked), ", ".join(sorted(set(checked)))))
return False
4 changes: 4 additions & 0 deletions apps/predbat/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@
test_get_ha_config_normalises_int_default_for_fractional_step,
test_metric_battery_cycle_fractional_value_not_truncated,
test_metric_battery_value_scaling_step_resolves_export_margin,
test_float_declared_keys_never_read_with_int_default,
test_component_arg_specs_resolve_float_declared_keys_as_float,
)
from tests.test_predbat_metrics_data_age import test_data_age_metrics_round_trip
from tests.test_metrics_dashboard_control_conflicts import test_control_conflicts_metrics_round_trip, test_control_conflicts_dashboard_renders_section
Expand Down Expand Up @@ -669,6 +671,8 @@ def main():
("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),
("metric_battery_value_scaling_step", test_metric_battery_value_scaling_step_resolves_export_margin, "metric_battery_value_scaling step resolves the export margin (#4840)", False),
("float_keys_float_defaults", test_float_declared_keys_never_read_with_int_default, "No float-declared APPS_SCHEMA key is read with an int default (#4925)", False),
("component_arg_float_specs", test_component_arg_specs_resolve_float_declared_keys_as_float, "COMPONENT_LIST arg specs resolve float-declared keys as float (#4925)", False),
("data_age_metrics", test_data_age_metrics_round_trip, "Metrics dashboard data_age_days/data_age_required_days tests", False),
("control_conflicts_metrics", test_control_conflicts_metrics_round_trip, "Metrics dashboard control_conflicts round-trip tests", False),
("control_conflicts_dashboard", test_control_conflicts_dashboard_renders_section, "Metrics dashboard control_conflicts section render tests", False),
Expand Down
Loading