Skip to content

Add configurable market price hard refresh at scheduled time#367

Merged
MaStr merged 8 commits into
mainfrom
claude/market-price-refresh
May 23, 2026
Merged

Add configurable market price hard refresh at scheduled time#367
MaStr merged 8 commits into
mainfrom
claude/market-price-refresh

Conversation

@MaStr
Copy link
Copy Markdown
Owner

@MaStr MaStr commented May 23, 2026

Summary

Implements a configurable daily hard refresh of market prices at a scheduled UTC time (default 12:30) to pick up newly published dynamic tariff data, such as EPEX Spot next-day prices. This addresses issue #366.

Key Changes

  • Core scheduling: Added market_price_refresh_time configuration parameter (default "12:30" UTC) in battery_control_expert section, with a new scheduled job that triggers _hard_refresh_prices() daily
  • Force refresh capability: Extended DynamicTariffBaseclass.refresh_data() with a force parameter that bypasses cache validity checks (next_update_ts) to ensure fresh data is fetched
  • Scheduler enhancement: Updated schedule_at() function to support optional timezone parameter, allowing jobs to be scheduled in specific timezones (e.g., UTC) rather than only server local time
  • Logging and documentation: Added informative logging for forced refreshes and updated configuration example with commented documentation

Implementation Details

  • The hard refresh is scheduled via the existing scheduler infrastructure using schedule_at() with tz='UTC' to ensure consistent timing across deployments
  • When force=True is passed to refresh_data(), it logs the action and skips the next_update_ts check, then updates next_update_ts after fetching to prevent excessive requests
  • The scheduler's schedule_at() method now conditionally applies timezone to the job definition only when specified, maintaining backward compatibility
  • Comprehensive test coverage added for the new functionality including default behavior, custom configuration, forced refresh behavior, and cache bypass logic

