src/activities/reader/EpubReaderPercentSelectionActivity.h (1)

25-30: Remove stale RTOS-related comments.

The comments at Line 27 and Line 29 refer to fields that no longer exist and are now misleading.

🧹 Proposed cleanup

-  // Render dirty flag for the task loop.
-
-  // FreeRTOS task and mutex for rendering.
-

src/activities/settings/ClearCacheActivity.cpp (1)

117-124: Use RenderLock instead of manual mutex operations for consistency.

The renderingMutex still exists and the manual xSemaphoreTake/xSemaphoreGive calls will compile and work correctly. However, other parts of the codebase use the RenderLock RAII wrapper (e.g., Activity.cpp:12, Activity.cpp:30, ActivityWithSubactivity.cpp:14). Consider refactoring to use RenderLock lock(*this); at line 121 instead of manual mutex operations for consistency with the established pattern.

src/activities/Activity.h (1)

20-21: Verify [[noreturn]] on the trampoline function.

The [[noreturn]] attribute on renderTaskTrampoline is technically correct since it calls renderTaskLoop() which never returns. However, from the implementation in Activity.cpp, the trampoline returns void and calls the loop:

void Activity::renderTaskTrampoline(void* param) {
  auto* self = static_cast<Activity*>(param);
  return self->renderTaskLoop();  // returns void, then calls noreturn
}

The attribute works here because control never returns from renderTaskLoop(), but marking a void-returning function as [[noreturn]] when it delegates to another [[noreturn]] function is unconventional. Consider removing [[noreturn]] from the trampoline or changing the return statement to just self->renderTaskLoop(); without return.

src/activities/reader/EpubReaderActivity.cpp (1)

600-603: Potential unbounded recursion on persistent load failures.

The recursive return render() call could cause stack overflow if the page file remains corrupt after cache clearing. Consider adding a retry counter or returning an error state instead.

🔧 Suggested safeguard

   {
     auto p = section->loadPageFromSectionFile();
     if (!p) {
       Serial.printf("[%lu] [ERS] Failed to load page from SD - clearing section cache\n", millis());
       section->clearCache();
       section.reset();
-      return render();
+      // Request another render attempt rather than recursing
+      requestUpdate();
+      return;
     }

src/activities/home/MyLibraryActivity.cpp (1)

86-88: Minor: Double semicolon.

Line 88 has a stray semicolon after the statement.

🧹 Fix

   const bool upReleased = mappedInput.wasReleased(MappedInputManager::Button::Left) ||
                           mappedInput.wasReleased(MappedInputManager::Button::Up);
-  ;

src/activities/settings/OtaUpdateActivity.cpp (1)

23-50: Replace raw semaphore calls with RenderLock for state updates throughout this activity.

This keeps mutex ownership scoped and reduces the risk of future imbalance. The pattern is established across the codebase and is the idiomatic way to protect critical sections.

Apply to both occurrences: lines 23-50 and 159-178.

♻️ Example refactor for one transition (apply similarly to other state changes)

-  xSemaphoreTake(renderingMutex, portMAX_DELAY);
-  state = CHECKING_FOR_UPDATE;
-  xSemaphoreGive(renderingMutex);
-  requestUpdate();
+  {
+    Activity::RenderLock lock(*this);
+    state = CHECKING_FOR_UPDATE;
+  }
+  requestUpdate();

src/activities/reader/KOReaderSyncActivity.cpp (1)

51-58: Migrate manual mutex pairs to Activity::RenderLock RAII pattern.
RenderLock is now the standard approach across the codebase for handling rendering synchronization. This file has multiple manual xSemaphoreTake/xSemaphoreGive pairs that should be wrapped in RenderLock to reduce the risk of missed releases and maintain consistency with the render sync model.

Example refactor for lines 53–56

-  xSemaphoreTake(renderingMutex, portMAX_DELAY);
-  state = SYNCING;
-  statusMessage = "Syncing time...";
-  xSemaphoreGive(renderingMutex);
+  {
+    Activity::RenderLock lock(*this);
+    state = SYNCING;
+    statusMessage = "Syncing time...";
+  }

Apply this pattern to all remaining manual mutex pairs in this file (at least 13 other occurrences).

src/activities/Activity.h (1)

45-53: RenderLock is missing move semantics despite being passed by rvalue reference.

The render(RenderLock&&) signature suggests move semantics, but RenderLock only has deleted copy operations without explicit move constructor/assignment. While this works because the compiler won't implicitly generate move operations when copy operations are deleted, it may cause confusion and prevent storing the lock in certain scenarios.

Consider explicitly defaulting or defining move operations for clarity:

♻️ Suggested improvement

   public:
     explicit RenderLock(Activity& activity);
     RenderLock(const RenderLock&) = delete;
     RenderLock& operator=(const RenderLock&) = delete;
+    RenderLock(RenderLock&&) = default;
+    RenderLock& operator=(RenderLock&&) = delete;
     ~RenderLock();

src/activities/reader/EpubReaderActivity.cpp (1)

222-231: Consider wrapping manual mutex operations with a scoped RAII helper for consistency.

This file contains 6 paired xSemaphoreTake/xSemaphoreGive calls (at lines 224-228, 244-248, 256-260, 328-333, 406-423, 461-477) that protect inter-task synchronization in the loop() function. While these operations are correctly paired and necessary to prevent concurrent access to the section object during render, the manual mutex handling is inconsistent with the RAII RenderLock pattern used elsewhere.

Notably, other reader activities (ReaderActivity, TxtReaderActivity, XtcReaderActivity) do not employ this pattern, making EpubReaderActivity an outlier. A scoped mutex wrapper (e.g., a simple RAII guard) around these critical sections would improve code consistency and reduce error-proneness without requiring architectural changes.

src/activities/settings/ButtonRemapActivity.cpp (1)

64-93: Remove unnecessary block braces; fix typo in FIXME.

The block {} at lines 64-93 no longer serves a scoping purpose (no RAII object or early-exit requiring it) and can be removed to reduce indentation. Also, minor typo: "one it's" → "once it's".

♻️ Suggested cleanup

-  {
-    // Wait for the UI to refresh before accepting another assignment.
-    // This avoids rapid double-presses that can advance the step without a visible redraw.
-    delay(100);  // FIXME `@ngxson` : use requestUpdateAndWait() one it's implemented
+  // Wait for the UI to refresh before accepting another assignment.
+  // This avoids rapid double-presses that can advance the step without a visible redraw.
+  delay(100);  // FIXME `@ngxson` : use requestUpdateAndWait() once it's implemented

-    // Wait for a front button press to assign to the current role.
-    const int pressedButton = mappedInput.getPressedFrontButton();
-    if (pressedButton < 0) {
-      return;
-    }
+  // Wait for a front button press to assign to the current role.
+  const int pressedButton = mappedInput.getPressedFrontButton();
+  if (pressedButton < 0) {
+    return;
+  }

-    // Update temporary mapping and advance the remap step.
-    // Only accept the press if this hardware button isn't already assigned elsewhere.
-    if (!validateUnassigned(static_cast<uint8_t>(pressedButton))) {
-      requestUpdate();
-      return;
-    }
-    tempMapping[currentStep] = static_cast<uint8_t>(pressedButton);
-    currentStep++;
-
-    if (currentStep >= kRoleCount) {
-      // All roles assigned; save to settings and exit.
-      applyTempMapping();
-      SETTINGS.saveToFile();
-      onBack();
-      return;
-    }
+  // Update temporary mapping and advance the remap step.
+  // Only accept the press if this hardware button isn't already assigned elsewhere.
+  if (!validateUnassigned(static_cast<uint8_t>(pressedButton))) {
+    requestUpdate();
+    return;
+  }
+  tempMapping[currentStep] = static_cast<uint8_t>(pressedButton);
+  currentStep++;

-    requestUpdate();
+  if (currentStep >= kRoleCount) {
+    // All roles assigned; save to settings and exit.
+    applyTempMapping();
+    SETTINGS.saveToFile();
+    onBack();
+    return;
   }
+
+  requestUpdate();

src/activities/settings/OtaUpdateActivity.cpp (1)

23-50: Use RenderLock RAII for these state transitions to complete the migration.

The onWifiSelectionComplete() method still uses manual semaphore pairs while the rest of this file has migrated to Activity::RenderLock. For consistency and to protect against semaphore imbalance on early returns, replace the manual pairs with RenderLock here as well.

♻️ Suggested pattern (apply similarly to all state updates in this method)

-  xSemaphoreTake(renderingMutex, portMAX_DELAY);
-  state = CHECKING_FOR_UPDATE;
-  xSemaphoreGive(renderingMutex);
+  {
+    RenderLock lock(*this);
+    state = CHECKING_FOR_UPDATE;
+  }

src/activities/settings/ClearCacheActivity.cpp (1)

121-126: Remove the 100ms delay after requestUpdate() or replace with proper synchronization.

The event-driven render task is signaled via xTaskNotify(), which wakes it immediately; an artificial delay provides no benefit and unnecessarily blocks the calling thread. Once requestUpdateAndWait() is implemented as planned, use that instead to wait for render completion without wasting CPU cycles.

src/activities/reader/KOReaderSyncActivity.cpp (1)

88-93: Remove blocking delays after requestUpdate() or replace with requestUpdateAndWait().

The delay(100) calls at lines 92 and 138 block the sync task unnecessarily. Since rendering runs in a dedicated task, waiting is not needed. Either drop the delay entirely or implement requestUpdateAndWait() to wait for the render to complete. This pattern appears across multiple activities (see WifiSelectionActivity, ButtonRemapActivity, ClearCacheActivity, OtaUpdateActivity).

src/activities/settings/OtaUpdateActivity.cpp (2)

23-50: Complete RenderLock migration for state transitions.

Manual xSemaphoreTake/xSemaphoreGive usage around Line 23 (and the other state updates in this method) is a leftover from the old render model. Prefer Activity::RenderLock to keep locking consistent with the centralized renderer and prevent leaks on future early returns. Apply the same pattern to the NO_UPDATE branch as well.

♻️ Suggested pattern for each state update

-  xSemaphoreTake(renderingMutex, portMAX_DELAY);
-  state = CHECKING_FOR_UPDATE;
-  xSemaphoreGive(renderingMutex);
+  {
+    RenderLock lock(*this);
+    state = CHECKING_FOR_UPDATE;
+  }

-    xSemaphoreTake(renderingMutex, portMAX_DELAY);
-    state = FAILED;
-    xSemaphoreGive(renderingMutex);
+    {
+      RenderLock lock(*this);
+      state = FAILED;
+    }

-  xSemaphoreTake(renderingMutex, portMAX_DELAY);
-  state = WAITING_CONFIRMATION;
-  xSemaphoreGive(renderingMutex);
+  {
+    RenderLock lock(*this);
+    state = WAITING_CONFIRMATION;
+  }

---

169-170: Avoid fixed delays after requestUpdate in the activity thread.

Line 169 blocks the activity loop for 100 ms; with centralized rendering this can stall input and background work. Same pattern at Line 27. Consider a non-blocking state step or requestUpdateAndWait once available.