Repository navigation
Combine verified server jobs and compact schedule UI - #1
virtitnerd merged 4 commits into
Conversation
virtitnerd
left a comment
There was a problem hiding this comment.
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:
- Keep the rollback on the legacy edit path.
updateTimerInternalused 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? - Imports and trailing lines.
ScheduleApi.kt,VacuumRepository.kt,ScheduleScreen.ktand 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. - 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.
|
Thanks for the review, @virtitnerd. All three items are addressed:
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
left a comment
There was a problem hiding this comment.
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.
Purpose
Add the agreed DonSidro#60 contributions on top of this branch so upstream DonSidro#37 contains one combined implementation.
Changes
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.