From b50c83c386877bc59c74691cf0bb765a4405f090 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 28 Aug 2026 18:46:02 +0100 Subject: [PATCH 1/2] fix(octopus): scope the zero-kWh dispatch exemption to genuinely parsed input decode_octopus_slot() previously coerced any unparseable charge_in_kwh value to 0.0, indistinguishable by value alone from a real zero-kWh SMART grid-flex event. rate_add_io_slots()'s exemption from the octopus_slot_max cap and the #4482 need-check was scoped to source == "SMART" as a first pass, but a malformed entry can carry that source too. Threads a kwh_valid flag through decode_octopus_slot()'s return - True unless a present kwh value failed to parse as a number - and requires it in the exemption condition, closing the gap regardless of source. #4807, review follow-up on #4483 from Speshman. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/octopus.py | 34 ++++++++++++-------- apps/predbat/tests/test_rate_add_io_slots.py | 12 +++++++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/apps/predbat/octopus.py b/apps/predbat/octopus.py index 360cd8ec8..3418fd1ba 100644 --- a/apps/predbat/octopus.py +++ b/apps/predbat/octopus.py @@ -2839,6 +2839,12 @@ def minute_in_iog_fixed_window(self, minute_abs): def decode_octopus_slot(self, car_n, slot, raw=False): """ Decode IOG slot + + The sixth return value, kwh_valid, is False only when a present charge_in_kwh/energy/ + chargeKwh value couldn't be parsed as a number - not when kwh is legitimately absent (and + synthesised from the slot duration) or the slot is a genuine zero-length/empty entry. This + lets callers tell a malformed dispatch entry apart from a real zero-kWh one (#4483 review + follow-up, Speshman) even though both end up with kwh == 0. """ if "start" in slot: start = datetime.strptime(slot["start"], TIME_FORMAT) @@ -2860,7 +2866,7 @@ def decode_octopus_slot(self, car_n, slot, raw=False): end_minutes = max(min(end_minutes, self.forecast_minutes + self.minutes_now), start_minutes) if start_minutes == end_minutes: - return 0, 0, 0, source, location + return 0, 0, 0, source, location, True cap_minutes = end_minutes - start_minutes @@ -2874,23 +2880,25 @@ def decode_octopus_slot(self, car_n, slot, raw=False): # Remove empty slots if kwh is None and location == "" and source == "": - return 0, 0, 0, source, location + return 0, 0, 0, source, location, True # Create kWh if missing if kwh is None: kwh = org_minutes * self.car_charging_rate[car_n] / 60.0 + kwh_valid = True try: kwh = abs(float(kwh)) except (ValueError, TypeError): kwh = 0.0 + kwh_valid = False if org_minutes > 0: kwh = kwh * cap_minutes / org_minutes else: kwh = 0 - return start_minutes, end_minutes, kwh, source, location + return start_minutes, end_minutes, kwh, source, location, kwh_valid def load_octopus_slots(self, car_n, octopus_slots, octopus_intelligent_consider_full): """ @@ -2909,7 +2917,7 @@ def load_octopus_slots(self, car_n, octopus_slots, octopus_intelligent_consider_ # Decode the slots for slot in octopus_slots: - start_minutes, end_minutes, kwh, source, location = self.decode_octopus_slot(car_n, slot) + start_minutes, end_minutes, kwh, source, location, _ = self.decode_octopus_slot(car_n, slot) # Octopus zeros chargeKwh once it calculates the car has hit its target SoC, but the # dispatch window stays open and the charger may still draw power. Preserve active slots # with a duration-based kwh so the "Hold for car" guard in execute.py still fires. @@ -3086,7 +3094,7 @@ def rate_add_io_slots(self, car_n, rates, octopus_slots): if octopus_slots: # Add in IO slots for slot in octopus_slots: - start_minutes, end_minutes, kwh, source, location = self.decode_octopus_slot(car_n, slot, raw=True) + start_minutes, end_minutes, kwh, source, location, kwh_valid = self.decode_octopus_slot(car_n, slot, raw=True) # Ignore bump-charge slots as their cost won't change if source != "bump-charge" and source != "BOOST" and (not location or location == "AT_HOME"): @@ -3142,15 +3150,13 @@ def rate_add_io_slots(self, car_n, rates, octopus_slots): # octopus_slot_count_zero_kwh restores the old behaviour of counting every # dispatch entry, zero-kWh or not, toward the cap like any other. # - # Scoped to source == "SMART" (#4483 review follow-up, Speshman): kwh only - # ever reaches 0 either from a genuine zero-kWh entry or from - # decode_octopus_slot() silently coercing malformed/unparseable input to - # 0.0 - the two are indistinguishable by value alone. Requiring the source - # this feature was actually built for narrows a parse failure exploiting the - # exemption to the coincidence of also carrying source=="SMART", rather than - # any garbage entry with any source bypassing both the cap and the - # #4482 need-check. - zero_kwh_exempt = (kwh <= 0) and (source == "SMART") and not self.octopus_slot_count_zero_kwh + # Scoped to source == "SMART" and kwh_valid (#4483 review follow-up, + # Speshman): kwh only ever reaches 0 either from a genuine zero-kWh entry + # or from decode_octopus_slot() failing to parse charge_in_kwh - the two + # are indistinguishable by value alone, and a malformed entry can carry + # source=="SMART" too. Requiring kwh_valid closes that gap entirely, + # regardless of source, rather than just narrowing it to a coincidence. + zero_kwh_exempt = (kwh <= 0) and kwh_valid and (source == "SMART") and not self.octopus_slot_count_zero_kwh # At the start of each 30-min slot, decide if we can add it if minute % 30 == 0: diff --git a/apps/predbat/tests/test_rate_add_io_slots.py b/apps/predbat/tests/test_rate_add_io_slots.py index 861e0c393..2cf795cd9 100644 --- a/apps/predbat/tests/test_rate_add_io_slots.py +++ b/apps/predbat/tests/test_rate_add_io_slots.py @@ -614,6 +614,18 @@ def run_rate_add_io_slots_tests(my_predbat): expected_rates_30 = {minute: 10.0 for minute in range(slot_start_minute_30, slot_start_minute_30 + 30)} # rejected, not needed, not exempt failed |= run_rate_add_io_slots_test("test30_zero_kwh_non_smart_source_not_exempt", my_predbat, slots_30, True, 12, expected_rates_30) + # Test 31 (#4807): a malformed charge_in_kwh (not a genuine zero-kWh event) must not get the + # exemption even when its source is "SMART" - decode_octopus_slot() coerces the unparseable + # value to kwh==0 the same way a real zero-kWh entry would, so source alone can't tell them + # apart. kwh_valid closes that gap. + print("\n**** Test 31: Malformed charge_in_kwh with source SMART is not exempt ****") + slot_start_31 = midnight_utc + timedelta(hours=14) # future, out-of-window + slot_end_31 = slot_start_31 + timedelta(minutes=30) + slots_31 = [{"start": slot_start_31.strftime(TIME_FORMAT), "end": slot_end_31.strftime(TIME_FORMAT), "charge_in_kwh": "not-a-number", "source": "SMART", "location": ""}] + slot_start_minute_31 = int((slot_start_31 - midnight_utc).total_seconds() / 60) + expected_rates_31 = {minute: 10.0 for minute in range(slot_start_minute_31, slot_start_minute_31 + 30)} # rejected, not needed, not exempt + failed |= run_rate_add_io_slots_test("test31_malformed_smart_kwh_not_exempt", my_predbat, slots_31, True, 12, expected_rates_31) + my_predbat.octopus_slot_count_zero_kwh = True # Restore default for any subsequent tests my_predbat.octopus_intelligent_limit_future_slots = False # Restore default for any subsequent tests my_predbat.car_charging_slots[0] = saved_car_charging_slots From 239551575661f9241b9a1cb1b6a745e1266632dc Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 28 Aug 2026 18:55:21 +0100 Subject: [PATCH 2/2] fix(octopus): compare a dispatch's real start, not its rounded settlement block, for "already underway" rate_add_io_slots()'s exemption from the #4482 future-need check compared the 30-min-rounded slot_start against a similarly rounded current_block, so a dispatch starting partway through the current settlement period (e.g. 10:20, checked at 10:05) read as already-underway from the top of that period (10:00), before it had actually started. Captures the dispatch's real, unrounded start separately and compares that against minutes_now instead - current_block is no longer needed and is removed. #4808, review follow-up on #4483 from Speshman. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/octopus.py | 14 ++++++++++++-- apps/predbat/tests/test_rate_add_io_slots.py | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/apps/predbat/octopus.py b/apps/predbat/octopus.py index 3418fd1ba..32215acee 100644 --- a/apps/predbat/octopus.py +++ b/apps/predbat/octopus.py @@ -3072,7 +3072,6 @@ def rate_add_io_slots(self, car_n, rates, octopus_slots): slots_added_set = set() plan_interval_minutes = self.plan_interval_minutes saved_slots = set() # For logging purposes, track which slots we actually applied as low rate - current_block = (self.minutes_now // 30) * 30 # #4482: Octopus often grants more daytime dispatch slots than the car actually needs - it # can't see the car's real SoC, only Predbat can (car_charging_soc/car_charging_limit). @@ -3095,6 +3094,11 @@ def rate_add_io_slots(self, car_n, rates, octopus_slots): # Add in IO slots for slot in octopus_slots: start_minutes, end_minutes, kwh, source, location, kwh_valid = self.decode_octopus_slot(car_n, slot, raw=True) + # The dispatch's real (unrounded) start, kept apart from the 30-min-rounded + # start_minutes below - used only for the "already underway" check (#4808) so a + # dispatch starting partway through the current settlement period isn't treated as + # already started from the top of that period. + raw_start_minutes = start_minutes # Ignore bump-charge slots as their cost won't change if source != "bump-charge" and source != "BOOST" and (not location or location == "AT_HOME"): @@ -3138,7 +3142,13 @@ def rate_add_io_slots(self, car_n, rates, octopus_slots): # slot already underway or completed is trusted regardless of what # car_charging_slots now says about future need, and the fixed window is # never affected since it's guaranteed cheap by the tariff itself. - needed = (not limit_future_slots) or (slot_start <= current_block) or (slot_start in expected_blocks) or self.minute_in_iog_fixed_window(slot_start) + # + # "Already underway" compares the dispatch's real start (#4808, review + # follow-up on #4483 from Speshman) rather than its 30-min-rounded + # slot_start, so a dispatch starting partway through the current + # settlement period isn't trusted early just because the period itself has + # begun. + needed = (not limit_future_slots) or (raw_start_minutes <= self.minutes_now) or (slot_start in expected_blocks) or self.minute_in_iog_fixed_window(slot_start) # Whether this dispatch entry actually delivers charge to the car. A # zero-kWh entry (e.g. a plug-independent SMART grid-flex event - #4483 diff --git a/apps/predbat/tests/test_rate_add_io_slots.py b/apps/predbat/tests/test_rate_add_io_slots.py index 2cf795cd9..2df22c109 100644 --- a/apps/predbat/tests/test_rate_add_io_slots.py +++ b/apps/predbat/tests/test_rate_add_io_slots.py @@ -409,6 +409,22 @@ def run_rate_add_io_slots_tests(my_predbat): expected_rates_21 = {minute: 4.0 for minute in range(570, 600)} # 09:30-10:00 failed |= run_rate_add_io_slots_test("test21_current_dispatch_stays_low_rate", my_predbat, slots_21, True, 12, expected_rates_21) + # Test 21b (#4808, review follow-up on #4483 from Speshman): a dispatch starting partway + # through the current settlement period must not be treated as already underway just because + # the period itself has started - only compares the dispatch's real start against minutes_now, + # not the 30-min-rounded slot_start against the rounded current block. + print("\n**** Test 21b: A dispatch starting later in the current period is not yet underway ****") + my_predbat.octopus_intelligent_limit_future_slots = True + now_minutes_21b = my_predbat.minutes_now # whatever "now" actually is - built relative to it, not a hardcoded wall-clock time + slot_start_21b = midnight_utc + timedelta(minutes=now_minutes_21b + 20) # 20 minutes into the current period - genuinely still future + slot_end_21b = slot_start_21b + timedelta(minutes=30) + slots_21b = [{"start": slot_start_21b.strftime(TIME_FORMAT), "end": slot_end_21b.strftime(TIME_FORMAT), "charge_in_kwh": 2.5, "source": "smart-charge", "location": "AT_HOME"}] + my_predbat.car_charging_slots[0] = [] # Car doesn't need it + rounded_start_21b = (now_minutes_21b // 30) * 30 + rounded_end_21b = ((now_minutes_21b + 20 + 30 + 29) // 30) * 30 + expected_rates_21b = {minute: 10.0 for minute in range(rounded_start_21b, rounded_end_21b)} # rejected, not yet started, not needed + failed |= run_rate_add_io_slots_test("test21b_mid_period_future_dispatch_not_yet_underway", my_predbat, slots_21b, True, 12, expected_rates_21b) + # Test 22: feature disabled - existing (unconditional) behaviour is unchanged even with an # empty car_charging_slots that would otherwise have excluded every block. print("\n**** Test 22: Switch off restores unconditional low rate ****")