https://claude.ai/code/session_012kMDVz2ykrD1pRpfghzpPP

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.
Copilot AI review requested due to automatic review settings May 23, 2026 17:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a configurable daily “hard refresh” for dynamic market prices at a scheduled UTC time (default 12:30) to reliably pick up newly published next-day tariffs (issue #366).

Changes:

  • Introduces battery_control_expert.market_price_refresh_time and schedules a daily UTC job that forces a tariff refresh.
  • Extends scheduler support to optionally schedule daily jobs in a specified timezone (tz).
  • Extends DynamicTariffBaseclass.refresh_data() with a force flag and adds tests for forced refresh + scheduling behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/batcontrol/core.py Adds config-driven daily UTC job to force-refresh dynamic tariff prices.
src/batcontrol/dynamictariff/baseclass.py Adds force parameter to bypass next_update_ts checks for refresh.
src/batcontrol/scheduler.py Adds optional tz parameter to schedule_at() and logs timezone context.
config/batcontrol_config_dummy.yaml Documents the new expert config option (commented) and its format/intent.
tests/batcontrol/test_scheduler.py Adds tests ensuring schedule_at() registers jobs with/without timezone.
tests/batcontrol/test_core.py Adds tests for default/custom refresh time and hard refresh calling force=True.
tests/batcontrol/dynamictariff/test_baseclass.py Adds tests validating force bypass behavior and next_update_ts update.

Comment thread src/batcontrol/dynamictariff/baseclass.py
Comment thread src/batcontrol/scheduler.py Outdated
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response

Comment 1 — baseclass.py: DST/UTC mismatch in schedule_next_refresh() (not implemented)

schedule_next_refresh() uses time.localtime() for the one-time cache-expiry job — this behaviour is pre-existing and unrelated to this PR. The new UTC hard refresh and the existing cache-expiry schedule serve independent purposes and don't interfere. Fixing DST handling in schedule_next_refresh() is out of scope here and should be tracked separately if needed.

Comment 2 — core.py: Validate market_price_refresh_time format (implemented — commit c74f951)

Added _validate_market_price_refresh_time() which calls datetime.strptime(value, "%H:%M") at config-load time. Invalid values like "noon" or "25:99" now raise a clear ValueError at startup instead of crashing later in the scheduler. New parametrised tests cover multiple valid and invalid inputs.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/batcontrol/dynamictariff/baseclass.py
Comment thread src/batcontrol/scheduler.py
- 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().
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 2

Kommentar 3 — TariffInterface.refresh_data() ohne force-Parameter (implementiert — commit af6606b)

dynamictariff_interface.py aktualisiert: refresh_data(self, force: bool = False) — Interface und Implementierung sind jetzt synchron. Andere Interface-Hierarchien (forecastsolar, forecastconsumption) sind davon unabhängig und wurden nicht angepasst.

Kommentar 4 — tz="" in schedule_at() inkonsistent (implementiert — commit af6606b)

tz wird jetzt zu Beginn via tz if tz and tz.strip() else None normalisiert. Leere Strings und Whitespace-only-Werte werden identisch zu None behandelt — Log und Schedule-Verhalten stimmen damit immer überein. Zwei neue Tests decken tz="" und tz=" " ab.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/batcontrol/scheduler.py
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 3

Kommentar 5 — scheduler.py: tz.strip() wurde nicht in effective_tz übernommen (implementiert — commit 94ec92e)

Echter Bug: effective_tz = tz if tz and tz.strip() else None prüfte korrekt auf Whitespace, aber wies den ungestrippten Wert zu. " UTC " hätte den Guard passiert und wäre mit Leerzeichen an schedule.Job.at() übergeben worden.

Fix: effective_tz = tz.strip() if tz and tz.strip() else None — jetzt wird der gestrippte Wert zugewiesen. Neuer Test deckt tz=" UTC " explizit ab.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/batcontrol/scheduler.py
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 4

Kommentar 6 — schedule_at(): wrapped_job.__name__ nicht gesetzt (implementiert — commit 912d464)

Beim Durchsehen fiel auf, dass schedule_once() das gleiche Problem hatte. Beide Funktionen setzen jetzt wrapped_job.__name__ = name, konsistent mit schedule_every().


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/batcontrol/dynamictariff/baseclass.py
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 5

Kommentar 7 — baseclass.py: next_update_ts auf Basis von now vor dem Sleep berechnet (implementiert — commit 2ffe6d3)

Pre-existing Bug, nicht durch diesen PR eingeführt, aber da wir refresh_data() ohnehin angefasst haben: now + min_time_between_updatestime.time() + min_time_between_updates. Der Timestamp wird jetzt nach dem erfolgreichen Fetch gesetzt, sodass min_time_between_updates korrekt ab Abschluss des API-Calls gerechnet wird.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/batcontrol/core.py
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 6

Kommentar 8 — core.py: Jobs akkumulieren bei Mehrfach-Instanziierung (implementiert — commit a65db30)

shutdown() ruft jetzt self.scheduler.clear_jobs() vor stop() auf. Der private _job_registry wird damit bei jedem Shutdown sauber geleert — kein Job-Leakage bei Restart oder in Tests. Neuer Test verifiziert, dass get_jobs() nach shutdown() leer ist.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/batcontrol/core.py Outdated
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.
Copy link
Copy Markdown
Owner Author

MaStr commented May 23, 2026

Copilot Review Response — Runde 7

Kommentar 9 — core.py: Race Condition zwischen clear_jobs() und run_pending() (implementiert — commit 603ab8f)

Selbst verursachter Bug durch falsche Reihenfolge im letzten Commit. stop() setzt den Stop-Event und joined den Thread (mit 5s Timeout) — danach ist der Thread garantiert beendet. clear_jobs() wird jetzt erst nach stop() aufgerufen, sodass keine nebenläufige Mutation der Job-Liste möglich ist.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@MaStr MaStr merged commit d450005 into main May 23, 2026
17 checks passed
@MaStr MaStr deleted the claude/market-price-refresh branch May 23, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants