From a03d232dd3248522c8c203abf650c11ed92a56ed Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 13 Apr 2026 20:24:28 +0200 Subject: [PATCH] Finalize implementation --- docs/contributing/architecture.md | 2 +- docs/contributing/koreader-synchronization.md | 33 +++++++++++++++++++ lib/KOReaderSync/KOReaderSyncClient.cpp | 33 ++++++++++++++++--- src/activities/reader/EpubReaderActivity.cpp | 20 ++++++++--- src/activities/reader/EpubReaderActivity.h | 4 +++ .../reader/KOReaderSyncActivity.cpp | 11 ++++--- src/activities/reader/KOReaderSyncActivity.h | 9 +++-- 7 files changed, 96 insertions(+), 16 deletions(-) diff --git a/docs/contributing/architecture.md b/docs/contributing/architecture.md index cab0db95..d7c98394 100644 --- a/docs/contributing/architecture.md +++ b/docs/contributing/architecture.md @@ -147,7 +147,7 @@ are documented in: Two singletons are central: - `src/CrossPointSettings.h` (`SETTINGS`): user preferences and behavior flags -- `src/CrossPointState.h` (`APP_STATE`): runtime/session state such as current book and sleep context +- `src/CrossPointState.h` (`APP_STATE`): runtime/session state such as current book, sleep context, and standalone KOReader sync handoff/outcome state Typical persisted areas on SD: diff --git a/docs/contributing/koreader-synchronization.md b/docs/contributing/koreader-synchronization.md index 31aa74ad..8fa08606 100644 --- a/docs/contributing/koreader-synchronization.md +++ b/docs/contributing/koreader-synchronization.md @@ -17,6 +17,39 @@ The synchronization layer is designed to: - Keep transport/client logic separated from parsing/mapping logic. - Prefer precise anchors when available, but degrade gracefully. +## Standalone Sync Lifecycle + +KOReader sync no longer runs as a child activity on top of a live EPUB reader. +Instead, the reader persists a compact handoff record in `APP_STATE`, then the +activity stack is replaced with `KOReaderSyncActivity`. + +Why this exists: +- HTTPS/TLS setup on ESP32-C3 is sensitive to heap fragmentation. +- Keeping the full reader activity alive underneath sync reclaimed too little + memory and made failures harder to reason about. +- A standalone sync screen makes memory snapshots and failure modes easier to + interpret. + +Current lifecycle: +1. `EpubReaderActivity` stores the EPUB path, current local position, sync + intent, and any future sync result slots in `APP_STATE.koReaderSyncSession`. +2. The reader activity stack is replaced with `KOReaderSyncActivity`. +3. `KOReaderSyncActivity` lazily reloads EPUB data only for mapping work and + releases it again before network-heavy phases. +4. On completion, cancel, or failure, sync persists an outcome and reopens the + book through the normal `ReaderActivity -> EpubReaderActivity` path. +5. `EpubReaderActivity::applyPendingSyncSession()` consumes that outcome: + - remote-apply writes the reopen position into `progress.bin` before normal + reader startup loads it + - upload-complete keeps the existing local `progress.bin` unchanged + - paragraph-level correction metadata is still carried separately because + `progress.bin` stores only spine/page/pageCount + +Memory notes: +- Reader-owned state is reclaimed by fully exiting the reader before sync. +- Sync still trims its own renderer/font caches right before TLS because sync UI + rendering can repopulate those caches after the reader is gone. + ## Data Model Mismatch CrossPoint stores position as chapter/page-centric state. diff --git a/lib/KOReaderSync/KOReaderSyncClient.cpp b/lib/KOReaderSync/KOReaderSyncClient.cpp index a32e0f40..b3a05a9b 100644 --- a/lib/KOReaderSync/KOReaderSyncClient.cpp +++ b/lib/KOReaderSync/KOReaderSyncClient.cpp @@ -31,6 +31,7 @@ esp_http_client_handle_t g_sessionClient = nullptr; // the longest expected message including esp_err name (~32 chars), opcode (~10), heap // numbers, and HTTP status. Single-threaded sync flow makes static safe. char g_failureDetailBuf[160] = {0}; +char g_lastResponsePreview[160] = {0}; std::string previewBody(const char* body, const size_t maxLen = 120) { if (!body || !*body) { @@ -56,6 +57,12 @@ std::string previewBody(const char* body, const size_t maxLen = 120) { return preview; } +void rememberResponsePreview(const char* body) { + const std::string preview = previewBody(body, sizeof(g_lastResponsePreview) - 1); + strncpy(g_lastResponsePreview, preview.c_str(), sizeof(g_lastResponsePreview) - 1); + g_lastResponsePreview[sizeof(g_lastResponsePreview) - 1] = '\0'; +} + // Reset the static diagnostic state at the start of each request and capture pre-flight // heap so failure reporting always reflects what was available when the request started. void beginRequest(const char* operation) { @@ -64,6 +71,7 @@ void beginRequest(const char* operation) { KOReaderSyncClient::lastHttpCode = 0; KOReaderSyncClient::lastHeapAtFailure = ESP.getFreeHeap(); KOReaderSyncClient::lastContigHeapAtFailure = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); + g_lastResponsePreview[0] = '\0'; } // Skip a leading UTF-8 BOM (EF BB BF) and ASCII whitespace, returning a pointer @@ -475,7 +483,8 @@ KOReaderSyncClient::Error KOReaderSyncClient::getProgress(const std::string& doc LOG_DBG("KOSync", "GET %s -> %d (err: %s) [attempt %d body_len=%u]", url.c_str(), httpCode, esp_err_to_name(err), attempt, static_cast(bodyLen)); if (err == ESP_OK && (httpCode < 200 || httpCode >= 300)) { - LOG_ERR("KOSync", "GET failure body preview: %s", previewBody(activeBuf->data).c_str()); + rememberResponsePreview(activeBuf->data); + LOG_ERR("KOSync", "GET failure body preview: %s", g_lastResponsePreview); } // Retry exactly once for connect-level failures only. @@ -597,9 +606,10 @@ KOReaderSyncClient::Error KOReaderSyncClient::updateProgress(const KOReaderProgr LOG_DBG("KOSync", "PUT %s -> %d (err: %s) [attempt %d body_len=%u]", url.c_str(), httpCode, esp_err_to_name(err), attempt, static_cast(bodyLen)); if (err == ESP_OK && (httpCode < 200 || httpCode >= 300)) { - LOG_ERR("KOSync", "PUT failure body preview: %s", previewBody(activeBuf->data).c_str()); - LOG_ERR("KOSync", "PUT failure request summary: document=%s percentage=%.4f progress=%s", progress.document.c_str(), - progress.percentage, progress.progress.c_str()); + rememberResponsePreview(activeBuf->data); + LOG_ERR("KOSync", "PUT failure body preview: %s", g_lastResponsePreview); + LOG_ERR("KOSync", "PUT failure request summary: document=%s percentage=%.4f progress=%s", + progress.document.c_str(), progress.percentage, progress.progress.c_str()); } // Retry exactly once for connect-level failures only. @@ -670,6 +680,21 @@ const char* KOReaderSyncClient::lastFailureDetail() { } // Server case: got an HTTP status the client didn't recognize as success. if (lastHttpCode != 0) { + if (lastHttpCode == 404 && lastOperation && strcmp(lastOperation, "update progress") == 0) { + std::string lowerPreview = g_lastResponsePreview; + std::transform(lowerPreview.begin(), lowerPreview.end(), lowerPreview.begin(), + [](unsigned char c) { return static_cast(std::tolower(c)); }); + if (lowerPreview.find("book not found") != std::string::npos) { + snprintf(g_failureDetailBuf, sizeof(g_failureDetailBuf), + "%s: server does not know this book yet; server expects the same file to already exist there, usually " + "downloaded via OPDS", + lastOperation); + return g_failureDetailBuf; + } + snprintf(g_failureDetailBuf, sizeof(g_failureDetailBuf), + "%s: HTTP 404 (upload rejected; server may require book to be known)", lastOperation); + return g_failureDetailBuf; + } snprintf(g_failureDetailBuf, sizeof(g_failureDetailBuf), "%s: HTTP %d", lastOperation, lastHttpCode); return g_failureDetailBuf; } diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index baf492f8..8a5047e6 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -39,7 +39,8 @@ void logReaderMemSnapshot(const char* stage) { LOG_DBG("ERS", "Reader mem[%s]: free=%lu contig=%lu", stage, freeHeap, contigHeap); } -bool writeReaderProgressCache(const std::string& cachePath, const int spineIndex, const int currentPage, const int pageCount) { +bool writeReaderProgressCache(const std::string& cachePath, const int spineIndex, const int currentPage, + const int pageCount) { FsFile f; if (!Storage.openFileForWrite("ERS", cachePath + "/progress.bin", f)) { LOG_ERR("ERS", "Failed to open progress cache for sync restore: %s", cachePath.c_str()); @@ -566,7 +567,18 @@ void EpubReaderActivity::applyPendingSyncSession() { return; } - LOG_DBG("ERS", "Applying pending sync session outcome=%d path=%s", static_cast(sync.outcome), sync.epubPath.c_str()); + LOG_DBG("ERS", "Applying pending sync session outcome=%d path=%s", static_cast(sync.outcome), + sync.epubPath.c_str()); + + // Upload-complete returns to the same local position the reader already persisted + // before sync launched, so there is no need to rewrite progress.bin here. + if (sync.outcome == KOReaderSyncOutcomeState::UPLOAD_COMPLETE) { + LOG_DBG("ERS", "Upload-complete resume keeps existing local progress.bin unchanged"); + sync.clear(); + APP_STATE.saveToFile(); + logReaderMemSnapshot("after_apply_pending_sync_session"); + return; + } int restoreSpineIndex = sync.spineIndex; int restorePage = sync.page; @@ -578,8 +590,8 @@ void EpubReaderActivity::applyPendingSyncSession() { restorePage = sync.resultPage; pendingParagraphLookup = sync.resultHasParagraphIndex; pendingParagraphIndex = sync.resultParagraphIndex; - LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d paragraph=%u hasParagraph=%s", - restoreSpineIndex, restorePage, pendingParagraphIndex, pendingParagraphLookup ? "yes" : "no"); + LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d paragraph=%u hasParagraph=%s", restoreSpineIndex, + restorePage, pendingParagraphIndex, pendingParagraphLookup ? "yes" : "no"); } else { LOG_DBG("ERS", "Restored local pre-sync position: spine=%d page=%d paragraph=%u hasParagraph=%s", restoreSpineIndex, restorePage, pendingParagraphIndex, pendingParagraphLookup ? "yes" : "no"); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 07cb710a..8e23dd18 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -71,6 +71,10 @@ class EpubReaderActivity final : public Activity { void jumpToPercent(int percent); void onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction action); void launchKOReaderSync(SyncLaunchMode mode = SyncLaunchMode::COMPARE); + // Consume a persisted standalone KOReader sync session for this EPUB. Remote + // apply writes the mapped reopen position into progress.bin before the normal + // reader startup path reads it. Upload-complete leaves the existing local + // progress.bin untouched and simply clears the pending session marker. void applyPendingSyncSession(); void applyOrientation(uint8_t orientation); void applyTextDarkness(uint8_t textDarkness); diff --git a/src/activities/reader/KOReaderSyncActivity.cpp b/src/activities/reader/KOReaderSyncActivity.cpp index 496c5423..871a7630 100644 --- a/src/activities/reader/KOReaderSyncActivity.cpp +++ b/src/activities/reader/KOReaderSyncActivity.cpp @@ -29,9 +29,10 @@ void logSyncMemSnapshot(const char* stage) { integrityOk ? "ok" : "fail"); } -// Frees renderer-owned caches right before network work. -// Why: TLS handshake needs a large contiguous block, and font cache memory can -// increase fragmentation even when total free heap looks acceptable. +// Frees renderer-owned caches inside the standalone sync activity right before +// network work. The reader activity is already gone by this point, but sync UI +// rendering (status popups, compare screen, result screen) can repopulate font +// caches and chip away at the largest free block needed for TLS. void trimMemoryBeforeTls(const GfxRenderer& renderer) { if (auto* cacheManager = renderer.getFontCacheManager()) { cacheManager->clearCache(); @@ -330,8 +331,8 @@ void KOReaderSyncActivity::performUpload() { return; } - // Result screen rendering repopulates glyph caches; trim again right before - // the upload handshake to maximize contiguous heap for TLS. + // Sync UI rendering can repopulate glyph caches after the initial GET / compare + // phase, so trim again right before the upload request. trimMemoryBeforeTls(renderer); logSyncMemSnapshot("after_trim_before_updateProgress"); diff --git a/src/activities/reader/KOReaderSyncActivity.h b/src/activities/reader/KOReaderSyncActivity.h index 06ac7bb5..6f827623 100644 --- a/src/activities/reader/KOReaderSyncActivity.h +++ b/src/activities/reader/KOReaderSyncActivity.h @@ -4,14 +4,19 @@ #include #include "ChapterXPathIndexer.h" +#include "CrossPointState.h" #include "KOReaderSyncClient.h" #include "ProgressMapper.h" -#include "CrossPointState.h" #include "activities/Activity.h" /** * Activity for syncing reading progress with KOReader sync server. * + * This activity is launched as a standalone replacement screen, not as a + * child activity of the reader. The reader persists a compact handoff record, + * is destroyed to reclaim memory before WiFi/TLS work begins, and a fresh + * reader instance is reopened after sync completes or is cancelled. + * * Shared pipeline: * 1. Connect to WiFi (if not connected) * 2. Optionally sync NTP (if stale) @@ -21,7 +26,7 @@ * - COMPARE: fetch remote progress, show full comparison screen, let user * choose Apply or Upload. * - PULL_REMOTE: fetch and map remote progress, show success feedback, then - * return applied SyncResult to reader. + * persist an applied SyncResult for the reopened reader. * - PUSH_LOCAL: compute local mapping, warm session with GET, then upload via * reused connection to avoid a second full TLS handshake. */