Skip to content

Commit 84c9380

Browse files
Merge pull request #5086 from springfall2008/fix/component-minutes-now
fix(component_base): stop components reading calculate_yesterday's faked minutes_now
2 parents 33d0ffd + d4eb4b6 commit 84c9380

6 files changed

Lines changed: 166 additions & 15 deletions

File tree

‎apps/predbat/component_base.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
from abc import ABC, abstractmethod
2222
from datetime import datetime, timezone
23+
24+
from utils import minutes_since_midnight
2325
import asyncio
2426
import time
2527
import traceback
@@ -179,8 +181,23 @@ def now_utc_exact(self):
179181

180182
@property
181183
def minutes_now(self):
182-
"""Get the current time in minutes since midnight"""
183-
return self.base.minutes_now
184+
"""Get the current time in minutes since midnight
185+
186+
Derived from the base's now_utc for the same reason as midnight_utc: calculate_yesterday()
187+
(output.py) fakes the shared base.minutes_now to 0 for the duration of the savings
188+
calculation, and a component reading it mid-rewind reads 0 - which, unlike a rewound date,
189+
looks like a perfectly legitimate "just after midnight" (GH#4804).
190+
191+
This is the same calculation update_time() makes, through the same helper, so the value is
192+
identical to base.minutes_now outside that window.
193+
194+
now_utc is snapshotted rather than read twice (once here, once through self.midnight_utc):
195+
update_time() runs on the main thread and can replace it between the two reads, which at a
196+
day boundary would subtract the new day's midnight from the old timestamp and return a
197+
negative minute.
198+
"""
199+
now_utc = self.base.now_utc
200+
return minutes_since_midnight(now_utc, now_utc.replace(hour=0, minute=0, second=0, microsecond=0))
184201

185202
@property
186203
def plan_interval_minutes(self):

