From 7061993c81608a995454f1d70c61fa7d583e222f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 17:35:29 +0000 Subject: [PATCH 1/8] Add daily hard price refresh at configurable UTC time (issue #366) Dynamic tariff data (e.g. EPEX Spot next-day prices) is published around 12:00-13:00 UTC. The existing hourly fail-safe scheduler may not pick up fresh data promptly. This change adds a dedicated daily hard refresh that bypasses the cache and always fetches from the provider. Changes: - dynamictariff/baseclass.py: Add force=True parameter to refresh_data() to bypass next_update_ts and always fetch fresh data. - scheduler.py: Add optional tz parameter to schedule_at() / SchedulerThread to support timezone-aware daily scheduling via schedule library. - core.py: Read market_price_refresh_time from battery_control_expert config (default "12:30" UTC) and schedule a daily _hard_refresh_prices() call. - config/batcontrol_config_dummy.yaml: Add commented market_price_refresh_time option under battery_control_expert. - Tests: Cover force refresh behaviour, tz-aware schedule_at, and core wiring. --- config/batcontrol_config_dummy.yaml | 3 + src/batcontrol/core.py | 24 +++++++ src/batcontrol/dynamictariff/baseclass.py | 13 +++- src/batcontrol/scheduler.py | 19 ++++-- .../dynamictariff/test_baseclass.py | 46 ++++++++++++- tests/batcontrol/test_core.py | 67 +++++++++++++++++++ tests/batcontrol/test_scheduler.py | 24 +++++++ 7 files changed, 187 insertions(+), 9 deletions(-) diff --git a/config/batcontrol_config_dummy.yaml b/config/batcontrol_config_dummy.yaml index 1a1c04d..474a3c0 100644 --- a/config/batcontrol_config_dummy.yaml +++ b/config/batcontrol_config_dummy.yaml @@ -34,6 +34,9 @@ battery_control_expert: production_offset_percent: 1.0 # Adjust production forecast by a percentage (1.0 = 100%, 0.8 = 80%, etc.) # Useful for winter mode when solar panels are covered with snow preserve_min_grid_charge_soc: false # If true, also preserve min_grid_charge_soc as reserved energy during cheap/pre-expensive slots + # market_price_refresh_time: "12:30" # UTC time for a daily hard price refresh. + # Use this to pick up newly published next-day prices (e.g. EPEX Spot ~12:00 UTC). + # Format: "HH:MM". Default: "12:30". #-------------------------- # Peak Shaving diff --git a/src/batcontrol/core.py b/src/batcontrol/core.py index 2aba98e..728b542 100644 --- a/src/batcontrol/core.py +++ b/src/batcontrol/core.py @@ -249,6 +249,7 @@ def __init__(self, configdict: dict): self.round_price_digits = 4 self.production_offset_percent = 1.0 # Default: no offset + self.market_price_refresh_time = "12:30" if self.config.get('battery_control_expert', None) is not None: battery_control_expert = self.config.get( @@ -262,6 +263,9 @@ def __init__(self, configdict: dict): self.preserve_min_grid_charge_soc = battery_control_expert.get( 'preserve_min_grid_charge_soc', self.preserve_min_grid_charge_soc) + self.market_price_refresh_time = battery_control_expert.get( + 'market_price_refresh_time', + self.market_price_refresh_time) self.general_logic = CommonLogic.get_instance( charge_rate_multiplier=self.batconfig.get( @@ -381,6 +385,14 @@ def __init__(self, configdict: dict): 'hours', self.fc_consumption.refresh_data, 'forecast-consumption-every') + # Hard refresh at market publish time (default 12:30 UTC) to pick up + # newly published next-day prices (e.g. EPEX spot). + self.scheduler.schedule_at( + self.market_price_refresh_time, + self._hard_refresh_prices, + 'market-price-hard-refresh', + tz='UTC' + ) # Run initial data fetch try: self.fc_solar.refresh_data() @@ -406,6 +418,18 @@ def shutdown(self): except Exception as exc: logger.exception("Error during Batcontrol shutdown: %s", exc) + def _hard_refresh_prices(self) -> None: + """Force a price refresh regardless of cache state. + + Called at market_price_refresh_time (UTC) to pick up newly published + dynamic tariff data (e.g. EPEX next-day prices published ~12:00 UTC). + """ + logger.info( + 'Hard price refresh at %s UTC - fetching fresh market prices', + self.market_price_refresh_time + ) + self.dynamic_tariff.refresh_data(force=True) + def reset_forecast_error(self): """ Reset the forecast error timer """ self.time_at_forecast_error = -1 diff --git a/src/batcontrol/dynamictariff/baseclass.py b/src/batcontrol/dynamictariff/baseclass.py index fede30b..35bd6da 100644 --- a/src/batcontrol/dynamictariff/baseclass.py +++ b/src/batcontrol/dynamictariff/baseclass.py @@ -85,11 +85,18 @@ def store_raw_data(self, data: dict) -> None: """Store raw data in cache.""" self.cache.store_new_entry(data) - def refresh_data(self) -> None: - """Refresh data from provider if needed.""" + def refresh_data(self, force: bool = False) -> None: + """Refresh data from provider if needed. + + Args: + force: When True, bypass next_update_ts and always fetch fresh data. + Use for scheduled market-publish refreshes (e.g. 12:30 UTC). + """ with self._refresh_data_lock: now = time.time() - if now > self.next_update_ts: + if force: + logger.info('%s: Forced price refresh triggered', self.__class__.__name__) + if force or now > self.next_update_ts: # Not on initial call if self.next_update_ts > 0 and self.delay_evaluation_by_seconds > 0: sleeptime = random.randrange(0, self.delay_evaluation_by_seconds, 1) diff --git a/src/batcontrol/scheduler.py b/src/batcontrol/scheduler.py index 34c4da8..79ef837 100644 --- a/src/batcontrol/scheduler.py +++ b/src/batcontrol/scheduler.py @@ -98,7 +98,8 @@ def wrapped_job(): return obtained_unit.do(wrapped_job) -def schedule_at(time_str: str, job: Callable, job_name: str = ""): +def schedule_at(time_str: str, job: Callable, job_name: str = "", + tz: Optional[str] = None): """ Schedule a job to run at a specific time each day (globally accessible) @@ -106,12 +107,15 @@ def schedule_at(time_str: str, job: Callable, job_name: str = ""): time_str: Time string in HH:MM format (e.g., "14:30") job: The callable function to execute job_name: Optional name for the job (for logging purposes) + tz: Optional timezone name (e.g., "UTC", "Europe/Berlin"). + When None, the server's local time is used. Returns: The scheduled job object """ name = job_name or job.__name__ - logger.info("Scheduling job '%s' to run daily at %s", name, time_str) + tz_label = tz if tz else "local" + logger.info("Scheduling job '%s' to run daily at %s %s", name, time_str, tz_label) # Wrap the job to catch exceptions and add logging def wrapped_job(): @@ -122,7 +126,10 @@ def wrapped_job(): except Exception as e: logger.error("Error in scheduled job '%s': %s", name, e, exc_info=True) - return _get_job_registry().every().day.at(time_str).do(wrapped_job) + job_def = _get_job_registry().every().day + if tz is not None: + return job_def.at(time_str, tz).do(wrapped_job) + return job_def.at(time_str).do(wrapped_job) def schedule_once(time: str, job: Callable, job_name: str = ""): @@ -268,7 +275,8 @@ def schedule_every(self, interval: int, unit: str, job: Callable, job_name: str """ return schedule_every(interval, unit, job, job_name) - def schedule_at(self, time_str: str, job: Callable, job_name: str = ""): + def schedule_at(self, time_str: str, job: Callable, job_name: str = "", + tz: Optional[str] = None): """ Schedule a job to run at a specific time each day @@ -279,11 +287,12 @@ def schedule_at(self, time_str: str, job: Callable, job_name: str = ""): time_str: Time string in HH:MM format (e.g., "14:30") job: The callable function to execute job_name: Optional name for the job (for logging purposes) + tz: Optional timezone name (e.g., "UTC"). None uses server local time. Returns: The scheduled job object """ - return schedule_at(time_str, job, job_name) + return schedule_at(time_str, job, job_name, tz) def schedule_once(self, time: str, job: Callable, job_name: str = ""): """ diff --git a/tests/batcontrol/dynamictariff/test_baseclass.py b/tests/batcontrol/dynamictariff/test_baseclass.py index 56b4284..15476b2 100644 --- a/tests/batcontrol/dynamictariff/test_baseclass.py +++ b/tests/batcontrol/dynamictariff/test_baseclass.py @@ -1,10 +1,11 @@ """ Test module for DynamicTariffBaseclass and providers """ +import time import pytest import pytz from datetime import datetime, timezone as dt_timezone -from unittest.mock import patch +from unittest.mock import patch, MagicMock from batcontrol.dynamictariff.baseclass import DynamicTariffBaseclass @@ -202,6 +203,49 @@ def test_replicate_hourly_to_15min_method(self, baseclass_15min_target): idx = h * 4 + q assert result[idx] == hourly[h] + def test_refresh_data_force_bypasses_next_update_ts(self, timezone): + """force=True must fetch even when next_update_ts is in the future.""" + instance = ConcreteTariffProvider(timezone) + instance.get_raw_data_from_provider = MagicMock(return_value={}) + instance.delay_evaluation_by_seconds = 0 + + # Simulate data already fresh: next update far in the future + instance.next_update_ts = time.time() + 9999 + + instance.refresh_data(force=True) + + instance.get_raw_data_from_provider.assert_called_once() + + def test_refresh_data_no_force_respects_next_update_ts(self, timezone): + """Without force, refresh_data must skip fetch when cache is still valid.""" + instance = ConcreteTariffProvider(timezone) + instance.get_raw_data_from_provider = MagicMock(return_value={}) + + # Simulate data already fresh + instance.next_update_ts = time.time() + 9999 + + instance.refresh_data(force=False) + + instance.get_raw_data_from_provider.assert_not_called() + + def test_refresh_data_force_updates_next_update_ts(self, timezone): + """After a forced refresh, next_update_ts must be pushed forward.""" + instance = ConcreteTariffProvider(timezone) + instance.get_raw_data_from_provider = MagicMock(return_value={}) + instance.delay_evaluation_by_seconds = 0 + + old_ts = time.time() + 9999 + instance.next_update_ts = old_ts + + before = time.time() + instance.refresh_data(force=True) + after = time.time() + + # next_update_ts must be reset to now + min_time_between_updates + expected_min = before + instance.min_time_between_updates + expected_max = after + instance.min_time_between_updates + assert expected_min <= instance.next_update_ts <= expected_max + class TestAwattarProvider: """Tests for Awattar provider""" diff --git a/tests/batcontrol/test_core.py b/tests/batcontrol/test_core.py index 4204db3..31c0073 100644 --- a/tests/batcontrol/test_core.py +++ b/tests/batcontrol/test_core.py @@ -957,5 +957,72 @@ def test_evcc_no_limit_active_no_change( assert calc_params.peak_shaving.enabled is False +class TestMarketPriceRefresh: + """Tests for the configurable market price hard refresh (issue #366).""" + + BASE_CONFIG = { + 'timezone': 'Europe/Berlin', + 'time_resolution_minutes': 60, + 'inverter': { + 'type': 'dummy', + 'max_grid_charge_rate': 5000, + 'max_pv_charge_rate': 3000, + 'min_pv_charge_rate': 0, + }, + 'utility': {'type': 'tibber', 'apikey': 'test_token'}, + 'pvinstallations': [], + 'consumption_forecast': {'type': 'simple', 'value': 500}, + 'battery_control': { + 'max_charging_from_grid_limit': 0.8, + 'min_price_difference': 0.05, + }, + 'mqtt': {'enabled': False}, + } + + def _patch_core(self, mocker): + mock_inverter = mocker.MagicMock() + mock_inverter.max_pv_charge_rate = 3000 + mock_inverter.get_max_capacity.return_value = 10000 + mocker.patch('batcontrol.core.tariff_factory.create_tarif_provider', + autospec=True, return_value=mocker.MagicMock()) + mocker.patch('batcontrol.core.inverter_factory.create_inverter', + autospec=True, return_value=mock_inverter) + mocker.patch('batcontrol.core.solar_factory.create_solar_provider', + autospec=True, return_value=mocker.MagicMock()) + mocker.patch('batcontrol.core.consumption_factory.create_consumption', + autospec=True, return_value=mocker.MagicMock()) + + def test_default_market_price_refresh_time(self, mocker): + """Without expert config, market_price_refresh_time defaults to 12:30.""" + self._patch_core(mocker) + config = dict(self.BASE_CONFIG) + bc = Batcontrol(config) + assert bc.market_price_refresh_time == "12:30" + bc.shutdown() + + def test_custom_market_price_refresh_time_from_expert_config(self, mocker): + """market_price_refresh_time from battery_control_expert is used.""" + self._patch_core(mocker) + config = dict(self.BASE_CONFIG) + config['battery_control_expert'] = {'market_price_refresh_time': '13:00'} + bc = Batcontrol(config) + assert bc.market_price_refresh_time == "13:00" + bc.shutdown() + + def test_hard_refresh_prices_calls_force_refresh(self, mocker): + """_hard_refresh_prices() must call dynamic_tariff.refresh_data(force=True).""" + self._patch_core(mocker) + config = dict(self.BASE_CONFIG) + bc = Batcontrol(config) + + mock_refresh = mocker.MagicMock() + bc.dynamic_tariff.refresh_data = mock_refresh + + bc._hard_refresh_prices() + + mock_refresh.assert_called_once_with(force=True) + bc.shutdown() + + if __name__ == '__main__': pytest.main([__file__, '-v']) diff --git a/tests/batcontrol/test_scheduler.py b/tests/batcontrol/test_scheduler.py index 9072f3a..24d8865 100644 --- a/tests/batcontrol/test_scheduler.py +++ b/tests/batcontrol/test_scheduler.py @@ -57,3 +57,27 @@ def test_schedule_every_rejects_invalid_unit(): with pytest.raises(ValueError): scheduler.schedule_every(1, "fortnights", _noop, "bad-unit") + + +def test_schedule_at_registers_daily_job(): + """schedule_at() must register exactly one job.""" + scheduler.reset_scheduler() + scheduler.schedule_at("12:30", _noop, "daily-job") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() + + +def test_schedule_at_with_utc_timezone(): + """schedule_at() with tz='UTC' must register a job without raising.""" + scheduler.reset_scheduler() + scheduler.schedule_at("12:30", _noop, "utc-job", tz="UTC") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() + + +def test_schedule_at_without_tz_uses_local_time(): + """schedule_at() without tz must register a job (uses server local time).""" + scheduler.reset_scheduler() + scheduler.schedule_at("14:00", _noop, "local-job") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() From c74f95155550dc3d56d0f36bcdd3a23f8a452658 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 17:44:11 +0000 Subject: [PATCH 2/8] Validate market_price_refresh_time format at config load Add _validate_market_price_refresh_time() to catch invalid values like 'noon' or '25:99' early at startup with a clear error message, rather than failing silently at runtime when the scheduler tries to register the job. Addresses Copilot review comment on PR #367. --- src/batcontrol/core.py | 15 ++++++++++++++- tests/batcontrol/test_core.py | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/batcontrol/core.py b/src/batcontrol/core.py index 728b542..a52734a 100644 --- a/src/batcontrol/core.py +++ b/src/batcontrol/core.py @@ -263,9 +263,11 @@ def __init__(self, configdict: dict): self.preserve_min_grid_charge_soc = battery_control_expert.get( 'preserve_min_grid_charge_soc', self.preserve_min_grid_charge_soc) - self.market_price_refresh_time = battery_control_expert.get( + raw_refresh_time = battery_control_expert.get( 'market_price_refresh_time', self.market_price_refresh_time) + self._validate_market_price_refresh_time(raw_refresh_time) + self.market_price_refresh_time = raw_refresh_time self.general_logic = CommonLogic.get_instance( charge_rate_multiplier=self.batconfig.get( @@ -418,6 +420,17 @@ def shutdown(self): except Exception as exc: logger.exception("Error during Batcontrol shutdown: %s", exc) + @staticmethod + def _validate_market_price_refresh_time(value: str) -> None: + """Raise ValueError if value is not a valid HH:MM time string.""" + try: + datetime.datetime.strptime(value, "%H:%M") + except (ValueError, TypeError) as exc: + raise ValueError( + f"battery_control_expert.market_price_refresh_time must be " + f"a valid HH:MM time string (e.g. '12:30'), got: {value!r}" + ) from exc + def _hard_refresh_prices(self) -> None: """Force a price refresh regardless of cache state. diff --git a/tests/batcontrol/test_core.py b/tests/batcontrol/test_core.py index 31c0073..fc2ae85 100644 --- a/tests/batcontrol/test_core.py +++ b/tests/batcontrol/test_core.py @@ -1023,6 +1023,29 @@ def test_hard_refresh_prices_calls_force_refresh(self, mocker): mock_refresh.assert_called_once_with(force=True) bc.shutdown() + @pytest.mark.parametrize("invalid_value", ["25:99", "noon", "12-30", "", 1230]) + def test_invalid_market_price_refresh_time_raises(self, mocker, invalid_value): + """Invalid market_price_refresh_time must raise ValueError at init.""" + self._patch_core(mocker) + config = dict(self.BASE_CONFIG) + config['battery_control_expert'] = { + 'market_price_refresh_time': invalid_value + } + with pytest.raises(ValueError, match="market_price_refresh_time"): + Batcontrol(config) + + @pytest.mark.parametrize("valid_value", ["12:30", "00:00", "23:59", "13:00"]) + def test_valid_market_price_refresh_time_accepted(self, mocker, valid_value): + """Valid HH:MM values must be accepted without error.""" + self._patch_core(mocker) + config = dict(self.BASE_CONFIG) + config['battery_control_expert'] = { + 'market_price_refresh_time': valid_value + } + bc = Batcontrol(config) + assert bc.market_price_refresh_time == valid_value + bc.shutdown() + if __name__ == '__main__': pytest.main([__file__, '-v']) From af6606bff478a7f5c3ecb26672b8c0b0fc21517c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 17:52:42 +0000 Subject: [PATCH 3/8] Fix interface/scheduler edge cases from Copilot review - dynamictariff_interface.py: Add force: bool = False to refresh_data() so TariffInterface matches DynamicTariffBaseclass and type checkers do not report a mismatch when core calls refresh_data(force=True). - scheduler.py: Normalize empty/whitespace tz to None in schedule_at() so logging and schedule.Job.at() behaviour are always consistent. An empty string is now treated identically to None (local time). - tests: Add edge-case tests for tz="" and tz=" " in schedule_at(). --- .../dynamictariff/dynamictariff_interface.py | 8 ++++++-- src/batcontrol/scheduler.py | 8 +++++--- tests/batcontrol/test_scheduler.py | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/batcontrol/dynamictariff/dynamictariff_interface.py b/src/batcontrol/dynamictariff/dynamictariff_interface.py index 795e095..5a8849e 100644 --- a/src/batcontrol/dynamictariff/dynamictariff_interface.py +++ b/src/batcontrol/dynamictariff/dynamictariff_interface.py @@ -13,5 +13,9 @@ def get_prices(self) -> dict[int, float]: """ get prices in processable format with hours as keys """ @abstractmethod - def refresh_data(self) -> None: - """ Refresh data from provider """ \ No newline at end of file + def refresh_data(self, force: bool = False) -> None: + """ Refresh data from provider. + + Args: + force: When True, bypass cache and always fetch fresh data. + """ \ No newline at end of file diff --git a/src/batcontrol/scheduler.py b/src/batcontrol/scheduler.py index 79ef837..9a30e59 100644 --- a/src/batcontrol/scheduler.py +++ b/src/batcontrol/scheduler.py @@ -114,7 +114,9 @@ def schedule_at(time_str: str, job: Callable, job_name: str = "", The scheduled job object """ name = job_name or job.__name__ - tz_label = tz if tz else "local" + # Normalize empty/whitespace tz to None so log and schedule behaviour agree. + effective_tz = tz if tz and tz.strip() else None + tz_label = effective_tz if effective_tz else "local" logger.info("Scheduling job '%s' to run daily at %s %s", name, time_str, tz_label) # Wrap the job to catch exceptions and add logging @@ -127,8 +129,8 @@ def wrapped_job(): logger.error("Error in scheduled job '%s': %s", name, e, exc_info=True) job_def = _get_job_registry().every().day - if tz is not None: - return job_def.at(time_str, tz).do(wrapped_job) + if effective_tz is not None: + return job_def.at(time_str, effective_tz).do(wrapped_job) return job_def.at(time_str).do(wrapped_job) diff --git a/tests/batcontrol/test_scheduler.py b/tests/batcontrol/test_scheduler.py index 24d8865..c9a1594 100644 --- a/tests/batcontrol/test_scheduler.py +++ b/tests/batcontrol/test_scheduler.py @@ -81,3 +81,20 @@ def test_schedule_at_without_tz_uses_local_time(): scheduler.schedule_at("14:00", _noop, "local-job") assert len(scheduler.get_jobs()) == 1 scheduler.reset_scheduler() + + +def test_schedule_at_empty_tz_treated_as_local(): + """schedule_at() with tz='' must not forward the empty string to schedule.""" + scheduler.reset_scheduler() + # Would raise if "" were passed through to schedule.Job.at() + scheduler.schedule_at("14:00", _noop, "empty-tz-job", tz="") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() + + +def test_schedule_at_whitespace_tz_treated_as_local(): + """schedule_at() with tz containing only whitespace must be treated as None.""" + scheduler.reset_scheduler() + scheduler.schedule_at("14:00", _noop, "whitespace-tz-job", tz=" ") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() From 94ec92edd95aeb442f5707fdb74707cd3fb1c229 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 18:03:22 +0000 Subject: [PATCH 4/8] Strip whitespace from tz in schedule_at() before forwarding to schedule The previous normalization checked tz.strip() to detect whitespace-only values but assigned the original tz (with spaces) to effective_tz. A value like ' UTC ' would pass the None-guard and be forwarded as-is to schedule.Job.at(), causing an UnknownTimeZoneError at runtime. Fix: assign tz.strip() instead of tz so the stripped value is used. Adds test for ' UTC ' (padded timezone string). Addresses Copilot review comment on PR #367. --- src/batcontrol/scheduler.py | 3 ++- tests/batcontrol/test_scheduler.py | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/batcontrol/scheduler.py b/src/batcontrol/scheduler.py index 9a30e59..f2edeca 100644 --- a/src/batcontrol/scheduler.py +++ b/src/batcontrol/scheduler.py @@ -115,7 +115,8 @@ def schedule_at(time_str: str, job: Callable, job_name: str = "", """ name = job_name or job.__name__ # Normalize empty/whitespace tz to None so log and schedule behaviour agree. - effective_tz = tz if tz and tz.strip() else None + # Also strip surrounding whitespace so " UTC " is treated the same as "UTC". + effective_tz = tz.strip() if tz and tz.strip() else None tz_label = effective_tz if effective_tz else "local" logger.info("Scheduling job '%s' to run daily at %s %s", name, time_str, tz_label) diff --git a/tests/batcontrol/test_scheduler.py b/tests/batcontrol/test_scheduler.py index c9a1594..994f712 100644 --- a/tests/batcontrol/test_scheduler.py +++ b/tests/batcontrol/test_scheduler.py @@ -98,3 +98,12 @@ def test_schedule_at_whitespace_tz_treated_as_local(): scheduler.schedule_at("14:00", _noop, "whitespace-tz-job", tz=" ") assert len(scheduler.get_jobs()) == 1 scheduler.reset_scheduler() + + +def test_schedule_at_tz_with_surrounding_whitespace_is_stripped(): + """schedule_at() with tz=' UTC ' must forward 'UTC', not ' UTC '.""" + scheduler.reset_scheduler() + # Would raise UnknownTimeZoneError if the spaces were not stripped. + scheduler.schedule_at("12:30", _noop, "padded-tz-job", tz=" UTC ") + assert len(scheduler.get_jobs()) == 1 + scheduler.reset_scheduler() From 912d4642fdd40d8107ce53e1d988b9d2610c6975 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 18:17:59 +0000 Subject: [PATCH 5/8] Set wrapped_job.__name__ in schedule_at() and schedule_once() schedule_every() already sets wrapped_job.__name__ = name for consistent job identification in logs and schedule's job representation. Apply the same pattern to schedule_at() and schedule_once() for consistency across all scheduling helpers. Addresses Copilot review comment on PR #367. --- src/batcontrol/scheduler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/batcontrol/scheduler.py b/src/batcontrol/scheduler.py index f2edeca..8ff8ba4 100644 --- a/src/batcontrol/scheduler.py +++ b/src/batcontrol/scheduler.py @@ -129,6 +129,7 @@ def wrapped_job(): except Exception as e: logger.error("Error in scheduled job '%s': %s", name, e, exc_info=True) + wrapped_job.__name__ = name job_def = _get_job_registry().every().day if effective_tz is not None: return job_def.at(time_str, effective_tz).do(wrapped_job) @@ -160,6 +161,7 @@ def wrapped_job(): except Exception as e: logger.error("Error in scheduled one-time job '%s': %s", name, e, exc_info=True) + wrapped_job.__name__ = name return ( _get_job_registry() .every() From 2ffe6d352424acdb350c9d0b873840c7aeddae06 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 18:36:04 +0000 Subject: [PATCH 6/8] Fix next_update_ts computed before sleep in refresh_data() now = time.time() was captured before the optional random delay, then used to set next_update_ts after the fetch. This shortened the effective min_time_between_updates by up to delay_evaluation_by_seconds seconds. Fix: use time.time() directly when setting next_update_ts so the timestamp reflects when the successful fetch actually completed. Pre-existing issue, addressed while modifying refresh_data() for #366. --- src/batcontrol/dynamictariff/baseclass.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/batcontrol/dynamictariff/baseclass.py b/src/batcontrol/dynamictariff/baseclass.py index 35bd6da..eb3f9da 100644 --- a/src/batcontrol/dynamictariff/baseclass.py +++ b/src/batcontrol/dynamictariff/baseclass.py @@ -106,7 +106,7 @@ def refresh_data(self, force: bool = False) -> None: time.sleep(sleeptime) try: self.store_raw_data(self.get_raw_data_from_provider()) - self.next_update_ts = now + self.min_time_between_updates + self.next_update_ts = time.time() + self.min_time_between_updates self.schedule_next_refresh() except (ConnectionError, TimeoutError) as e: logger.error('Error getting raw tariff data: %s', e) From a65db30090255fd4920288d34f6cc68246c4a2e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 19:29:21 +0000 Subject: [PATCH 7/8] Clear scheduled jobs on shutdown to prevent accumulation across restarts The module-level _job_registry is shared across Batcontrol instances in the same process. Without clearing jobs on shutdown, reconstructing Batcontrol (e.g. on restart) accumulates duplicate scheduled jobs and can trigger multiple forced price refreshes per cycle. Fix: call scheduler.clear_jobs() before stopping the scheduler thread in Batcontrol.shutdown(). Adds a test to verify the registry is empty after shutdown. Addresses Copilot review comment on PR #367. --- src/batcontrol/core.py | 1 + tests/batcontrol/test_core.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/batcontrol/core.py b/src/batcontrol/core.py index a52734a..4caf86e 100644 --- a/src/batcontrol/core.py +++ b/src/batcontrol/core.py @@ -409,6 +409,7 @@ def shutdown(self): try: # Stop scheduler thread if hasattr(self, 'scheduler') and self.scheduler is not None: + self.scheduler.clear_jobs() self.scheduler.stop() del self.scheduler diff --git a/tests/batcontrol/test_core.py b/tests/batcontrol/test_core.py index fc2ae85..dc02ce1 100644 --- a/tests/batcontrol/test_core.py +++ b/tests/batcontrol/test_core.py @@ -1046,6 +1046,20 @@ def test_valid_market_price_refresh_time_accepted(self, mocker, valid_value): assert bc.market_price_refresh_time == valid_value bc.shutdown() + def test_shutdown_clears_scheduled_jobs(self, mocker): + """shutdown() must clear the job registry so multiple instantiations do not accumulate jobs.""" + from batcontrol import scheduler as sched_module + self._patch_core(mocker) + sched_module.reset_scheduler() + + bc = Batcontrol(dict(self.BASE_CONFIG)) + jobs_after_init = len(sched_module.get_jobs()) + assert jobs_after_init > 0 + + bc.shutdown() + + assert sched_module.get_jobs() == [] + if __name__ == '__main__': pytest.main([__file__, '-v']) From 603ab8f32f9cfe5615db3a8664bdfbd386193c09 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 19:37:11 +0000 Subject: [PATCH 8/8] Fix race condition: stop scheduler thread before clearing jobs clear_jobs() was called before stop(), which could race with the scheduler thread's run_pending() loop mutating the job list concurrently. schedule.Scheduler is not thread-safe. Fix: stop() (which joins the thread) first, then clear_jobs() once the thread has terminated. Addresses Copilot review comment on PR #367. --- src/batcontrol/core.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/batcontrol/core.py b/src/batcontrol/core.py index 4caf86e..26001bd 100644 --- a/src/batcontrol/core.py +++ b/src/batcontrol/core.py @@ -407,10 +407,11 @@ def shutdown(self): """ Shutdown Batcontrol and dependent modules (inverter..) """ logger.info('Shutting down Batcontrol') try: - # Stop scheduler thread + # Stop scheduler thread first, then clear jobs. + # Clearing before stop() would race with run_pending() in the thread. if hasattr(self, 'scheduler') and self.scheduler is not None: - self.scheduler.clear_jobs() self.scheduler.stop() + self.scheduler.clear_jobs() del self.scheduler self.inverter.shutdown()