Split menu actions for smaller footprint

2026-04-07 21:39:03 +02:00
parent abbf69649e
commit 6dd905fb5a
9 changed files with 255 additions and 50 deletions
@@ -145,10 +145,12 @@ void EpubReaderActivity::loop() {
  }

  // Long press CONFIRM (1s+) goes directly to KOReader sync when credentials are configured.
+  // We intentionally keep long-press on the richer compare flow so advanced
+  // conflict-resolution behavior stays available even after simplifying menu UX.
  // Without credentials, fall through to the regular menu on release.
  if (mappedInput.isPressed(MappedInputManager::Button::Confirm) &&
      mappedInput.getHeldTime() >= ReaderUtils::GO_HOME_MS && KOREADER_STORE.hasCredentials()) {
-    launchKOReaderSync();
+    launchKOReaderSync(SyncLaunchMode::COMPARE);
    return;
  }

@@ -458,16 +460,26 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
      requestUpdate();
      break;
    }
-    case EpubReaderMenuActivity::MenuAction::SYNC: {
+    case EpubReaderMenuActivity::MenuAction::PULL_REMOTE: {
+      // One-tap pull path: run network preconditions and apply remote progress
+      // directly instead of showing an intermediate chooser screen.
      if (KOREADER_STORE.hasCredentials()) {
-        launchKOReaderSync();
+        launchKOReaderSync(SyncLaunchMode::PULL_REMOTE);
+      }
+      break;
+    }
+    case EpubReaderMenuActivity::MenuAction::PUSH_LOCAL: {
+      // One-tap push path: run network preconditions and upload local progress
+      // directly for KOReader-like "sync now" behavior.
+      if (KOREADER_STORE.hasCredentials()) {
+        launchKOReaderSync(SyncLaunchMode::PUSH_LOCAL);
      }
      break;
    }
  }
 }

