Skip to content

Commit 004f298

Browse files
fix: address review feedback on saving session entity fallback
- Distinguish "attribute genuinely missing" from "attribute present but an empty list" using a sentinel, so the binary_sensor -> event fallback no longer misfires when the configured entity is valid but currently has no events (a normal, common state). - Isolate the new regression test's mutation of my_predbat.args behind a save/restore, matching the pattern used by other tests in this suite. Addresses Copilot review comments on PR #4585.
1 parent 06f9f0b commit 004f298

2 files changed

Lines changed: 50 additions & 39 deletions

File tree

apps/predbat/octopus.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@
3535
DATE_STR_FORMAT = "%Y-%m-%d"
3636
DATE_TIME_STR_FORMAT = "%Y-%m-%dT%H:%M:%S%z"
3737

38+
# Sentinel distinguishing "attribute not present on the entity" from a genuinely empty list,
39+
# since both would otherwise look the same (falsy) to callers.
40+
_ATTRIBUTE_UNSET = object()
41+
3842
# Night-rate window definitions: start time, end time, whether the window crosses midnight.
3943
# Keys: "eco7" (Economy 7), "go" (Octopus GO / generic day-night), "iog" (Intelligent GO TOU).
4044
OCTOPUS_NIGHT_RATE_WINDOWS = {
@@ -2939,18 +2943,22 @@ def fetch_octopus_sessions(self, axle_sessions=None):
29392943
entity_id = self.get_arg("octopus_saving_session", indirect=False)
29402944
if entity_id:
29412945
state = self.get_arg("octopus_saving_session", False)
2942-
joined_events = self.get_state_wrapper(entity_id=entity_id, attribute="joined_events")
2943-
available_events = self.get_state_wrapper(entity_id=entity_id, attribute="available_events")
2944-
if not joined_events and not available_events:
2945-
# Legacy binary_sensor entities carry neither attribute - fall back to the
2946-
# newer event entity naming convention, but only adopt it if it actually has data
2946+
joined_events = self.get_state_wrapper(entity_id=entity_id, attribute="joined_events", default=_ATTRIBUTE_UNSET)
2947+
available_events = self.get_state_wrapper(entity_id=entity_id, attribute="available_events", default=_ATTRIBUTE_UNSET)
2948+
if joined_events is _ATTRIBUTE_UNSET and available_events is _ATTRIBUTE_UNSET:
2949+
# Legacy binary_sensor entities carry neither attribute at all - fall back to the
2950+
# newer event entity naming convention, but only adopt it if it actually has data.
2951+
# A configured entity that has the attributes but with no events right now (empty
2952+
# lists) is a valid state and must not trigger this fallback.
29472953
fallback_entity_id = entity_id.replace("binary_sensor.", "event.").replace("_sessions", "_session_events")
2948-
fallback_joined_events = self.get_state_wrapper(entity_id=fallback_entity_id, attribute="joined_events")
2949-
fallback_available_events = self.get_state_wrapper(entity_id=fallback_entity_id, attribute="available_events")
2950-
if fallback_joined_events or fallback_available_events:
2954+
fallback_joined_events = self.get_state_wrapper(entity_id=fallback_entity_id, attribute="joined_events", default=_ATTRIBUTE_UNSET)
2955+
fallback_available_events = self.get_state_wrapper(entity_id=fallback_entity_id, attribute="available_events", default=_ATTRIBUTE_UNSET)
2956+
if fallback_joined_events is not _ATTRIBUTE_UNSET or fallback_available_events is not _ATTRIBUTE_UNSET:
29512957
entity_id = fallback_entity_id
29522958
joined_events = fallback_joined_events
29532959
available_events = fallback_available_events
2960+
joined_events = [] if joined_events is _ATTRIBUTE_UNSET else joined_events
2961+
available_events = [] if available_events is _ATTRIBUTE_UNSET else available_events
29542962

29552963
if available_events and not self.get_arg("octopus_saving_auto_join", True):
29562964
self.log("Octopus: Saving session auto-join is disabled, not joining available events")

apps/predbat/tests/test_saving_session.py

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -624,40 +624,43 @@ def test_saving_session_custom_entity_no_rewrite_match(my_predbat):
624624
friendly_name: Predbat Octopus Power Down For Predbat
625625
"""
626626

627-
ha.dummy_items.clear()
628-
ha.dummy_items["binary_sensor.predbat_octopus_power_down_for_predbat"] = yaml.safe_load(session_binary)
629-
ha.dummy_items["sensor.octopus_free_session"] = {}
630-
my_predbat.args["octopus_saving_session"] = "binary_sensor.predbat_octopus_power_down_for_predbat"
631-
my_predbat.args["octopus_free_session"] = "sensor.octopus_free_session"
632-
if "octopus_free_url" in my_predbat.args:
633-
del my_predbat.args["octopus_free_url"]
634-
if "octopus_saving_session_join" in my_predbat.args:
635-
del my_predbat.args["octopus_saving_session_join"]
636-
my_predbat.args["octopus_saving_session_octopoints_per_penny"] = 10
637-
# Reset throttle so a join is attempted
638-
my_predbat.octopus_last_joined_try = None
639-
640-
ha.service_store_enable = True
641-
ha.service_store = []
642-
my_predbat.fetch_octopus_sessions()
643-
service_result = ha.get_service_store()
644-
ha.service_store_enable = False
627+
saved_args = my_predbat.args.copy()
628+
try:
629+
ha.dummy_items.clear()
630+
ha.dummy_items["binary_sensor.predbat_octopus_power_down_for_predbat"] = yaml.safe_load(session_binary)
631+
ha.dummy_items["sensor.octopus_free_session"] = {}
632+
my_predbat.args["octopus_saving_session"] = "binary_sensor.predbat_octopus_power_down_for_predbat"
633+
my_predbat.args["octopus_free_session"] = "sensor.octopus_free_session"
634+
if "octopus_free_url" in my_predbat.args:
635+
del my_predbat.args["octopus_free_url"]
636+
if "octopus_saving_session_join" in my_predbat.args:
637+
del my_predbat.args["octopus_saving_session_join"]
638+
my_predbat.args["octopus_saving_session_octopoints_per_penny"] = 10
639+
# Reset throttle so a join is attempted
640+
my_predbat.octopus_last_joined_try = None
645641

646-
join_calls = [svc for svc in service_result if "join" in svc[0]]
647-
if len(join_calls) != 1:
648-
print(f"ERROR: Expected 1 join call reading available_events from the configured entity, got {len(join_calls)}: {service_result}")
649-
failed = True
650-
elif join_calls[0][1].get("entity_id") != "binary_sensor.predbat_octopus_power_down_for_predbat":
651-
print(f"ERROR: Expected join call to use the configured entity, got {join_calls[0][1]}")
652-
failed = True
653-
else:
654-
print(" PASS: available_events read from the configured entity despite no rewrite match")
642+
ha.service_store_enable = True
643+
ha.service_store = []
644+
my_predbat.fetch_octopus_sessions()
645+
service_result = ha.get_service_store()
646+
ha.service_store_enable = False
655647

656-
if not failed:
657-
print("PASS: Custom entity name (no rewrite match) auto-join test passed")
648+
join_calls = [svc for svc in service_result if "join" in svc[0]]
649+
if len(join_calls) != 1:
650+
print(f"ERROR: Expected 1 join call reading available_events from the configured entity, got {len(join_calls)}: {service_result}")
651+
failed = True
652+
elif join_calls[0][1].get("entity_id") != "binary_sensor.predbat_octopus_power_down_for_predbat":
653+
print(f"ERROR: Expected join call to use the configured entity, got {join_calls[0][1]}")
654+
failed = True
655+
else:
656+
print(" PASS: available_events read from the configured entity despite no rewrite match")
658657

659-
# Restore default throttle state so we do not leak it to other tests
660-
my_predbat.octopus_last_joined_try = None
658+
if not failed:
659+
print("PASS: Custom entity name (no rewrite match) auto-join test passed")
660+
finally:
661+
my_predbat.args = saved_args
662+
# Restore default throttle state so we do not leak it to other tests
663+
my_predbat.octopus_last_joined_try = None
661664

662665
return failed
663666

0 commit comments

Comments
 (0)