feat: auto KOSync on book open/close#163
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac80b35240
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Compute local percentage from persisted spine position (section not loaded yet). | ||
| // Use epub's cumulative byte sizes as a rough percentage estimate. | ||
| float localPct = 0.0f; | ||
| if (epub->getBookSize() > 0 && currentSpineIndex > 0) { |
There was a problem hiding this comment.
Include spine 0 in local-progress comparison before auto-apply
This guard leaves localPct at 0 whenever the saved position is in spine 0, so any remote progress above ~0.5% is treated as "ahead" and can overwrite a newer local position in the first chapter. In practice, if a reader is still in spine 0 locally (e.g., 20%) and the server has an older value (e.g., 5%), the open-sync path will incorrectly jump backward. The comparison should compute local progress for spine 0 as well (or otherwise use a valid fallback) before deciding to apply remote progress.
Useful? React with 👍 / 👎.
| if (shouldSkip(false)) return SyncStatus::SKIPPED; | ||
|
|
||
| // Check if position actually changed since last sync | ||
| if (!hasPositionChanged(payload.documentHash, 0, 0)) { |
There was a problem hiding this comment.
Compare close-sync change detection using actual current position
The close-sync change check always passes because it compares against hardcoded (spine=0,page=0) but success recording stores a percentage-derived page value (recordPosition(..., 0, localPercentage*10000)). After the first successful sync, hasPositionChanged(payload.documentHash, 0, 0) remains true for nearly all books, so the "position unchanged" skip path never triggers and the device still wakes WiFi and performs GETs on unchanged closes, hurting battery and latency.
Useful? React with 👍 / 👎.
|
@togotago - Have you tested this with the custom SD card fonts? Curious if you ran into any memory issues since the SD card fonts put a lot of pressure on memory and free heap |
|
@uxjulia I have been running this on patched crossink for the last days exclusively with SD card fonts and have run into no issues. I believe I had one crash of an EPUB where it returned me to the end of the book, with that said I don't believe it was during any KOSync events and most likely not related to the auto-sync. With that said when I get a chance ill look into the auto-sync logs |
|
@uxjulia Actually, just checked the logs and they are looking rock solid - no crashes, no OOM. Only thing I am seeing are Wifi timeout errors but that is by design + due to weak wifi (esp32 C3 inherently having bad wifi connectivity), and correct timeouts anytime I am away from my home network. All looking clean. Have you been able to test it out at all on your end yet? Curious to see how it is working on other devices |
Admittedly I haven't yet, I've been trying to improve wifi stability and throughput for the fonts, but I plan to test this out today |
|
@togotago - So I rebased your changes on the latest development branch and was testing this out and the logs show that it starts to try to sync by getting my progress when I enter and exit, but I don't actually see any syncing happening both in the logs or when I check my server (I'm using koinsight). But If I manually sync, then I start seeing the activity in the logs that weren't happening automatically and then progress shows up on the server. I think on the very first open it did sync, but thereafter I wasn't seeing anything. This is what I see in the debug logs when I exit, but it doesn't actually sync. Can you test this locally with the latest development changes and let me know if it's working for you? |
|
@uxjulia Could you check if The debug log you shared ( My guess is the rebase had conflicts in When I get some time, I'll flash the latest dev on my device and test it to see if I get the same issues you're seeing. I can also rebase my branch onto latest development, resolve the conflicts, and force-push so the PR is up to date and you can just pull and test cleanly. Let me know what you think. |
Add automatic KOReader progress sync with three-state setting: - Off (default): manual sync only - On Close: upload progress when exiting book - On Open + Close: fetch remote on open, upload on close Direction-aware: only pushes when local is ahead, only pulls when remote is ahead. Battery-optimized with skip rules (no creds, session too short, recent sync cooldown, failure backoff). Sync-on-open runs in ReaderActivity before loadEpub() where full heap is available for TLS. Sync-on-close precomputes payload while epub is alive in onExit(), executes after epub.reset() frees heap. Catches all exit paths including sleep (ESP32-C3 deep sleep triggers onExit via goToSleep -> replaceActivity -> exitActivity). New files: - lib/KOReaderSync/AutoKOSync.h/.cpp (self-contained sync library) Modified: - CrossPointSettings.h (autoKOSync uint8_t field) - SettingsList.h (three-option enum in KOReader Sync section) - english.yaml (4 new i18n strings) - ReaderActivity (sync-on-open before loadEpub, pass remote via ctor) - EpubReaderActivity (apply remote on open, close payload + execution)
|
Went ahead and rebased onto latest development, tested on my device and auto-sync open and close both working just fine. Force-pushed the branch so the PR is up to date. Give this a shot and let me know if you run into the same problem. |
ac80b35 to
49cb6e5
Compare
|
@togotago - I haven't checked the latest rebase yet, I just wanted to take a look at the logs to see what was going on and a couple of times my session was too short (makes sense), but a few times the wifi connection failed silently. I don't think there's any indication that it failed right now right? Unless I missed that? Might be a good idea to show a pop-up saying the sync failed, and asking if the user wants to manually sync or something. I'm merging in latest upstream crosspoint updates right now and then I'll rebase your PR on top of it again and test, hopefully I don't mess up the rebase 😅. Sorry, there's a lot of performance updates coming in on Crosspoint that I'd like to fold into Crossink that improves WiFi and SD card fonts, so it's been a bit of a moving target. |
|
@uxjulia No problem. Having fun working on this. Overall the auto-sync fails a lot by design (failing fast when no wifi connection), and normally there is a "sync failed" or "no wifi" UI notification showing that the sync was not successful. Did you see either of these on your device? Either way, ill probably keep my feature branch up to date as I plan to keep using it and rolling in the new features until this possibly is merged, so no worries on the rebase, happy to help there if need be. Moving target indeed! |
## Summary * Swap out Bookerly font due to licensing issues, replace default font with Aleo * I did a bunch of searching around for a nice replacement font, and this trumped several other like Literata, Merriwether, Vollkorn, etc * Add Noto Sans, and Open Dyslexic as font options * They can be selected in the settings screen * Add font size options (Small, Medium, Large, Extra Large) * Adjustable in settings * Swap out uses of reader font in headings and replaced with slightly larger Ubuntu font * Replaced PixelArial14 font as it was difficult to track down, replace with Space Grotesk * Remove auto formatting on generated font files * Massively speeds up formatting step now that there is a lot more CPP font source * Include fonts with their licenses in the repo ## Additional Context Line compression setting will follow | Font | Small | Medium | Large | X Large | | --- | --- | --- | --- | --- | | Aleo |  |  |  |  | | Noto Sans |  |  |  |  | | Open Dyslexic |  |  |  |  |
Add automatic KOReader progress sync with three-state setting:
Direction-aware: only pushes when local is ahead, only pulls when remote is ahead. Battery-optimized with skip rules (no creds, session too short, recent sync cooldown, failure backoff).
Sync-on-open runs in ReaderActivity before loadEpub() where full heap is available for TLS. Sync-on-close precomputes payload while epub is alive in onExit(), executes after epub.reset() frees heap.
Catches all exit paths including sleep (ESP32-C3 deep sleep triggers onExit via goToSleep -> replaceActivity -> exitActivity).
New files:
Modified:
Summary
What changes are included?