-void EpubReaderActivity::launchKOReaderSync() {
+void EpubReaderActivity::launchKOReaderSync(const SyncLaunchMode mode) {
  if (!epub) {
    return;
  }
@@ -499,10 +511,19 @@ void EpubReaderActivity::launchKOReaderSync() {
  LOG_DBG("ERS", "Pre-sync trim: spine=%d page=%d/%d heap=%lu", currentSpineIndex, currentPage, totalPages,
          static_cast<unsigned long>(esp_get_free_heap_size()));

-  startActivityForResult(std::make_unique<KOReaderSyncActivity>(renderer, mappedInput, std::shared_ptr<Epub>{},
-                                                                syncEpubPath,
-                                                                currentSpineIndex, currentPage, totalPages),
-                         [this](const ActivityResult& result) { handleSyncResult(result); });
+  // Map reader-level launch mode to activity-level intent once, then pass a
+  // stable intent into KOReaderSyncActivity so it can own the sync state machine.
+  KOReaderSyncActivity::SyncIntent syncIntent = KOReaderSyncActivity::SyncIntent::COMPARE;
+  if (mode == SyncLaunchMode::PULL_REMOTE) {
+    syncIntent = KOReaderSyncActivity::SyncIntent::PULL_REMOTE;
+  } else if (mode == SyncLaunchMode::PUSH_LOCAL) {
+    syncIntent = KOReaderSyncActivity::SyncIntent::PUSH_LOCAL;
+  }
+
+  startActivityForResult(
+      std::make_unique<KOReaderSyncActivity>(renderer, mappedInput, std::shared_ptr<Epub>{}, syncEpubPath,
+                                             currentSpineIndex, currentPage, totalPages, 0, false, syncIntent),
+      [this](const ActivityResult& result) { handleSyncResult(result); });
 }

 void EpubReaderActivity::handleSyncResult(const ActivityResult& result) {
@@ -9,6 +9,17 @@
 #include "activities/Activity.h"

 class EpubReaderActivity final : public Activity {
+  // Reader can launch sync in three UX modes:
+  // - COMPARE: legacy chooser (apply/upload) for power users.
+  // - PULL_REMOTE / PUSH_LOCAL: direct one-step actions from menu entries.
+  // Keeping this split in the caller avoids branching on menu semantics deep
+  // inside generic reader state handling.
+  enum class SyncLaunchMode {
+    COMPARE,
+    PULL_REMOTE,
+    PUSH_LOCAL,
+  };
+
  std::shared_ptr<Epub> epub;
  std::unique_ptr<Section> section = nullptr;
  int currentSpineIndex = 0;
@@ -58,7 +69,7 @@ class EpubReaderActivity final : public Activity {
  // Jump to a percentage of the book (0-100), mapping it to spine and page.
  void jumpToPercent(int percent);
  void onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction action);
-  void launchKOReaderSync();
+  void launchKOReaderSync(SyncLaunchMode mode = SyncLaunchMode::COMPARE);
  void handleSyncResult(const ActivityResult& result);
  void applyOrientation(uint8_t orientation);
  void toggleAutoPageTurn(uint8_t selectedPageTurnOption);
@@ -25,7 +25,7 @@ EpubReaderMenuActivity::EpubReaderMenuActivity(GfxRenderer& renderer, MappedInpu

 std::vector<EpubReaderMenuActivity::MenuItem> EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes) {
  std::vector<MenuItem> items;
-  items.reserve(10);
+  items.reserve(12);
  items.push_back({MenuAction::SELECT_CHAPTER, StrId::STR_SELECT_CHAPTER});
  if (hasFootnotes) {
    items.push_back({MenuAction::FOOTNOTES, StrId::STR_FOOTNOTES});
@@ -39,7 +39,8 @@ std::vector<EpubReaderMenuActivity::MenuItem> EpubReaderMenuActivity::buildMenuI
  items.push_back({MenuAction::DISPLAY_QR, StrId::STR_DISPLAY_QR});
  items.push_back({MenuAction::GO_HOME, StrId::STR_GO_HOME_BUTTON});
  if (KOREADER_STORE.hasCredentials()) {
-    items.push_back({MenuAction::SYNC, StrId::STR_SYNC_PROGRESS});
+    items.push_back({MenuAction::PULL_REMOTE, StrId::STR_PULL_PROGRESS_FROM_OTHER_DEVICES});
+    items.push_back({MenuAction::PUSH_LOCAL, StrId::STR_PUSH_PROGRESS_FROM_THIS_DEVICE});
  }
  items.push_back({MenuAction::DELETE_CACHE, StrId::STR_DELETE_CACHE});
  return items;
@@ -22,7 +22,8 @@ class EpubReaderMenuActivity final : public Activity {
    SCREENSHOT,
    DISPLAY_QR,
    GO_HOME,
-    SYNC,
+    PULL_REMOTE,
+    PUSH_LOCAL,
    DELETE_CACHE
  };

@@ -1,10 +1,10 @@
 #include "KOReaderSyncActivity.h"

+#include <FontCacheManager.h>
 #include <GfxRenderer.h>
 #include <HalClock.h>
 #include <I18n.h>
 #include <Logging.h>
-#include <FontCacheManager.h>
 #include <WiFi.h>
 #include <esp_heap_caps.h>
 #include <esp_system.h>
@@ -116,19 +116,52 @@ void KOReaderSyncActivity::performSync() {

  LOG_DBG("KOSync", "Document hash: %s", documentHash.c_str());

-  // Precompute local mapping before first network request so the expensive
-  // inflate/index work happens before TLS. This avoids a second local mapping
-  // pass later and keeps the upload path lightweight.
-  {
-    RenderLock lock(*this);
-    statusMessage = tr(STR_MAPPING_LOCAL);
+  // Local mapping is only needed for compare/upload paths.
+  // Pull-only mode can skip this expensive step and go straight to remote fetch.
+  if (syncIntent != SyncIntent::PULL_REMOTE) {
+    // Precompute local mapping before first network request so the expensive
+    // inflate/index work happens before TLS. This avoids a second local mapping
+    // pass later and keeps the upload path lightweight.
+    {
+      RenderLock lock(*this);
+      statusMessage = tr(STR_MAPPING_LOCAL);
+    }
+    requestUpdateAndWait();
+    computeLocalProgressAndChapter();
  }
-  requestUpdateAndWait();
-  computeLocalProgressAndChapter();

  // Drop EPUB state before HTTPS to maximize contiguous heap for TLS.
  releaseEpubForMapping();

+  // Push intent skips comparison UI but still warms an HTTP/TLS session first
+  // so PUT can reuse the connection instead of forcing a fresh handshake.
+  if (syncIntent == SyncIntent::PUSH_LOCAL) {
+    // Direct push previously started with no reusable HTTP/TLS session, forcing
+    // a fresh handshake in updateProgress. Compare flow often succeeds because
+    // upload reuses the GET session. Warm the session here so push can take the
+    // same reuse path without showing comparison UI.
+    KOReaderSyncClient::beginPersistentSession();
+    KOReaderProgress warmupProgress;
+    const auto warmupResult = KOReaderSyncClient::getProgress(documentHash, warmupProgress);
+    if (warmupResult != KOReaderSyncClient::OK && warmupResult != KOReaderSyncClient::NOT_FOUND) {
+      KOReaderSyncClient::endPersistentSession();
+      {
+        RenderLock lock(*this);
+        state = SYNC_FAILED;
+        statusMessage = KOReaderSyncClient::errorString(warmupResult);
+        const char* detail = KOReaderSyncClient::lastFailureDetail();
+        if (detail && detail[0]) {
+          statusMessage += " — ";
+          statusMessage += detail;
+        }
+      }
+      requestUpdate(true);
+      return;
+    }
+    performUpload();
+    return;
+  }
+
  {
    RenderLock lock(*this);
    statusMessage = tr(STR_FETCH_PROGRESS);
@@ -143,6 +176,19 @@ void KOReaderSyncActivity::performSync() {
  const auto result = KOReaderSyncClient::getProgress(documentHash, remoteProgress);

  if (result == KOReaderSyncClient::NOT_FOUND) {
+    if (syncIntent == SyncIntent::PULL_REMOTE) {
+      // Pull intent must not silently fall back to upload when server has no
+      // remote progress. Failing explicitly keeps action semantics predictable.
+      KOReaderSyncClient::endPersistentSession();
+      {
+        RenderLock lock(*this);
+        state = SYNC_FAILED;
+        statusMessage = tr(STR_NO_REMOTE_MSG);
+      }
+      requestUpdate(true);
+      return;
+    }
+
    // Keep session open so an immediate upload can reuse the same connection.
    // No remote progress - offer to upload
    {
@@ -189,6 +235,34 @@ void KOReaderSyncActivity::performSync() {
  }
  remoteChapterLabel = tr(STR_UNNAMED);

+  if (syncIntent == SyncIntent::PULL_REMOTE) {
+    // Pull intent applies immediately and exits. We bypass chooser UI to keep
+    // reader menu actions deterministic ("pull" always means apply remote).
+    if (!ensureRemotePositionMapped()) {
+      {
+        RenderLock lock(*this);
+        state = SYNC_FAILED;
+        statusMessage = tr(STR_SYNC_FAILED_MSG);
+      }
+      requestUpdate(true);
+      return;
+    }
+
+    // Preserve the apply result and show explicit confirmation before returning
+    // to the reader so users can tell pull succeeded.
+    setResult(SyncResult{remotePosition.spineIndex, remotePosition.pageNumber, remotePosition.paragraphIndex,
+                         remotePosition.hasParagraphIndex});
+    {
+      RenderLock lock(*this);
+      state = APPLY_COMPLETE;
+      uploadCompleteTime = millis();
+    }
+    requestUpdate(true);
+    return;
+  }
+
+  // Compare intent keeps the legacy chooser flow (apply vs upload), which is
+  // still useful for manual conflict decisions.
  // Local progress was precomputed before network; keep using the cached value.
  releaseEpubForMapping();

@@ -242,8 +316,9 @@ void KOReaderSyncActivity::performUpload() {
  // that only appear on PUT due to allocator state changes.
  logSyncMemSnapshot("before_updateProgress");

-  // Ensure a session exists for upload. When GET succeeded, this should reuse
-  // the existing connection and typically skip a second handshake.
+  // Ensure a session exists for upload. In compare flow this comes from the
+  // earlier GET; in direct-push flow it comes from the warmup GET above.
+  // In both cases, reuse avoids a second full TLS handshake.
  KOReaderSyncClient::beginPersistentSession();

  KOReaderProgress progress;
@@ -341,7 +416,7 @@ void KOReaderSyncActivity::render(RenderLock&&) {
  }

  if (state == SYNCING || state == UPLOADING) {
-    renderer.drawCenteredText(UI_10_FONT_ID, 300, statusMessage.c_str(), true, EpdFontFamily::BOLD);
+    GUI.drawPopup(renderer, statusMessage.c_str());
    renderer.displayBuffer();
    return;
  }
@@ -422,6 +497,15 @@ void KOReaderSyncActivity::render(RenderLock&&) {
    return;
  }

+  if (state == APPLY_COMPLETE) {
+    renderer.drawCenteredText(UI_10_FONT_ID, 300, tr(STR_PULL_SUCCESS), true, EpdFontFamily::BOLD);
+
+    const auto labels = mappedInput.mapLabels(tr(STR_BACK), "", "", "");
+    GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4);
+    renderer.displayBuffer();
+    return;
+  }
+
  if (state == SYNC_FAILED) {
    renderer.drawCenteredText(UI_10_FONT_ID, 280, tr(STR_SYNC_FAILED_MSG), true, EpdFontFamily::BOLD);
    renderer.drawCenteredText(UI_10_FONT_ID, 320, statusMessage.c_str());
@@ -488,9 +572,9 @@ void KOReaderSyncActivity::computeLocalProgressAndChapter() {
  localProgress = ProgressMapper::toKOReader(epub, localPos);

  const int localTocIndex = epub->getTocIndexForSpineIndex(currentSpineIndex);
-  localChapterLabel =
-      (localTocIndex >= 0) ? epub->getTocItem(localTocIndex).title
-                           : (std::string(tr(STR_SECTION_PREFIX)) + std::to_string(currentSpineIndex + 1));
+  localChapterLabel = (localTocIndex >= 0)
+                          ? epub->getTocItem(localTocIndex).title
+                          : (std::string(tr(STR_SECTION_PREFIX)) + std::to_string(currentSpineIndex + 1));
 }

 void KOReaderSyncActivity::computeRemoteChapter() {
@@ -498,20 +582,31 @@ void KOReaderSyncActivity::computeRemoteChapter() {
    return;
  }
  const int remoteTocIndex = epub->getTocIndexForSpineIndex(remotePosition.spineIndex);
-  remoteChapterLabel =
-      (remoteTocIndex >= 0) ? epub->getTocItem(remoteTocIndex).title
-                            : (std::string(tr(STR_SECTION_PREFIX)) + std::to_string(remotePosition.spineIndex + 1));
+  remoteChapterLabel = (remoteTocIndex >= 0)
+                           ? epub->getTocItem(remoteTocIndex).title
+                           : (std::string(tr(STR_SECTION_PREFIX)) + std::to_string(remotePosition.spineIndex + 1));
 }

 void KOReaderSyncActivity::loop() {
-  if (state == NO_CREDENTIALS || state == SYNC_FAILED || state == UPLOAD_COMPLETE) {
+  if (state == NO_CREDENTIALS || state == SYNC_FAILED || state == UPLOAD_COMPLETE || state == APPLY_COMPLETE) {
    if (mappedInput.wasReleased(MappedInputManager::Button::Back)) {
-      closeCancelled();
+      // APPLY_COMPLETE already has a valid SyncResult, so exit normally.
+      // Other terminal states are treated as cancelled when backing out.
+      if (state == APPLY_COMPLETE) {
+        finish();
+      } else {
+        closeCancelled();
+      }
      return;
    }

-    if (state == UPLOAD_COMPLETE && millis() - uploadCompleteTime >= 3000) {
-      closeCancelled();
+    if ((state == UPLOAD_COMPLETE || state == APPLY_COMPLETE) && millis() - uploadCompleteTime >= 3000) {
+      // Keep pull/apply result on auto-close; upload-complete remains cancel-style.
+      if (state == APPLY_COMPLETE) {
+        finish();
+      } else {
+        closeCancelled();
+      }
    }
    return;
  }
@@ -12,19 +12,37 @@
 /**
 * Activity for syncing reading progress with KOReader sync server.
 *
- * Flow:
+ * Shared pipeline:
 * 1. Connect to WiFi (if not connected)
- * 2. Calculate document hash
- * 3. Fetch remote progress
- * 4. Show comparison and options (Apply/Upload)
- * 5. Apply or upload progress
+ * 2. Optionally sync NTP (if stale)
+ * 3. Calculate document hash
+ *
+ * Intent-specific behavior:
+ * - 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.
+ * - PUSH_LOCAL: compute local mapping, warm session with GET, then upload via
+ *   reused connection to avoid a second full TLS handshake.
 */
 class KOReaderSyncActivity final : public Activity {
 public:
+  // Intent controls UI/behavior split for the same sync pipeline.
+  // - COMPARE: fetch then let user choose apply/upload.
+  // - PULL_REMOTE: fetch and apply immediately.
+  // - PUSH_LOCAL: upload immediately.
+  // This keeps WiFi/NTP/hash/memory handling centralized while enabling a
+  // simpler KOReader-like reader menu UX.
+  enum class SyncIntent {
+    COMPARE,
+    PULL_REMOTE,
+    PUSH_LOCAL,
+  };
+
  explicit KOReaderSyncActivity(GfxRenderer& renderer, MappedInputManager& mappedInput,
                                const std::shared_ptr<Epub>& epub, const std::string& epubPath, int currentSpineIndex,
                                int currentPage, int totalPagesInSpine, uint16_t paragraphIndex = 0,
-                                bool hasParagraphIndex = false)
+                                bool hasParagraphIndex = false, SyncIntent syncIntent = SyncIntent::COMPARE)
      : Activity("KOReaderSync", renderer, mappedInput),
        epub(epub),
        epubPath(epubPath),
@@ -33,6 +51,7 @@ class KOReaderSyncActivity final : public Activity {
        totalPagesInSpine(totalPagesInSpine),
        localParagraphIndex(paragraphIndex),
        hasLocalParagraphIndex(hasParagraphIndex),
+        syncIntent(syncIntent),
        remoteProgress{},
        remotePosition{},
        localProgress{} {}
@@ -51,6 +70,7 @@ class KOReaderSyncActivity final : public Activity {
    SHOWING_RESULT,
    UPLOADING,
    UPLOAD_COMPLETE,
+    APPLY_COMPLETE,
    NO_REMOTE_PROGRESS,
    SYNC_FAILED,
    NO_CREDENTIALS
@@ -63,6 +83,7 @@ class KOReaderSyncActivity final : public Activity {
  int totalPagesInSpine;
  uint16_t localParagraphIndex;
  bool hasLocalParagraphIndex;
+  SyncIntent syncIntent = SyncIntent::COMPARE;

  State state = WIFI_SELECTION;
  std::string statusMessage;
@@ -82,7 +103,7 @@ class KOReaderSyncActivity final : public Activity {
  // Selection in result screen (0=Apply, 1=Upload)
  int selectedOption = 0;

-  // Timestamp when UPLOAD_COMPLETE state was entered (for auto-close)
+  // Timestamp when completion state was entered (for auto-close)
  unsigned long uploadCompleteTime = 0;
  bool closeRequested = false;