Skip to content

Combine verified server jobs and compact schedule UI - #1

Merged
virtitnerd merged 4 commits into
virtitnerd:feat/schedule-reworkfrom
pponce:SCHEDULE_JOBS_INTEGRATION
Oct 27, 2026
Merged

virtitnerd merged 4 commits into
virtitnerd:feat/schedule-reworkfrom
pponce:SCHEDULE_JOBS_INTEGRATION

Conversation

@pponce

@pponce pponce commented Oct 26, 2026

Copy link
Copy Markdown

Purpose

Add the agreed DonSidro#60 contributions on top of this branch so upstream DonSidro#37 contains one combined implementation.

Changes

  • Read the HTTP jobs API directly, including imported native-linked jobs and jobs without a native timer name. Use legacy timers only when the jobs collection returns 404/405.
  • Preserve job IDs, native references, room ordering and unknown/unedited settings. Ordinary server-job edits do not recreate native timers or change enablement.
  • Share verified enablement between the list and editor: fresh reads, only necessary writes, native/server read-back, conflict checks and explicit partial-save reporting. Accommodate the server's own verified enablement reconciliation without redundant HTTP writes.
  • Verify native-linked creation/deletion outcomes; read back uncertain responses without repeated writes or blind rollback. Never substitute the HTTP job ID for a missing native name.
  • Use the agreed compact cards and full-screen editor: start time, repeat, Full/rooms with map selection, cleaning mode and Enabled. Diagnostics stay under More details.
  • Retain this branch's native create/delete support, protocol selection, SmartPlan, water-slide codes, supported mop routes, legacy compatibility and dated one-time schedules. Add Custom alongside SmartPlan, keeping their settings distinct.
  • Add regression tests and schedule usage documentation. No per-PR workflow is included.

Room editing uses the currently loaded map and preserves an imported job's map reference; changing floors still happens through Maps.

Validation

Android unit tests/build and iOS shared compilation passed:
https://github.com/pponce/LocalRock/actions/runs/40400800283

Validation used a separate fork-only workflow. The contribution differs from the tested source only in formatting/documentation and omission of that workflow.

Earlier work successfully started my S7 (a15, firmware 02.16.12) at its scheduled time, using LocalRock and the OEM app for schedule control. TestFlight and scheduled-run validation of this combined build remain pending.

Dependencies

The combined feature needs the server release containing Python-roborock/local_roborock_server#179 with the hardening and enablement synchronization from #213. That hardening is being contributed into virtitnerd:feat/schedule-store so it can land through one server PR. The #214 admin dashboard is not a dependency.

Keep upstream DonSidro#60 as a reference until this integration is accepted.

@virtitnerd virtitnerd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this. I read the shared logic and the repository changes and ran the unit tests on the branch (they pass). The approach matches what we agreed on DonSidro#37: the list comes from the jobs API, the legacy view is used only on a 404 or 405, native create and delete keep their own verified paths, and enablement is checked by read-back with the server's own reconciliation accepted as a valid change to enabled. Surfacing a failed list load on the screen is also what I wanted.

Three adjustments before I merge it:

  1. Keep the rollback on the legacy edit path. updateTimerInternal used to put the old schedule back if the replacement was refused. Now it throws "the old native timer was removed, but its replacement could not be confirmed" and leaves the user with no schedule. For jobs from the server, not rolling back blindly makes sense. For a legacy-only record, where delete then add is the only way to edit, the old behaviour was safe and tested. Could that path restore the old entry again, and report it if even that fails?
  2. Imports and trailing lines. ScheduleApi.kt, VacuumRepository.kt, ScheduleScreen.kt and the schedule files use wildcard imports (kotlinx.serialization.json.*, ...shared.schedule.*) where the rest of the project lists them, and several files gain an empty line at the end. Please use explicit imports and drop the extra lines.
  3. A test for the fallback rule. The 404/405 rule is the one that decides which robots keep working on an older server. A test that a 404 and a 405 fall back to the legacy list, and that a 500 does not, would pin it.

I have not looked at the new schedule screen on a phone yet, so I am not commenting on the layout. I will do that once these are in. After that I will merge it here so it lands in DonSidro#37 with your commits.

Restore the original legacy entry when a replacement fails and report restoration failures. Keep server-job edit behavior unchanged. Exercise 404/405 fallback, 500 propagation, empty jobs and legacy restoration outcomes. Use explicit schedule imports and remove trailing blank lines.
@pponce

pponce commented Oct 27, 2026

Copy link
Copy Markdown
Author

Thanks for the review, @virtitnerd. All three items are addressed:

  • Restored rollback for legacy delete-and-add edits, preserving the original schedule’s settings and enabled state. Errors now report whether restoration succeeded or also failed. Server-job editing behavior is unchanged.
  • Replaced wildcard imports with explicit imports and removed extra trailing blank lines.
  • Added tests confirming that 404 and 405 fall back to the legacy list, while 500 does not. An empty jobs list also remains authoritative. Added regression coverage for legacy restoration outcomes and cancellation.

Android unit tests/build and iOS shared compilation passed. The validation workflow remains outside this PR.

The changes are pushed and ready for another look. Phone/TestFlight testing of the combined build remains pending.

@virtitnerd virtitnerd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The three changes are in, the tests pass here, and the compact layout loads my three real schedules from the jobs API on a Qrevo Pro and opens the editor. Merging into feat/schedule-rework with your commits.

@virtitnerd
virtitnerd merged commit 001da16 into virtitnerd:feat/schedule-rework Oct 27, 2026
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.

2 participants