-
Notifications
You must be signed in to change notification settings - Fork 52
feat: new batt schedule command for fully automatic calibration
#95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # pkg/daemon/calibration.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds scheduled automatic calibration functionality to batt, allowing users to configure recurring calibration runs using cron expressions. The feature includes a 5-minute lead notification period before execution and supports postpone/skip operations.
Key changes:
- New
Schedulercomponent with cron-based scheduling and precheck retry logic - REST API endpoints (
/schedule,/schedule/postpone,/schedule/skip) for schedule management - CLI command
batt schedulewith subcommands for setting, disabling, postponing, and skipping schedules
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/daemon/scheduler.go | Core scheduler implementation with cron parsing, timer management, and control message handling |
| pkg/daemon/scheduler_test.go | Unit tests for scheduler functionality including postpone, skip, and precheck failure scenarios |
| pkg/daemon/daemon.go | Scheduler initialization with task and precheck callbacks; adds schedule-related event publishing |
| pkg/daemon/calibration.go | Schedule persistence logic and helper functions for schedule, postpone, and skip operations |
| pkg/daemon/handlers.go | HTTP handlers for schedule management endpoints with validation |
| pkg/config/config.go | Config interface additions for cron expression getter/setter |
| pkg/config/file.go | File-based config implementation for persisting cron expressions |
| pkg/client/apis.go | Client API methods for schedule operations |
| cmd/batt/schedule.go | CLI command implementation for schedule management |
| cmd/batt/main.go | Registration of schedule command |
| README.md | Documentation for scheduled calibration feature |
| go.mod, go.sum | Added robfig/cron/v3 dependency |
| pkg/calibration/types.go | New action types and ScheduledAt field for status |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this 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 14 out of 15 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@charlie0129 pls take a look. I am still testing. |
|
@brookqin Thanks! I will review and test it in this week when I am free. |
| Aliases: []string{"sch"}, | ||
| Short: "Manage automatic calibration schedule", | ||
| Long: `Examples: | ||
| batt schedule '0 10 * * 0' # Note: Cron expressions containing * must be quoted! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add some examples on cron expressions?
For example:
batt schedule 'minute hour day month weekday'
batt schedule '0 10 * * 0' (At 10:00 on Sunday)
batt schedule '0 10 1 * *' (At 10:00 on the first day of every month)
batt schedule '0 10 1 */2 *' (At 10:00 on the first day of every two months)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
By the way, do you know what timezone is the cron using? UTC or local timezone? |
time.Now() should be UTC. Scheduler may have a bug, every time computer goes to sleep, timer trigger will be delayed. time: NewTimer firing later if computer sleeps, how to use wall clock? seems have to use a loop with a short timer. |
You mean if a timer is supposed to fire in 5 minutes, but if the computer goes to sleep now, the timer will fire 5 minutes after waking up? I think I experienced the opposite. If the computer goes to sleep, the timer will fire at the correct time. If the computer slept for longer than 5 minutes, it fires immediately after waking up. Correct me if I am wrong. |
|
I think the cron should use user's local time. Using UTC will cause confusion for users. Adding |
you are right, I will fix this.
My mac sleep whole day last sunday, seems there are issues with both time zone and timer. |
|
Maybe the implementation of robfig/cron will be prolonged by sleep? I remember that time.After will not be affected by sleep. I will check how robfig/cron handles this. |
hi @charlie0129,
I'm working on
schedulecommand, will add scheduled calibration functionality to batt, allowing users to set up and manage calibration schedules using cron expressions.Scheduled calibration feature:
/calibration/scheduleendpoint to accept a cron expression or null to enable or disable scheduling.batt schedule.calibratecommand.CLI examples: