Tariffs: support cron expressions for refresh scheduling - #33127
Conversation
1abd7a5 to
5455bfd
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tariff/schedule.go" line_range="65-73" />
<code_context>
+ return ch
+ }
+
+ go func() {
+ for {
+ next := t.sched.Next(time.Now())
+ if next.IsZero() {
+ return
+ }
+ tt := <-time.After(time.Until(next))
+ ch <- tt
+ }
+ }()
+ return ch
</code_context>
<issue_to_address>
**issue (bug_risk):** When a provider's refresh loop exits because of a fatal error, the goroutine started by `refreshTimer.C` continues running forever. For cron schedules it remains blocked in `time.After` or on the channel send, and for duration schedules it remains forwarding ticks, so every failed provider instance leaves a live goroutine and timer behind.
**Triggers:** When a tariff refresh goroutine terminates after reporting an unrecoverable error.
**Suggested fix:** Give `refreshTimer.C` a cancellation path and stop its timer goroutine when the owning refresh loop exits.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and an incorrect schedule can leave tariff data stale or trigger refreshes too frequently, and the resulting cached data or provider throttling can outlive a revert. Reverting restores the old refresh behavior, but stale caches may need a refresh or other bounded repair.
Blocking findings: tariff/schedule.go:73
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
It's not clear to me why we'd need this and I'm reluctant to expose cron semantics to end users. /cc @naltatis |
|
Ne, Cron syntax is very user hostile. We should not expose that in UI. We cant/and shouldn't assume our users to know anything about unix systems. |
|
Fair point on cron being too raw for the UI. My use case: providers like pvnode rate-limit free tiers to a couple fetches a month. With a plain interval, the schedule anchors to the last restart, not a fixed time — so if evcc restarts at 3pm, it fetches the next day at 3pm instead of ~00:00 when the new forecast is actually available. Maybe instead of raw cron, PropertyField could offer a simple "daily at HH:MM" option that compiles to cron internally — gets the fixed-time behavior without exposing cron syntax to users. |
|
Especially with pvnode Ou can just fetch every hour and they will redeliver the old response. |
|
Isn't the pvnode v2 API limited to 250 requests per month? It's only updated once per day, but you still get served from the cache each time — and that still counts against the request limit. So if I requested every hour, wouldn't I exceed the limit with around 750 requests? https://pvnode.com/blog/2026-06-pvnode-v2-launch |
|
It is, but luckily they don‘t error |
|
Can you clarify what you mean by that? The Api reponse? |
Most tariff providers refresh on a hardcoded interval (usually hourly), with no way to schedule a fetch at a fixed time of day. This adds a shared
scheduleconfig type:intervalnow accepts either a duration (15m,1h,24h) or a standard cron expression (15 0 * * *,@daily,@every 1h30m), evaluated in local time (CRON_TZ=prefix supported). It's squashed into every tariff provider's config, the genericcustom/Configurable tariff, and the caching proxy, so any tariff can now be pinned to e.g. a single daily fetch instead of polling continuously.tariff/schedule.go: newschedule/refreshTimertypes — auto-detects duration vs. cron, rejects impossible cron expressions (e.g. Feb 30) at config time so the refresh goroutine can't busy-looptime.Tick(fixedInterval)refresh loops replaced withtimer.C()util.Monitorstaleness window as a hardcoded2 * time.Hour, independent of the actual refresh interval. With a day-long cron schedule that madeRates()report outdated data long before the next scheduled fetch. The window is now derived from the timer (max(2h, timer.window()), the floor keeps today's failure tolerance for fast-polling providers)PropertyField.vue: interval fields get a "cron" unit option that switches the input to free text with a loose client-side pattern check (backend stays authoritative)util/templates/defaults.yamlinstead of duplicating it across the seven tariff templates