‎apps/predbat/tests/test_alphaess_api.py‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import predbat # noqa: F401 (import first - avoids circular import: config.py does `from predbat import THIS_VERSION`)
1212
import hashlib
1313
import pytz
14-
from datetime import datetime
14+
from datetime import datetime, timedelta
1515
from unittest.mock import AsyncMock, MagicMock, patch
1616
from alphaess import AlphaESSAPI
1717
from tests.test_infra import run_async as run_async_local, create_aiohttp_mock_response, create_aiohttp_mock_session
@@ -27,7 +27,11 @@ def __init__(self, app_id="alphatestappid00000", app_secret="secret0000000000",
2727
self.local_tz = pytz.timezone("Europe/London")
2828
self.base = MagicMock()
2929
self.base.args = {"user_id": "test-alphaess-1"}
30-
self.base.midnight_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
30+
# A real clock, not a MagicMock attribute: ComponentBase.minutes_now derives from
31+
# base.now_utc (GH#4804), so the mock base has to carry a coherent now_utc/midnight_utc
32+
# pair. set_mock_clock() below moves it; writing base.minutes_now alone does nothing.
33+
self.base.now_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
34+
self.base.midnight_utc = self.base.now_utc.replace(hour=0, minute=0, second=0, microsecond=0)
3135
self.base.minutes_now = 0
3236
self.state = {}
3337
self.published = {}
@@ -46,6 +50,16 @@ def __init__(self, app_id="alphatestappid00000", app_secret="secret0000000000",
4650
api_delay=0,
4751
)
4852

53+
def set_mock_clock(self, minutes_now):
54+
"""Move the mock base's clock to minutes_now past midnight.
55+
56+
ComponentBase.minutes_now is derived from base.now_utc, so a test that wants a particular
57+
time of day has to move the clock rather than write base.minutes_now (GH#4804). Both are
58+
set here so a direct base.minutes_now read stays honest too.
59+
"""
60+
self.base.now_utc = self.base.midnight_utc + timedelta(minutes=minutes_now)
61+
self.base.minutes_now = minutes_now
62+
4963
def log(self, message):
5064
"""Capture logs."""
5165
self.log_messages.append(message)

‎apps/predbat/tests/test_alphaess_control.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ def test_alphaess_disabled_planned_hold_recovers_the_charge_window():
379379
"""
380380
failed = False
381381
client = _client()
382-
client.base.minutes_now = 2 * 60
382+
client.set_mock_clock(2 * 60)
383383
schedule = _schedule(reserve=41, charge={"enable": False, "soc": 40, "power": 3000, "start": "01:00:00", "end": "05:00:00"})
384384
charge = client.build_charge_payload("AL70", schedule)
385385
discharge = client.build_discharge_payload("AL70", schedule)
@@ -404,10 +404,10 @@ def test_alphaess_planned_hold_releases_when_its_window_ends():
404404
"""
405405
failed = False
406406
client = _client()
407-
client.base.minutes_now = 2 * 60
407+
client.set_mock_clock(2 * 60)
408408
schedule = _schedule(reserve=41, charge={"enable": False, "soc": 40, "power": 3000, "start": "01:00:00", "end": "05:00:00"})
409409
first = client.build_charge_payload("AL70", schedule)
410-
client.base.minutes_now = 6 * 60
410+
client.set_mock_clock(6 * 60)
411411
second = client.build_charge_payload("AL70", schedule)
412412
if first.get("gridCharge") != 1:
413413
print(f"ERROR: test setup did not enter the recovered hold: {first}")
@@ -441,7 +441,7 @@ def test_alphaess_real_charge_survives_a_simultaneous_discharge_hold():
441441
"""An EV/iBoost hold during real grid charging must not lower the charge target."""
442442
failed = False
443443
client = _client()
444-
client.base.minutes_now = 2 * 60
444+
client.set_mock_clock(2 * 60)
445445
schedule = _schedule(reserve=10, charge={"enable": True, "soc": 90, "power": 3000, "start": "01:00:00", "end": "05:00:00"}, export_power=0)
446446
charge = client.build_charge_payload("AL70", schedule)
447447
discharge = client.build_discharge_payload("AL70", schedule)
@@ -463,7 +463,7 @@ def test_alphaess_future_charge_does_not_satisfy_a_current_discharge_hold():
463463
"""
464464
failed = False
465465
client = _client()
466-
client.base.minutes_now = 22 * 60
466+
client.set_mock_clock(22 * 60)
467467
schedule = _schedule(reserve=10, charge={"enable": True, "soc": 90, "power": 3000, "start": "23:00:00", "end": "23:45:00"}, export_power=0)
468468
charge = client.build_charge_payload("AL70", schedule)
469469
discharge = client.build_discharge_payload("AL70", schedule)

‎apps/predbat/tests/test_component_base.py‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,94 @@ def test_component_base_midnight_utc_ignores_rewound_base(my_predbat):
524524
return False
525525

526526

527+
def test_component_base_minutes_now_follows_update_time(my_predbat):
528+
"""
529+
Test ComponentBase.minutes_now matches update_time()'s own value and ignores the faked one (GH#4804).
530+
531+
calculate_yesterday() (output.py) fakes base.minutes_now to 0 alongside its midnight_utc rewind.
532+
0 is the nastier of the two, since it reads as a legitimate "just after midnight" rather than an
533+
obviously wrong date - it would tell octopus.py to keep every expired dispatch slot, and the
534+
AlphaESS/Deye/Sunsynk adapters that pick the live TOU slot by time of day to believe it is
535+
midnight while writing to a real inverter.
536+
537+
The derived value has to agree with update_time()'s (predbat.py), including its PREDICT_STEP
538+
flooring - so the first half of this test compares the two on the real base rather than
539+
restating the formula, and would catch the two drifting apart later.
540+
"""
541+
print("\n*** Test: ComponentBase.minutes_now follows update_time and ignores the faked value ***")
542+
543+
# The fixture pins its own clock (unit_test.py's create_predbat) and the whole suite shares this
544+
# instance, so every field update_time() writes has to go back - not just the ones read here, or
545+
# a later test inherits this one's wall-clock state.
546+
saved = {name: getattr(my_predbat, name) for name in ("now_utc", "now_utc_real", "midnight_utc", "minutes_now", "minutes_to_midnight", "difference_minutes", "local_tz")}
547+
try:
548+
my_predbat.update_time(print=False)
549+
component = TestComponent(my_predbat)
550+
assert component.minutes_now == my_predbat.minutes_now, f"Derived {component.minutes_now} but update_time() computed {my_predbat.minutes_now}"
551+
assert component.minutes_now % 5 == 0, f"Should stay on a PREDICT_STEP boundary, got {component.minutes_now}"
552+
finally:
553+
for name, value in saved.items():
554+
setattr(my_predbat, name, value)
555+
556+
base = MockBase()
557+
base.now_utc = datetime(2025, 6, 15, 14, 37, 0, tzinfo=timezone.utc)
558+
base.midnight_utc = base.now_utc.replace(hour=0, minute=0, second=0, microsecond=0)
559+
base.minutes_now = 14 * 60 + 35
560+
component = TestComponent(base)
561+
562+
assert component.minutes_now == 14 * 60 + 35, f"Expected 14:35 floored to a PREDICT_STEP boundary, got {component.minutes_now}"
563+
564+
# A concurrent calculate_yesterday() is mid-flight: the shared fields say midnight yesterday.
565+
base.minutes_now = 0
566+
base.midnight_utc = base.midnight_utc - timedelta(days=1)
567+
assert component.minutes_now == 14 * 60 + 35, f"Faked base.minutes_now leaked into the component: {component.minutes_now}"
568+
569+
print("PASS: minutes_now follows update_time's computation and ignores the faked value")
570+
return False
571+
572+
573+
def test_component_base_minutes_now_snapshots_the_clock(my_predbat):
574+
"""
575+
Test minutes_now survives update_time() landing mid-read.
576+
577+
The value needs now_utc and the midnight derived from it, and self.midnight_utc goes back to
578+
base.now_utc for its own read - so a naive implementation reads the field twice. update_time()
579+
runs on the main thread while components run on theirs, so those two reads can straddle it: at
580+
a day boundary that subtracts the new day's midnight from the old day's timestamp and collapses
581+
23:59 to 0, which reads as midnight rather than as an error.
582+
583+
A base whose now_utc advances a day between reads reproduces that deterministically, without
584+
needing two threads to interleave.
585+
"""
586+
print("\n*** Test: ComponentBase.minutes_now snapshots now_utc ***")
587+
588+
class RollingClockBase(MockBase):
589+
"""A base whose clock rolls into the next day between one read of now_utc and the next."""
590+
591+
def __init__(self):
592+
"""Start the day before, with a read counter."""
593+
super().__init__()
594+
self.reads = 0
595+
596+
@property
597+
def now_utc(self):
598+
"""Return 23:59 on the first read and the next day's noon on every read after it."""
599+
self.reads += 1
600+
if self.reads == 1:
601+
return datetime(2025, 6, 15, 23, 59, 0, tzinfo=timezone.utc)
602+
return datetime(2025, 6, 16, 12, 0, 0, tzinfo=timezone.utc)
603+
604+
base = RollingClockBase()
605+
component = TestComponent(base)
606+
607+
minutes_now = component.minutes_now
608+
assert base.reads == 1, f"now_utc should be read once per call, was read {base.reads} times"
609+
assert minutes_now == 23 * 60 + 55, f"Expected the first read's 23:59 floored to 23:55, got {minutes_now}"
610+
611+
print("PASS: minutes_now reads the clock once and stays on that reading")
612+
return False
613+
614+
527615
def test_component_base_all(my_predbat):
528616
"""Run all component_base tests"""
529617
tests = [
@@ -539,6 +627,8 @@ def test_component_base_all(my_predbat):
539627
("set_arg_auto_keep", test_component_base_set_arg_auto_keeps_user_setting, "set_arg_auto(overwrite=False) keeps an explicit apps.yaml setting"),
540628
("set_state_external", test_component_base_set_state_external, "set_state_external forwards to the HA interface"),
541629
("midnight_utc_rewound", test_component_base_midnight_utc_ignores_rewound_base, "midnight_utc ignores a rewound base.midnight_utc"),
630+
("minutes_now_derived", test_component_base_minutes_now_follows_update_time, "minutes_now follows update_time and ignores the faked value"),
631+
("minutes_now_snapshot", test_component_base_minutes_now_snapshots_the_clock, "minutes_now snapshots now_utc rather than reading it twice"),
542632
]
543633

544634
failed = []

‎apps/predbat/tests/test_deye_api.py‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import predbat # noqa: F401 (import first - avoids circular import: config.py does `from predbat import THIS_VERSION`)
1212
import pytz
13-
from datetime import datetime
13+
from datetime import datetime, timedelta
1414
from unittest.mock import MagicMock, patch
1515
from deye import DeyeAPI
1616
from deye_const import DEYE_BASE_URLS, DEYE_TELEMETRY_KEYS, CONFIG_BATTERY_KEYS
@@ -62,8 +62,13 @@ def __init__(self, auth_method="app_credentials", data_center="eu", inverter_sn=
6262
self.local_tz = pytz.timezone("Europe/London")
6363
self.base = MagicMock()
6464
self.base.args = {"user_id": "test-deye-1"}
65-
self.base.midnight_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
66-
self.base.minutes_now = 0 # local minutes-since-midnight; tests set this for time-aware control
65+
# A real clock on the mock base, not a MagicMock attribute: ComponentBase.minutes_now
66+
# derives from base.now_utc (GH#4804), and MagicMock would answer that arithmetic with a
67+
# value of its own rather than failing. Tests wanting a particular time of day call
68+
# set_mock_clock(); writing base.minutes_now alone does nothing.
69+
self.base.now_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
70+
self.base.midnight_utc = self.base.now_utc.replace(hour=0, minute=0, second=0, microsecond=0)
71+
self.base.minutes_now = 0
6772
self._init_oauth(auth_method, "test-token", None, "deye")
6873

6974
def with_rating(self, *serials, watts=MOCK_RATED_POWER):
@@ -72,6 +77,16 @@ def with_rating(self, *serials, watts=MOCK_RATED_POWER):
7277
self.device_rated_power[sn] = float(watts)
7378
return self
7479

80+
def set_mock_clock(self, minutes_now):
81+
"""Move the mock base's clock to minutes_now past midnight.
82+
83+
ComponentBase.minutes_now is derived from base.now_utc, so a test that wants a particular
84+
time of day moves the clock rather than writing base.minutes_now (GH#4804). Both are set
85+
so a direct base.minutes_now read stays honest too.
86+
"""
87+
self.base.now_utc = self.base.midnight_utc + timedelta(minutes=minutes_now)
88+
self.base.minutes_now = minutes_now
89+
7590
def log(self, message):
7691
"""Capture logs."""
7792
self.log_messages.append(message)

‎apps/predbat/tests/test_sunsynk_api.py‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import aiohttp
1717
import pytz
1818
from contextlib import redirect_stdout
19-
from datetime import datetime
19+
from datetime import datetime, timedelta
2020
from unittest.mock import MagicMock, AsyncMock, patch
2121
from sunsynk import SunsynkAPI, test_sunsynk_api as run_sunsynk_cli_once
2222
from sunsynk_const import SUNSYNK_REGIONS, SUNSYNK_ENDPOINTS, SUNSYNK_RETRIES, SUNSYNK_MAX_DISCOVERY_PAGES
@@ -60,12 +60,27 @@ def __init__(self, auth_method="password", region="sunsynk", inverter_sn=None, c
6060
self.local_tz = pytz.timezone("Europe/London")
6161
self.base = MagicMock()
6262
self.base.args = {"user_id": "test-sunsynk-1"}
63-
self.base.midnight_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
64-
self.base.minutes_now = 0 # local minutes-since-midnight; tests set this for time-aware control
63+
# A real clock on the mock base, not a MagicMock attribute: ComponentBase.minutes_now
64+
# derives from base.now_utc (GH#4804), and MagicMock would answer that arithmetic with a
65+
# value of its own rather than failing. Tests wanting a particular time of day call
66+
# set_mock_clock(); writing base.minutes_now alone does nothing.
67+
self.base.now_utc = datetime.now(pytz.utc).replace(hour=0, minute=0, second=0, microsecond=0)
68+
self.base.midnight_utc = self.base.now_utc.replace(hour=0, minute=0, second=0, microsecond=0)
69+
self.base.minutes_now = 0
6570
# Straight through, like the component: _init_oauth owns self.auth_method, so
6671
# collapsing the modes here would hide password_legacy from fetch_token.
6772
self._init_oauth(auth_method, "test-token", None, "sunsynk")
6873

74+
def set_mock_clock(self, minutes_now):
75+
"""Move the mock base's clock to minutes_now past midnight.
76+
77+
ComponentBase.minutes_now is derived from base.now_utc, so a test that wants a particular
78+
time of day moves the clock rather than writing base.minutes_now (GH#4804). Both are set
79+
so a direct base.minutes_now read stays honest too.
80+
"""
81+
self.base.now_utc = self.base.midnight_utc + timedelta(minutes=minutes_now)
82+
self.base.minutes_now = minutes_now
83+
6984
def log(self, message):
7085
"""Capture logs."""
7186
self.log_messages.append(message)

0 commit comments

Comments
 (0)