refactor: implement ActivityManager#1016
Merged
ngxson merged 47 commits intocrosspoint-reader:masterfrom Feb 27, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes activity lifecycle and rendering under a new ActivityManager with a render task and RAII RenderLock; removes ActivityWithSubactivity and per-activity render tasks; introduces ActivityResult / startActivityForResult / setResult / finish APIs; migrates activities to stack/result-driven navigation and unified render signatures. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant AM as ActivityManager
participant RT as RenderTask
participant Mutex as RenderMutex
participant Act as CurrentActivity
App->>AM: begin()
Note over AM,RT: create render task (trampoline -> renderTaskLoop)
loop Main loop
App->>AM: loop()
alt Pending action (Push/Pop/Replace)
AM->>AM: process pending action
AM->>Act: onEnter()
end
Act->>AM: requestUpdate()
AM->>RT: notify render task
end
RT->>Mutex: lock()
RT->>AM: invoke renderTaskLoop
AM->>Act: render(RenderLock&&)
Act-->>AM: render complete
RT->>Mutex: unlock()
alt Activity finishes
Act->>AM: setResult(result)
Act->>AM: finish()
AM->>Act: onExit()
AM->>AM: invoke resultHandler(result) if present
end
sequenceDiagram
participant Parent as ParentActivity
participant AM as ActivityManager
participant Child as ChildActivity
Parent->>AM: startActivityForResult(unique_ptr<Child>, handler)
AM->>Child: pushActivity / onEnter()
Child->>Child: user interaction
Child->>Child: setResult(result)
Child->>AM: finish()
AM->>Parent: invoke handler(result)
AM->>Child: onExit() / delete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ngxson
commented
Feb 21, 2026
Comment on lines
-121
to
-141
| // Pass input responsibility to sub activity if exists | ||
| if (subActivity) { | ||
| subActivity->loop(); | ||
| // Deferred exit: process after subActivity->loop() returns to avoid use-after-free | ||
| if (pendingSubactivityExit) { | ||
| pendingSubactivityExit = false; | ||
| exitActivity(); | ||
| requestUpdate(); | ||
| skipNextButtonCheck = true; // Skip button processing to ignore stale events | ||
| } | ||
| // Deferred go home: process after subActivity->loop() returns to avoid race condition | ||
| if (pendingGoHome) { | ||
| pendingGoHome = false; | ||
| exitActivity(); | ||
| if (onGoHome) { | ||
| onGoHome(); | ||
| } | ||
| return; // Don't access 'this' after callback | ||
| } | ||
| return; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
A more practical example on how the ActivityManager can be useful is here. This whole logic is now removed and they are handled in a more generic way on ActivityManager
Contributor
Author
|
@daveallie Sorry for yet another big refactoring, but I believe this is quite necessary as it removes some dangerous patterns in the code base. Appreciate if we can prioritize this right after 1.1.0 release. CC @crosspoint-reader/firmware-maintainers too, appreciate if someone can do testings here. I already did some testings on my side, so far so good. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/activities/browser/OpdsBookBrowserActivity.cpp (1)
371-386:⚠️ Potential issue | 🟡 MinorMissing
requestUpdate()on WiFi selection failure path.When WiFi selection fails (line 378-386), the state is set to
ERRORbutrequestUpdate()is not called, unlike the success path at line 376. This could leave the UI displaying stale content.🐛 Proposed fix
} else { LOG_DBG("OPDS", "WiFi selection cancelled/failed"); // Force disconnect to ensure clean state for next retry // This prevents stale connection status from interfering WiFi.disconnect(); WiFi.mode(WIFI_OFF); state = BrowserState::ERROR; errorMessage = tr(STR_WIFI_CONN_FAILED); + requestUpdate(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/browser/OpdsBookBrowserActivity.cpp` around lines 371 - 386, In onWifiSelectionComplete (OpdsBookBrowserActivity::onWifiSelectionComplete) the failure branch sets state = BrowserState::ERROR and updates errorMessage but does not call requestUpdate(), leaving the UI potentially stale; add a call to requestUpdate() after setting errorMessage (and after performing WiFi.disconnect()/WiFi.mode(WIFI_OFF)) so the UI is refreshed when the error state is entered.
🧹 Nitpick comments (7)
src/activities/network/NetworkModeSelectionActivity.h (1)
8-9: Remove stale commented-out enum stub.🧹 Proposed cleanup
-// // Enum for network mode selection -// enum class NetworkMode { JOIN_NETWORK, CONNECT_CALIBRE, CREATE_HOTSPOT };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/network/NetworkModeSelectionActivity.h` around lines 8 - 9, Remove the stale commented-out enum stub for NetworkMode: delete the commented lines that declare "enum class NetworkMode { JOIN_NETWORK, CONNECT_CALIBRE, CREATE_HOTSPOT }" so the header no longer contains dead/commented-out code; if this enum is needed elsewhere, ensure the real definition (NetworkMode) resides in the proper header or source and update includes instead of keeping the commented stub.src/activities/ActivityResult.h (1)
7-8: Track the TODO for NetworkMode placement.Consider moving the enum to a dedicated network/activity header or linking this TODO to an issue.
If you want, I can draft an issue or propose the target header location.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/ActivityResult.h` around lines 7 - 8, The TODO indicates NetworkMode (enum class NetworkMode { JOIN_NETWORK, CONNECT_CALIBRE, CREATE_HOTSPOT }) is in the wrong place; move it into a dedicated network-related header (e.g., create a new NetworkMode.h or NetworkEnums.h) and replace the enum in ActivityResult.h with an include of that new header, update any files that reference NetworkMode to include the new header, and remove the TODO; also link this change to an issue or add a brief comment referencing an issue ID if you create one so the relocation is tracked.src/activities/util/KeyboardEntryActivity.cpp (1)
321-336: Avoid std::move on a const ref (it won’t move).Either pass by value and move, or just assign.
♻️ Minimal fix (keep const ref)
- result.inputText = std::move(text); + result.inputText = text;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/util/KeyboardEntryActivity.cpp` around lines 321 - 336, The onComplete implementation incorrectly uses std::move on a const std::string& (parameter text), which won’t move; either change the onComplete signature to take text by value (std::string text) and keep result.inputText = std::move(text) to enable move semantics, or keep the const std::string& signature and assign without move (result.inputText = text); update the onComplete function accordingly and leave onCancel unchanged (references: KeyboardEntryActivity::onComplete, ActivityResult::inputText, setResult, finish).src/activities/reader/EpubReaderPercentSelectionActivity.cpp (1)
53-58: Direction mapping may be counter-intuitive for a slider.
Upincreases percent whileDowndecreases it. For a horizontal slider UI, users might expectUp/Downto be swapped or to have no effect. However, this may be intentional for the device's button layout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderPercentSelectionActivity.cpp` around lines 53 - 58, The Up/Down handlers currently call adjustPercent(kLargeStep) and adjustPercent(-kLargeStep) which is counter-intuitive for a horizontal slider; either swap the Up/Down mappings so Up calls adjustPercent(-kLargeStep) and Down calls adjustPercent(kLargeStep), or remove/disable Up/Down handling if vertical input should be ignored. Update the buttonNavigator.onPressAndContinuous calls that reference MappedInputManager::Button::Up and MappedInputManager::Button::Down and ensure adjustPercent, kLargeStep and any inline comments reflect the chosen behavior.src/activities/ActivityManager.h (1)
27-28: Redundant forward declarations.
ActivityandRenderLockare already included viaActivity.h(line 11) andRenderLock.h(line 14). These forward declarations are unnecessary.🔧 Suggested removal
-class Activity; // forward declaration -class RenderLock; // forward declaration - /** * ActivityManager🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/ActivityManager.h` around lines 27 - 28, Remove the redundant forward declarations "class Activity;" and "class RenderLock;" from ActivityManager.h because Activity.h and RenderLock.h are already included (so the forward declarations are unnecessary); delete those two lines and rebuild to confirm no remaining references require forward declarations elsewhere in this header or switch to forward declarations only if you remove the corresponding `#include` lines instead.src/activities/browser/OpdsBookBrowserActivity.cpp (1)
49-54: Unnecessary base class call.
Activity::loop()is an empty virtual method (as seen in Activity.h:virtual void loop() {}). This call has no effect and could be removed for clarity. However, it's harmless.♻️ Optional: Remove unnecessary base call
// Handle WiFi selection subactivity if (state == BrowserState::WIFI_SELECTION) { - Activity::loop(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/browser/OpdsBookBrowserActivity.cpp` around lines 49 - 54, The call to the empty base implementation Activity::loop() inside OpdsBookBrowserActivity::loop when state == BrowserState::WIFI_SELECTION is unnecessary; remove the Activity::loop() invocation and simply return after handling the WIFI_SELECTION branch to simplify the method (locate the conditional in OpdsBookBrowserActivity::loop and delete the Activity::loop() call).src/activities/home/HomeActivity.cpp (1)
262-266: Optional: reuse base Activity::onSelectBook to avoid duplication.This method matches the base implementation exactly, so you could forward to the base to keep logic centralized.
♻️ Suggested tweak
void HomeActivity::onSelectBook(const std::string& path) { - Intent intent; - intent.path = path; - activityManager.goToReader(std::move(intent)); + Activity::onSelectBook(path); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/home/HomeActivity.cpp` around lines 262 - 266, HomeActivity::onSelectBook duplicates the base Activity::onSelectBook behavior; replace the body to forward to the base implementation instead of duplicating logic—call Activity::onSelectBook(path) (or the appropriate base class qualifier) so Intent construction and the activityManager.goToReader call remain centralized in Activity::onSelectBook.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (63)
src/activities/Activity.cppsrc/activities/Activity.hsrc/activities/ActivityManager.cppsrc/activities/ActivityManager.hsrc/activities/ActivityResult.hsrc/activities/ActivityWithSubactivity.cppsrc/activities/ActivityWithSubactivity.hsrc/activities/RenderLock.hsrc/activities/browser/OpdsBookBrowserActivity.cppsrc/activities/browser/OpdsBookBrowserActivity.hsrc/activities/home/HomeActivity.cppsrc/activities/home/HomeActivity.hsrc/activities/home/MyLibraryActivity.cppsrc/activities/home/MyLibraryActivity.hsrc/activities/home/RecentBooksActivity.cppsrc/activities/home/RecentBooksActivity.hsrc/activities/network/CalibreConnectActivity.cppsrc/activities/network/CalibreConnectActivity.hsrc/activities/network/CrossPointWebServerActivity.cppsrc/activities/network/CrossPointWebServerActivity.hsrc/activities/network/NetworkModeSelectionActivity.cppsrc/activities/network/NetworkModeSelectionActivity.hsrc/activities/network/WifiSelectionActivity.cppsrc/activities/network/WifiSelectionActivity.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderChapterSelectionActivity.cppsrc/activities/reader/EpubReaderChapterSelectionActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderPercentSelectionActivity.cppsrc/activities/reader/EpubReaderPercentSelectionActivity.hsrc/activities/reader/KOReaderSyncActivity.cppsrc/activities/reader/KOReaderSyncActivity.hsrc/activities/reader/ReaderActivity.cppsrc/activities/reader/ReaderActivity.hsrc/activities/reader/TxtReaderActivity.cppsrc/activities/reader/TxtReaderActivity.hsrc/activities/reader/XtcReaderActivity.cppsrc/activities/reader/XtcReaderActivity.hsrc/activities/reader/XtcReaderChapterSelectionActivity.cppsrc/activities/reader/XtcReaderChapterSelectionActivity.hsrc/activities/settings/ButtonRemapActivity.cppsrc/activities/settings/ButtonRemapActivity.hsrc/activities/settings/CalibreSettingsActivity.cppsrc/activities/settings/CalibreSettingsActivity.hsrc/activities/settings/ClearCacheActivity.cppsrc/activities/settings/ClearCacheActivity.hsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/settings/KOReaderAuthActivity.hsrc/activities/settings/KOReaderSettingsActivity.cppsrc/activities/settings/KOReaderSettingsActivity.hsrc/activities/settings/LanguageSelectActivity.cppsrc/activities/settings/LanguageSelectActivity.hsrc/activities/settings/OtaUpdateActivity.cppsrc/activities/settings/OtaUpdateActivity.hsrc/activities/settings/SettingsActivity.cppsrc/activities/settings/SettingsActivity.hsrc/activities/util/BmpViewerActivity.cppsrc/activities/util/BmpViewerActivity.hsrc/activities/util/KeyboardEntryActivity.cppsrc/activities/util/KeyboardEntryActivity.hsrc/main.cpp
💤 Files with no reviewable changes (2)
- src/activities/ActivityWithSubactivity.cpp
- src/activities/ActivityWithSubactivity.h
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/util/BmpViewerActivity.hsrc/activities/home/MyLibraryActivity.cppsrc/activities/home/RecentBooksActivity.hsrc/activities/browser/OpdsBookBrowserActivity.cppsrc/activities/settings/ClearCacheActivity.hsrc/activities/reader/XtcReaderChapterSelectionActivity.hsrc/activities/reader/EpubReaderPercentSelectionActivity.cppsrc/activities/settings/ButtonRemapActivity.hsrc/activities/ActivityResult.hsrc/activities/Activity.cppsrc/activities/reader/ReaderActivity.cppsrc/activities/network/CalibreConnectActivity.hsrc/activities/util/BmpViewerActivity.cppsrc/activities/reader/KOReaderSyncActivity.cppsrc/activities/home/RecentBooksActivity.cppsrc/activities/util/KeyboardEntryActivity.cppsrc/activities/settings/KOReaderSettingsActivity.hsrc/activities/home/HomeActivity.cppsrc/activities/settings/KOReaderAuthActivity.hsrc/activities/settings/OtaUpdateActivity.cppsrc/activities/reader/ReaderActivity.hsrc/activities/reader/KOReaderSyncActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/settings/LanguageSelectActivity.hsrc/activities/settings/CalibreSettingsActivity.cppsrc/main.cppsrc/activities/settings/SettingsActivity.cppsrc/activities/home/MyLibraryActivity.hsrc/activities/home/HomeActivity.hsrc/activities/settings/KOReaderSettingsActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/browser/OpdsBookBrowserActivity.hsrc/activities/reader/EpubReaderChapterSelectionActivity.cppsrc/activities/network/NetworkModeSelectionActivity.hsrc/activities/network/NetworkModeSelectionActivity.cppsrc/activities/network/CalibreConnectActivity.cppsrc/activities/reader/TxtReaderActivity.cppsrc/activities/reader/EpubReaderChapterSelectionActivity.hsrc/activities/settings/ClearCacheActivity.cppsrc/activities/Activity.hsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/reader/EpubReaderPercentSelectionActivity.hsrc/activities/settings/CalibreSettingsActivity.hsrc/activities/settings/SettingsActivity.hsrc/activities/ActivityManager.cppsrc/activities/ActivityManager.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/XtcReaderChapterSelectionActivity.cppsrc/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/XtcReaderActivity.hsrc/activities/reader/XtcReaderActivity.cppsrc/activities/reader/TxtReaderActivity.hsrc/activities/util/KeyboardEntryActivity.hsrc/activities/network/CrossPointWebServerActivity.cppsrc/activities/network/WifiSelectionActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().
Applied to files:
src/activities/browser/OpdsBookBrowserActivity.cppsrc/activities/reader/EpubReaderPercentSelectionActivity.cppsrc/activities/Activity.cppsrc/activities/reader/ReaderActivity.cppsrc/activities/reader/KOReaderSyncActivity.cppsrc/activities/home/RecentBooksActivity.cppsrc/activities/reader/EpubReaderMenuActivity.cppsrc/main.cppsrc/activities/settings/KOReaderSettingsActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderChapterSelectionActivity.cppsrc/activities/network/CalibreConnectActivity.cppsrc/activities/reader/TxtReaderActivity.cppsrc/activities/reader/EpubReaderChapterSelectionActivity.hsrc/activities/settings/ClearCacheActivity.cppsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/reader/EpubReaderPercentSelectionActivity.hsrc/activities/reader/XtcReaderChapterSelectionActivity.cppsrc/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/XtcReaderActivity.hsrc/activities/reader/XtcReaderActivity.cpp
📚 Learning: 2026-02-18T15:43:07.515Z
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Applied to files:
src/activities/browser/OpdsBookBrowserActivity.cppsrc/activities/reader/XtcReaderChapterSelectionActivity.hsrc/activities/reader/EpubReaderPercentSelectionActivity.cppsrc/activities/Activity.cppsrc/activities/reader/ReaderActivity.cppsrc/activities/reader/KOReaderSyncActivity.cppsrc/activities/settings/KOReaderSettingsActivity.hsrc/activities/settings/KOReaderAuthActivity.hsrc/activities/reader/ReaderActivity.hsrc/activities/reader/KOReaderSyncActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/settings/CalibreSettingsActivity.cppsrc/main.cppsrc/activities/settings/SettingsActivity.cppsrc/activities/settings/KOReaderSettingsActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/browser/OpdsBookBrowserActivity.hsrc/activities/reader/EpubReaderChapterSelectionActivity.cppsrc/activities/reader/TxtReaderActivity.cppsrc/activities/reader/EpubReaderChapterSelectionActivity.hsrc/activities/Activity.hsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/reader/EpubReaderPercentSelectionActivity.hsrc/activities/settings/CalibreSettingsActivity.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/XtcReaderChapterSelectionActivity.cppsrc/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/XtcReaderActivity.hsrc/activities/reader/XtcReaderActivity.cppsrc/activities/reader/TxtReaderActivity.h
📚 Learning: 2026-02-12T06:57:35.955Z
Learnt from: CaptainFrito
Repo: crosspoint-reader/crosspoint-reader PR: 725
File: src/activities/network/CalibreConnectActivity.cpp:218-264
Timestamp: 2026-02-12T06:57:35.955Z
Learning: In src/activities/network/CalibreConnectActivity.cpp, button hints (« Exit) are intentionally omitted from the SERVER_STARTING and ERROR states—only the SERVER_RUNNING state displays navigation hints. This is a deliberate design decision by the author.
Applied to files:
src/activities/browser/OpdsBookBrowserActivity.cppsrc/activities/ActivityResult.hsrc/activities/reader/ReaderActivity.cppsrc/activities/network/CalibreConnectActivity.hsrc/activities/reader/KOReaderSyncActivity.cppsrc/activities/settings/KOReaderSettingsActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/settings/CalibreSettingsActivity.cppsrc/main.cppsrc/activities/settings/KOReaderSettingsActivity.cppsrc/activities/browser/OpdsBookBrowserActivity.hsrc/activities/reader/EpubReaderChapterSelectionActivity.cppsrc/activities/network/NetworkModeSelectionActivity.hsrc/activities/network/CalibreConnectActivity.cppsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/settings/CalibreSettingsActivity.hsrc/activities/network/CrossPointWebServerActivity.hsrc/activities/reader/XtcReaderActivity.cppsrc/activities/network/CrossPointWebServerActivity.cppsrc/activities/network/WifiSelectionActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In EpubReaderActivity.cpp, renderContents() behavior is intentional: if uncached images exist, Phase 1 should call displayBuffer(forceFullRefresh) to perform a HALF_REFRESH and clear ghosting; Phase 2 should call renderer.displayBuffer() without forceFullRefresh for a faster refresh. Grayscale anti-aliasing is applied separately after renderContents() via displayGrayBuffer(). This guidance applies specifically to this file and situation.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
🧬 Code graph analysis (43)
src/activities/util/BmpViewerActivity.h (2)
src/activities/util/BmpViewerActivity.cpp (1)
BmpViewerActivity(11-12)src/main.cpp (1)
renderer(30-30)
src/activities/home/MyLibraryActivity.cpp (7)
src/activities/settings/KOReaderSettingsActivity.cpp (2)
render(113-153)render(113-113)src/activities/network/WifiSelectionActivity.cpp (2)
render(457-507)render(457-457)src/activities/reader/XtcReaderActivity.cpp (2)
render(128-144)render(128-128)src/activities/network/NetworkModeSelectionActivity.cpp (2)
render(57-85)render(57-57)src/activities/home/HomeActivity.cpp (2)
render(214-260)render(214-214)src/activities/home/RecentBooksActivity.cpp (2)
render(86-113)render(86-86)src/activities/reader/TxtReaderActivity.cpp (2)
render(339-372)render(339-339)
src/activities/home/RecentBooksActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/browser/OpdsBookBrowserActivity.h (1)
onEnter(30-54)
src/activities/browser/OpdsBookBrowserActivity.cpp (3)
src/activities/reader/KOReaderSyncActivity.cpp (6)
onEnter(184-227)onEnter(184-184)render(239-351)render(239-239)onWifiSelectionComplete(44-73)onWifiSelectionComplete(44-44)src/activities/Activity.cpp (2)
startActivityForResult(24-27)startActivityForResult(24-24)src/activities/network/CrossPointWebServerActivity.cpp (2)
onWifiSelectionComplete(144-165)onWifiSelectionComplete(144-144)
src/activities/settings/ClearCacheActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/Activity.cpp (2)
finish(31-31)finish(31-31)
src/activities/settings/ButtonRemapActivity.cpp (8)
src/activities/settings/KOReaderAuthActivity.cpp (2)
render(88-112)render(88-88)src/activities/settings/KOReaderSettingsActivity.cpp (2)
render(113-153)render(113-113)src/activities/network/WifiSelectionActivity.cpp (2)
render(457-507)render(457-457)src/activities/settings/CalibreSettingsActivity.cpp (2)
render(88-124)render(88-88)src/activities/settings/OtaUpdateActivity.cpp (2)
render(76-135)render(76-76)src/activities/network/CalibreConnectActivity.cpp (2)
render(161-220)render(161-161)src/activities/network/CrossPointWebServerActivity.cpp (2)
render(328-351)render(328-328)src/activities/settings/SettingsActivity.cpp (2)
render(196-245)render(196-196)
src/activities/reader/XtcReaderChapterSelectionActivity.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/reader/EpubReaderPercentSelectionActivity.cpp (5)
src/activities/settings/CalibreSettingsActivity.cpp (6)
onEnter(19-24)onEnter(19-19)onExit(26-26)onExit(26-26)render(88-124)render(88-88)src/activities/network/NetworkModeSelectionActivity.cpp (6)
onEnter(14-22)onEnter(14-14)onExit(24-24)onExit(24-24)render(57-85)render(57-57)src/activities/reader/EpubReaderMenuActivity.cpp (6)
onEnter(10-13)onEnter(10-10)onExit(15-15)onExit(15-15)render(54-117)render(54-54)src/activities/Activity.cpp (6)
requestUpdate(9-9)requestUpdate(9-9)setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)
src/activities/settings/ButtonRemapActivity.h (3)
src/activities/Activity.h (1)
Activity(14-53)src/activities/settings/ButtonRemapActivity.cpp (8)
onEnter(20-32)onEnter(20-20)onExit(34-34)onExit(34-34)loop(36-95)loop(36-36)render(97-150)render(97-97)src/activities/Activity.cpp (2)
finish(31-31)finish(31-31)
src/activities/settings/LanguageSelectActivity.cpp (6)
src/activities/reader/EpubReaderActivity.cpp (2)
render(398-529)render(398-398)src/activities/settings/KOReaderAuthActivity.cpp (2)
render(88-112)render(88-88)src/activities/settings/KOReaderSettingsActivity.cpp (2)
render(113-153)render(113-113)src/activities/network/WifiSelectionActivity.cpp (2)
render(457-507)render(457-457)src/activities/settings/CalibreSettingsActivity.cpp (2)
render(88-124)render(88-88)src/activities/network/NetworkModeSelectionActivity.cpp (2)
render(57-85)render(57-57)
src/activities/network/CalibreConnectActivity.h (4)
src/activities/Activity.h (1)
Activity(14-53)src/activities/network/CalibreConnectActivity.cpp (8)
onEnter(18-44)onEnter(18-18)onExit(46-57)onExit(46-46)loop(96-159)loop(96-96)render(161-220)render(161-161)src/activities/network/CrossPointWebServerActivity.cpp (8)
onEnter(35-53)onEnter(35-35)onExit(55-94)onExit(55-55)loop(254-326)loop(254-254)render(328-351)render(328-328)src/activities/network/NetworkModeSelectionActivity.cpp (8)
onEnter(14-22)onEnter(14-14)onExit(24-24)onExit(24-24)loop(26-55)loop(26-26)render(57-85)render(57-57)
src/activities/reader/KOReaderSyncActivity.cpp (8)
src/activities/Activity.cpp (6)
setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)startActivityForResult(24-27)startActivityForResult(24-24)src/activities/reader/EpubReaderActivity.cpp (6)
onEnter(61-106)onEnter(61-61)onExit(108-118)onExit(108-108)render(398-529)render(398-398)src/activities/settings/KOReaderSettingsActivity.cpp (6)
onEnter(21-26)onEnter(21-21)onExit(28-28)onExit(28-28)render(113-153)render(113-113)src/activities/browser/OpdsBookBrowserActivity.cpp (8)
onEnter(23-37)onEnter(23-23)onWifiSelectionComplete(371-387)onWifiSelectionComplete(371-371)onExit(39-47)onExit(39-39)render(139-225)render(139-139)src/activities/network/WifiSelectionActivity.cpp (6)
onEnter(16-71)onEnter(16-16)onExit(73-88)onExit(73-73)render(457-507)render(457-457)src/activities/settings/CalibreSettingsActivity.cpp (6)
onEnter(19-24)onEnter(19-19)onExit(26-26)onExit(26-26)render(88-124)render(88-88)src/activities/settings/OtaUpdateActivity.cpp (8)
onEnter(53-64)onEnter(53-53)onWifiSelectionComplete(13-51)onWifiSelectionComplete(13-13)onExit(66-74)onExit(66-66)render(76-135)render(76-76)src/activities/network/CrossPointWebServerActivity.cpp (2)
onWifiSelectionComplete(144-165)onWifiSelectionComplete(144-144)
src/activities/home/RecentBooksActivity.cpp (2)
src/activities/reader/EpubReaderActivity.cpp (2)
render(398-529)render(398-398)src/activities/reader/XtcReaderActivity.cpp (2)
render(128-144)render(128-128)
src/activities/util/KeyboardEntryActivity.cpp (4)
src/activities/network/WifiSelectionActivity.cpp (4)
onComplete(684-694)onComplete(684-684)render(457-507)render(457-457)src/activities/network/NetworkModeSelectionActivity.cpp (4)
onCancel(95-100)onCancel(95-95)render(57-85)render(57-57)src/activities/Activity.cpp (6)
requestUpdate(9-9)requestUpdate(9-9)setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)
src/activities/settings/KOReaderSettingsActivity.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/home/HomeActivity.cpp (1)
src/activities/Activity.cpp (2)
onSelectBook(18-22)onSelectBook(18-18)
src/activities/reader/ReaderActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/reader/ReaderActivity.cpp (2)
onGoBack(145-147)onGoBack(145-145)
src/activities/reader/EpubReaderMenuActivity.cpp (2)
src/activities/Activity.cpp (6)
requestUpdate(9-9)requestUpdate(9-9)setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)
src/activities/settings/LanguageSelectActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/settings/LanguageSelectActivity.cpp (2)
handleSelection(44-52)handleSelection(44-44)
src/activities/settings/CalibreSettingsActivity.cpp (3)
src/activities/Activity.cpp (4)
requestUpdate(9-9)requestUpdate(9-9)startActivityForResult(24-27)startActivityForResult(24-24)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)src/main.cpp (2)
activityManager(31-31)renderer(30-30)
src/activities/network/WifiSelectionActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/network/CrossPointWebServerActivity.h (1)
onEnter(58-62)
src/activities/home/MyLibraryActivity.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/settings/KOReaderSettingsActivity.cpp (3)
src/activities/network/WifiSelectionActivity.cpp (4)
loop(285-441)loop(285-285)render(457-507)render(457-457)src/activities/Activity.cpp (4)
requestUpdate(9-9)requestUpdate(9-9)startActivityForResult(24-27)startActivityForResult(24-24)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)
src/activities/reader/EpubReaderActivity.h (4)
src/activities/Activity.h (1)
Activity(14-53)src/activities/reader/EpubReaderActivity.cpp (8)
onEnter(61-106)onEnter(61-61)onExit(108-118)onExit(108-108)loop(120-233)loop(120-120)render(398-529)render(398-398)src/activities/reader/EpubReaderChapterSelectionActivity.cpp (8)
onEnter(28-42)onEnter(28-28)onExit(44-44)onExit(44-44)loop(46-89)loop(46-46)render(91-139)render(91-91)src/activities/reader/XtcReaderChapterSelectionActivity.cpp (8)
onEnter(41-51)onEnter(41-41)onExit(53-53)onExit(53-53)loop(55-93)loop(55-55)render(95-142)render(95-95)
src/activities/browser/OpdsBookBrowserActivity.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/network/NetworkModeSelectionActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/network/CrossPointWebServerActivity.cpp (8)
onEnter(35-53)onEnter(35-35)onExit(55-94)onExit(55-55)loop(254-326)loop(254-254)render(328-351)render(328-328)
src/activities/network/NetworkModeSelectionActivity.cpp (2)
src/activities/Activity.cpp (4)
setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/util/KeyboardEntryActivity.cpp (2)
onCancel(330-336)onCancel(330-330)
src/activities/reader/TxtReaderActivity.cpp (2)
src/activities/reader/ReaderActivity.cpp (2)
onEnter(110-143)onEnter(110-110)src/activities/reader/ReaderActivity.h (1)
onEnter(33-34)
src/activities/settings/ClearCacheActivity.cpp (5)
src/activities/settings/KOReaderAuthActivity.cpp (4)
onEnter(50-76)onEnter(50-50)render(88-112)render(88-88)src/activities/settings/KOReaderSettingsActivity.cpp (4)
onEnter(21-26)onEnter(21-21)render(113-153)render(113-113)src/activities/network/WifiSelectionActivity.cpp (4)
onEnter(16-71)onEnter(16-16)render(457-507)render(457-457)src/activities/Activity.cpp (2)
requestUpdate(9-9)requestUpdate(9-9)src/activities/ActivityManager.cpp (2)
requestUpdate(190-196)requestUpdate(190-190)
src/activities/settings/KOReaderAuthActivity.cpp (3)
src/activities/settings/OtaUpdateActivity.cpp (6)
onEnter(53-64)onEnter(53-53)onWifiSelectionComplete(13-51)onWifiSelectionComplete(13-13)render(76-135)render(76-76)src/activities/reader/KOReaderSyncActivity.cpp (6)
onEnter(184-227)onEnter(184-184)onWifiSelectionComplete(44-73)onWifiSelectionComplete(44-44)render(239-351)render(239-239)src/activities/Activity.cpp (4)
startActivityForResult(24-27)startActivityForResult(24-24)finish(31-31)finish(31-31)
src/activities/reader/EpubReaderPercentSelectionActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/reader/EpubReaderPercentSelectionActivity.cpp (8)
onEnter(16-20)onEnter(16-16)onExit(22-22)onExit(22-22)loop(35-58)loop(35-35)render(60-96)render(60-60)
src/activities/settings/CalibreSettingsActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/browser/OpdsBookBrowserActivity.cpp (8)
onEnter(23-37)onEnter(23-23)onExit(39-47)onExit(39-39)loop(49-137)loop(49-49)render(139-225)render(139-139)
src/activities/RenderLock.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/settings/SettingsActivity.h (1)
src/activities/Activity.h (1)
Activity(14-53)
src/activities/ActivityManager.cpp (3)
src/activities/Activity.cpp (2)
requestUpdate(9-9)requestUpdate(9-9)src/activities/network/CrossPointWebServerActivity.h (1)
preventAutoSleep(63-63)src/activities/settings/OtaUpdateActivity.h (1)
skipLoopDelay(35-35)
src/activities/ActivityManager.h (4)
src/activities/Activity.h (2)
Activity(14-53)void(29-29)src/activities/RenderLock.h (1)
RenderLock(6-16)src/activities/ActivityManager.cpp (25)
RenderLock(200-203)RenderLock(205-208)RenderLock(210-215)exitActivity(117-124)exitActivity(117-117)renderTaskTrampoline(26-29)renderTaskTrampoline(26-26)renderTaskLoop(31-40)renderTaskLoop(31-31)begin(16-24)begin(16-16)loop(42-115)loop(42-42)replaceActivity(126-138)replaceActivity(126-126)goHome(167-167)goHome(167-167)preventAutoSleep(184-184)preventAutoSleep(184-184)isReaderActivity(186-186)isReaderActivity(186-186)skipLoopDelay(188-188)skipLoopDelay(188-188)requestUpdate(190-196)requestUpdate(190-190)src/main.cpp (4)
renderer(30-30)loop(307-387)loop(307-307)activityManager(31-31)
src/activities/reader/XtcReaderChapterSelectionActivity.cpp (2)
src/activities/Activity.cpp (4)
setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/reader/XtcReaderActivity.cpp (2)
render(128-144)render(128-128)
src/activities/reader/EpubReaderMenuActivity.h (2)
src/activities/Activity.h (1)
Activity(14-53)src/activities/reader/EpubReaderActivity.cpp (8)
onEnter(61-106)onEnter(61-61)onExit(108-118)onExit(108-108)loop(120-233)loop(120-120)render(398-529)render(398-398)
src/activities/network/CrossPointWebServerActivity.h (3)
src/activities/Activity.h (1)
Activity(14-53)src/network/CrossPointWebServer.h (1)
CrossPointWebServer(20-108)src/activities/network/CrossPointWebServerActivity.cpp (8)
onEnter(35-53)onEnter(35-35)onExit(55-94)onExit(55-55)loop(254-326)loop(254-254)render(328-351)render(328-328)
src/activities/reader/XtcReaderActivity.cpp (3)
src/activities/reader/EpubReaderActivity.cpp (2)
render(398-529)render(398-398)src/activities/reader/TxtReaderActivity.cpp (2)
render(339-372)render(339-339)src/activities/Activity.cpp (2)
startActivityForResult(24-27)startActivityForResult(24-24)
src/activities/settings/OtaUpdateActivity.h (3)
src/activities/Activity.h (1)
Activity(14-53)src/activities/settings/OtaUpdateActivity.cpp (8)
onEnter(53-64)onEnter(53-53)onExit(66-74)onExit(66-66)loop(137-195)loop(137-137)render(76-135)render(76-76)src/activities/Activity.cpp (4)
onEnter(5-5)onEnter(5-5)onExit(7-7)onExit(7-7)
src/activities/network/CrossPointWebServerActivity.cpp (1)
src/activities/Activity.cpp (6)
startActivityForResult(24-27)startActivityForResult(24-24)onGoHome(16-16)onGoHome(16-16)requestUpdate(9-9)requestUpdate(9-9)
src/activities/network/WifiSelectionActivity.cpp (2)
src/activities/Activity.cpp (6)
startActivityForResult(24-27)startActivityForResult(24-24)setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)src/activities/util/KeyboardEntryActivity.cpp (2)
onComplete(321-328)onComplete(321-321)
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/activities/ActivityManager.cpp`:
- Around line 42-115: In ActivityManager::loop() the Pop path holds a RenderLock
across the branch that calls goHome(), which can deadlock if goHome() triggers
replaceActivity()/onEnter() that re-acquires RenderLock; release the lock before
calling goHome(): ensure the RenderLock acquired earlier (e.g., the one created
in the Pop branch) is unlocked (or destroyed/scope ended) immediately after
exitActivity(lock) and pendingAction reset, then call goHome() without holding
the RenderLock so onEnter()/replaceActivity() can safely acquire their own lock.
In `@src/activities/network/CrossPointWebServerActivity.cpp`:
- Around line 51-52: The two lambdas passed to startActivityForResult that
currently call onNetworkModeSelected(result.selectedNetworkMode) must guard
against cancellation like the handler in the other block: check
result.isCancelled and return early if true, otherwise call
onNetworkModeSelected with result.selectedNetworkMode; update both occurrences
(the lambda handling NetworkModeSelectionActivity created with
NetworkModeSelectionActivity(renderer, mappedInput) and the second similar
lambda) to use the same pattern, referencing ActivityResult::isCancelled and
onNetworkModeSelected(...) to avoid using an undefined/placeholder
selectedNetworkMode when cancelled.
In `@src/activities/reader/EpubReaderActivity.cpp`:
- Around line 126-130: Move the epub null-check to the start of the loop()
function so all branches return early if epub is null rather than only the
Confirm path; remove the inner guard around the Confirm handling (the
LOG_ERR("ERS","No book loaded") and return inside that branch) after adding the
top-of-loop check to avoid duplicate checks and ensure no code in loop()
dereferences epub when it's null. Ensure you reference the epub pointer and the
loop() method during the change and keep behavior identical when epub is
present.
In `@src/activities/reader/ReaderActivity.cpp`:
- Around line 145-147: ReaderActivity::onGoBack() currently calls onGoHome()
unconditionally which loses the previous activity because ReaderActivity was
created with replaceActivity(); either make back navigate to the originating
activity by changing the creation to use pushActivity() so the activity stack is
preserved (so onGoBack() can pop to the previous activity), or if Home is indeed
the intended destination, remove the TODO and document that behavior; update the
code paths that create ReaderActivity (where replaceActivity() is used) to
pushActivity() when expecting normal back navigation (Library/Recents), and keep
onGoBack() calling onGoHome() only if you decide Home is the correct and
documented fallback.
---
Outside diff comments:
In `@src/activities/browser/OpdsBookBrowserActivity.cpp`:
- Around line 371-386: In onWifiSelectionComplete
(OpdsBookBrowserActivity::onWifiSelectionComplete) the failure branch sets state
= BrowserState::ERROR and updates errorMessage but does not call
requestUpdate(), leaving the UI potentially stale; add a call to requestUpdate()
after setting errorMessage (and after performing
WiFi.disconnect()/WiFi.mode(WIFI_OFF)) so the UI is refreshed when the error
state is entered.
---
Nitpick comments:
In `@src/activities/ActivityManager.h`:
- Around line 27-28: Remove the redundant forward declarations "class Activity;"
and "class RenderLock;" from ActivityManager.h because Activity.h and
RenderLock.h are already included (so the forward declarations are unnecessary);
delete those two lines and rebuild to confirm no remaining references require
forward declarations elsewhere in this header or switch to forward declarations
only if you remove the corresponding `#include` lines instead.
In `@src/activities/ActivityResult.h`:
- Around line 7-8: The TODO indicates NetworkMode (enum class NetworkMode {
JOIN_NETWORK, CONNECT_CALIBRE, CREATE_HOTSPOT }) is in the wrong place; move it
into a dedicated network-related header (e.g., create a new NetworkMode.h or
NetworkEnums.h) and replace the enum in ActivityResult.h with an include of that
new header, update any files that reference NetworkMode to include the new
header, and remove the TODO; also link this change to an issue or add a brief
comment referencing an issue ID if you create one so the relocation is tracked.
In `@src/activities/browser/OpdsBookBrowserActivity.cpp`:
- Around line 49-54: The call to the empty base implementation Activity::loop()
inside OpdsBookBrowserActivity::loop when state == BrowserState::WIFI_SELECTION
is unnecessary; remove the Activity::loop() invocation and simply return after
handling the WIFI_SELECTION branch to simplify the method (locate the
conditional in OpdsBookBrowserActivity::loop and delete the Activity::loop()
call).
In `@src/activities/home/HomeActivity.cpp`:
- Around line 262-266: HomeActivity::onSelectBook duplicates the base
Activity::onSelectBook behavior; replace the body to forward to the base
implementation instead of duplicating logic—call Activity::onSelectBook(path)
(or the appropriate base class qualifier) so Intent construction and the
activityManager.goToReader call remain centralized in Activity::onSelectBook.
In `@src/activities/network/NetworkModeSelectionActivity.h`:
- Around line 8-9: Remove the stale commented-out enum stub for NetworkMode:
delete the commented lines that declare "enum class NetworkMode { JOIN_NETWORK,
CONNECT_CALIBRE, CREATE_HOTSPOT }" so the header no longer contains
dead/commented-out code; if this enum is needed elsewhere, ensure the real
definition (NetworkMode) resides in the proper header or source and update
includes instead of keeping the commented stub.
In `@src/activities/reader/EpubReaderPercentSelectionActivity.cpp`:
- Around line 53-58: The Up/Down handlers currently call
adjustPercent(kLargeStep) and adjustPercent(-kLargeStep) which is
counter-intuitive for a horizontal slider; either swap the Up/Down mappings so
Up calls adjustPercent(-kLargeStep) and Down calls adjustPercent(kLargeStep), or
remove/disable Up/Down handling if vertical input should be ignored. Update the
buttonNavigator.onPressAndContinuous calls that reference
MappedInputManager::Button::Up and MappedInputManager::Button::Down and ensure
adjustPercent, kLargeStep and any inline comments reflect the chosen behavior.
In `@src/activities/util/KeyboardEntryActivity.cpp`:
- Around line 321-336: The onComplete implementation incorrectly uses std::move
on a const std::string& (parameter text), which won’t move; either change the
onComplete signature to take text by value (std::string text) and keep
result.inputText = std::move(text) to enable move semantics, or keep the const
std::string& signature and assign without move (result.inputText = text); update
the onComplete function accordingly and leave onCancel unchanged (references:
KeyboardEntryActivity::onComplete, ActivityResult::inputText, setResult,
finish).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/activities/ActivityManager.cpp (1)
191-197: Clarify notification coalescing behavior in comment.The comment says "multiple rapid calls won't be lost," but
ulTaskNotifyTake(pdTRUE, ...)inrenderTaskLoopclears the notification value to 0 after waking, so multiple increments are coalesced into a single render. This is likely intentional (avoiding redundant renders), but the comment implies each notification triggers a separate render.📝 Suggested comment update
void ActivityManager::requestUpdate() { - // Using direct notification to signal the render task to update - // Increment counter so multiple rapid calls won't be lost + // Using direct notification to signal the render task to update. + // Multiple rapid calls are coalesced into a single render cycle. if (renderTaskHandle) { xTaskNotify(renderTaskHandle, 1, eIncrement); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/ActivityManager.cpp` around lines 191 - 197, The comment in ActivityManager::requestUpdate is misleading about notifications — explain that while xTaskNotify(renderTaskHandle, 1, eIncrement) increments the task notification value, renderTaskLoop uses ulTaskNotifyTake(pdTRUE, ...) which clears the value on wake so multiple rapid increments are coalesced into a single render; update the comment in requestUpdate to state this intentional coalescing behavior (i.e., redundant rapid calls collapse to one render) and reference renderTaskLoop/ulTaskNotifyTake so future readers know why multiple increments don't produce multiple renders.src/activities/reader/EpubReaderActivity.cpp (2)
353-364: MissingRenderLockbefore state modification (same pattern as chapter selection).Similar to the chapter selection callback, this modifies shared state and resets
sectionwithout holding the lock.🔒 Proposed fix
startActivityForResult(new KOReaderSyncActivity(renderer, mappedInput, epub, epub->getPath(), currentSpineIndex, currentPage, totalPages), [this](const ActivityResult& result) { if (!result.isCancelled) { if (currentSpineIndex != result.syncedSpineIndex || (section && section->currentPage != result.syncedPage)) { + RenderLock lock(*this); currentSpineIndex = result.syncedSpineIndex; nextPageNumber = result.syncedPage; section.reset(); } } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderActivity.cpp` around lines 353 - 364, The callback passed to startActivityForResult that handles KOReaderSyncActivity results modifies shared state (currentSpineIndex, nextPageNumber, and resets section) without acquiring the RenderLock; wrap the state-modifying block inside the same RenderLock used elsewhere (e.g. chapter selection) so you lock before checking/updating currentSpineIndex/section and release after (use the existing RenderLock type or RAII guard used in this file to ensure exception-safe unlocking).
304-311: Consider acquiringRenderLockbefore modifying shared state.The callback modifies
currentSpineIndex,nextPageNumber, and callssection.reset()without holding aRenderLock. Other mutation sites in this file (lines 192-196, 212-217, 224-230, 289-296) consistently acquire the lock before modifyingsectionto prevent races with the render task.🔒 Proposed fix
startActivityForResult(new EpubReaderChapterSelectionActivity(renderer, mappedInput, epub, path, spineIdx), [this](const ActivityResult& result) { if (!result.isCancelled && currentSpineIndex != result.selectedSpineIndex) { + RenderLock lock(*this); currentSpineIndex = result.selectedSpineIndex; nextPageNumber = 0; section.reset(); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderActivity.cpp` around lines 304 - 311, The callback passed to startActivityForResult mutates shared state (currentSpineIndex, nextPageNumber, and calls section.reset()) without holding the RenderLock; wrap the mutation block in the same RenderLock acquisition pattern used elsewhere so the render task cannot race with these changes. Specifically, in the lambda given to startActivityForResult, acquire the RenderLock (using the same RenderLock type/object used in this file) before comparing/updating currentSpineIndex, setting nextPageNumber, and calling section.reset(), then release the lock after the updates.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/activities/ActivityManager.cppsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/ReaderActivity.cpp
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/ActivityManager.cppsrc/activities/reader/ReaderActivity.cppsrc/activities/reader/EpubReaderActivity.cpp
📚 Learning: 2026-02-18T15:43:07.515Z
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Applied to files:
src/activities/reader/ReaderActivity.cppsrc/activities/reader/EpubReaderActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().
Applied to files:
src/activities/reader/ReaderActivity.cpp
📚 Learning: 2026-02-12T06:57:35.955Z
Learnt from: CaptainFrito
Repo: crosspoint-reader/crosspoint-reader PR: 725
File: src/activities/network/CalibreConnectActivity.cpp:218-264
Timestamp: 2026-02-12T06:57:35.955Z
Learning: In src/activities/network/CalibreConnectActivity.cpp, button hints (« Exit) are intentionally omitted from the SERVER_STARTING and ERROR states—only the SERVER_RUNNING state displays navigation hints. This is a deliberate design decision by the author.
Applied to files:
src/activities/reader/ReaderActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In EpubReaderActivity.cpp, renderContents() behavior is intentional: if uncached images exist, Phase 1 should call displayBuffer(forceFullRefresh) to perform a HALF_REFRESH and clear ghosting; Phase 2 should call renderer.displayBuffer() without forceFullRefresh for a faster refresh. Grayscale anti-aliasing is applied separately after renderContents() via displayGrayBuffer(). This guidance applies specifically to this file and situation.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
🧬 Code graph analysis (2)
src/activities/reader/ReaderActivity.cpp (3)
src/main.cpp (2)
activityManager(31-31)renderer(30-30)src/activities/reader/ReaderActivity.h (1)
onEnter(33-34)src/activities/Activity.cpp (2)
finish(31-31)finish(31-31)
src/activities/reader/EpubReaderActivity.cpp (1)
src/activities/Activity.cpp (6)
finish(31-31)finish(31-31)startActivityForResult(24-27)startActivityForResult(24-24)onGoHome(16-16)onGoHome(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (13)
src/activities/reader/ReaderActivity.cpp (3)
83-85: LGTM!Clean migration to Intent-based navigation through ActivityManager.
88-108: LGTM!The reader transitions now properly delegate to
activityManager.replaceActivity(), removing the previous manual exit/enter patterns.
110-145: LGTM - back navigation behavior is now well-defined.The
onGoBack()implementation callingfinish()properly delegates topopActivity(). When a book fails to load duringonEnter(), the activity is popped; if the stack is empty (typical sincegoToReader()usesreplaceActivity()), ActivityManager routes to Home. This is a clear improvement over the previous ambiguous behavior flagged in earlier reviews.src/activities/ActivityManager.cpp (4)
16-24: LGTM!Task creation with proper stack size and assert on failure is good defensive programming.
31-40: LGTM!The render loop correctly acquires both power and render locks before rendering, preventing low-power mode interruption during display updates.
42-116: LGTM - careful lock management throughout.The Pop path correctly unlocks before calling
goHome()(line 59) and before invoking result handlers (line 74), addressing potential deadlock concerns. The result handler is moved before invocation (line 72) to safely handle nestedstartActivityForResult()calls. Stack cleanup during Replace (lines 95-99) properly callsonExit()before deletion.
201-223: LGTM!The RenderLock RAII implementation correctly manages the mutex with proper double-release protection via
isLocked. TheRenderLock(Activity&)constructor preserves API compatibility for render method signatures while ignoring the parameter.src/activities/reader/EpubReaderActivity.cpp (6)
61-62: LGTM!Base class lifecycle calls correctly updated from
ActivityWithSubactivitytoActivity, aligning with the new ActivityManager architecture.Also applies to: 108-109
120-125: LGTM! Null guard properly placed at loop entry.The early
finish()call whenepubis null prevents null dereferences throughout the method and cleanly exits the activity.
137-144: LGTM!The callback correctly:
- Applies orientation changes unconditionally (intentional, as orientation selection persists regardless of menu action)
- Only processes menu actions when not cancelled
- Safely captures
thissince the parent activity remains on the stack when the callback executes
149-151: LGTM!Navigation via
activityManager.goToMyLibrary()is clean, and the ternary provides null-safety for the path.
321-326: LGTM!The callback delegates to
jumpToPercent(), which internally acquiresRenderLockbefore modifying state.
329-348: LGTM!The
GO_HOMEandDELETE_CACHEhandlers are well-structured:
DELETE_CACHEproperly acquiresRenderLockbefore state modifications- Adding
epub &&at line 336 provides defensive null-safety- Both paths correctly use
onGoHome()for navigation
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/activities/ActivityManager.cpp`:
- Around line 191-197: The comment in ActivityManager::requestUpdate is
misleading about notifications — explain that while
xTaskNotify(renderTaskHandle, 1, eIncrement) increments the task notification
value, renderTaskLoop uses ulTaskNotifyTake(pdTRUE, ...) which clears the value
on wake so multiple rapid increments are coalesced into a single render; update
the comment in requestUpdate to state this intentional coalescing behavior
(i.e., redundant rapid calls collapse to one render) and reference
renderTaskLoop/ulTaskNotifyTake so future readers know why multiple increments
don't produce multiple renders.
In `@src/activities/reader/EpubReaderActivity.cpp`:
- Around line 353-364: The callback passed to startActivityForResult that
handles KOReaderSyncActivity results modifies shared state (currentSpineIndex,
nextPageNumber, and resets section) without acquiring the RenderLock; wrap the
state-modifying block inside the same RenderLock used elsewhere (e.g. chapter
selection) so you lock before checking/updating currentSpineIndex/section and
release after (use the existing RenderLock type or RAII guard used in this file
to ensure exception-safe unlocking).
- Around line 304-311: The callback passed to startActivityForResult mutates
shared state (currentSpineIndex, nextPageNumber, and calls section.reset())
without holding the RenderLock; wrap the mutation block in the same RenderLock
acquisition pattern used elsewhere so the render task cannot race with these
changes. Specifically, in the lambda given to startActivityForResult, acquire
the RenderLock (using the same RenderLock type/object used in this file) before
comparing/updating currentSpineIndex, setting nextPageNumber, and calling
section.reset(), then release the lock after the updates.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/activities/ActivityManager.cppsrc/activities/network/CrossPointWebServerActivity.cppsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/util/KeyboardEntryActivity.cppsrc/activities/util/KeyboardEntryActivity.h
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/util/KeyboardEntryActivity.cppsrc/activities/network/CrossPointWebServerActivity.cppsrc/activities/settings/KOReaderAuthActivity.cppsrc/activities/util/KeyboardEntryActivity.hsrc/activities/ActivityManager.cpp
📚 Learning: 2026-02-12T06:57:35.955Z
Learnt from: CaptainFrito
Repo: crosspoint-reader/crosspoint-reader PR: 725
File: src/activities/network/CalibreConnectActivity.cpp:218-264
Timestamp: 2026-02-12T06:57:35.955Z
Learning: In src/activities/network/CalibreConnectActivity.cpp, button hints (« Exit) are intentionally omitted from the SERVER_STARTING and ERROR states—only the SERVER_RUNNING state displays navigation hints. This is a deliberate design decision by the author.
Applied to files:
src/activities/network/CrossPointWebServerActivity.cppsrc/activities/settings/KOReaderAuthActivity.cpp
📚 Learning: 2026-02-18T15:43:07.515Z
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Applied to files:
src/activities/settings/KOReaderAuthActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().
Applied to files:
src/activities/settings/KOReaderAuthActivity.cpp
🧬 Code graph analysis (5)
src/activities/util/KeyboardEntryActivity.cpp (2)
src/activities/ActivityManager.cpp (2)
requestUpdate(199-205)requestUpdate(199-199)src/activities/Activity.cpp (6)
requestUpdate(9-9)requestUpdate(9-9)setResult(29-29)setResult(29-29)finish(31-31)finish(31-31)
src/activities/network/CrossPointWebServerActivity.cpp (1)
src/activities/Activity.cpp (6)
startActivityForResult(24-27)startActivityForResult(24-24)onGoHome(16-16)onGoHome(16-16)requestUpdate(9-9)requestUpdate(9-9)
src/activities/settings/KOReaderAuthActivity.cpp (1)
src/activities/Activity.cpp (2)
finish(31-31)finish(31-31)
src/activities/util/KeyboardEntryActivity.h (4)
src/activities/Activity.h (1)
Activity(14-53)src/activities/network/CrossPointWebServerActivity.cpp (8)
onEnter(35-58)onEnter(35-35)onExit(60-99)onExit(60-60)loop(265-337)loop(265-265)render(339-362)render(339-339)src/activities/util/KeyboardEntryActivity.cpp (12)
onEnter(22-27)onEnter(22-22)onExit(29-29)onExit(29-29)loop(107-187)loop(107-107)render(189-319)render(189-189)onComplete(321-328)onComplete(321-321)onCancel(330-336)onCancel(330-330)src/activities/ActivityManager.cpp (2)
loop(44-118)loop(44-44)
src/activities/ActivityManager.cpp (4)
src/main.cpp (3)
loop(307-387)loop(307-307)activityManager(31-31)src/activities/Activity.cpp (2)
requestUpdate(9-9)requestUpdate(9-9)src/activities/network/CrossPointWebServerActivity.h (1)
preventAutoSleep(63-63)src/activities/settings/OtaUpdateActivity.h (1)
skipLoopDelay(35-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: cppcheck
🔇 Additional comments (35)
src/activities/util/KeyboardEntryActivity.h (2)
32-45: LGTM: constructor + render signature align with new ActivityManager flow.
60-62: LGTM: private handlers make the completion path explicit.src/activities/util/KeyboardEntryActivity.cpp (5)
85-88: LGTM: direct onComplete call matches internal handler design.
183-185: LGTM: direct onCancel call is consistent with new result flow.
189-189: LGTM: render signature updated for RenderLock.
321-328: LGTM: onComplete correctly sets result and finishes.
330-335: LGTM: onCancel correctly signals cancellation and finishes.src/activities/settings/KOReaderAuthActivity.cpp (4)
51-77: LGTM!The migration from
ActivityWithSubactivity::onEnter()toActivity::onEnter()and the adoption ofstartActivityForResultwith the callback pattern correctly aligns with the new ActivityManager lifecycle model.
79-87: LGTM!Base class call updated correctly. The WiFi cleanup logic is preserved as expected.
89-89: LGTM!Render signature correctly updated to use the unified
RenderLocktype.
115-121: Setting a result beforefinish()would be inconsistent with actual usage; the parent callback deliberately ignores it.
KOReaderAuthActivityis started viastartActivityForResultinKOReaderSettingsActivity(line 109), but the callback is an empty lambda[](const ActivityResult&) {}that explicitly ignores the result. The activity appears designed as a modal dialog for user feedback (displaying success/failure messages) rather than as a mechanism to communicate authentication outcomes back to its parent. CallingsetResult()is not necessary here.src/activities/ActivityManager.cpp (15)
16-24: Render task initialization looks solid.Clear task setup with an assert to surface failures early.
26-29: Trampoline entry is minimal and correct.
31-42: Render loop locking and power lock ordering are sensible.The TOCTOU-safe lock and power lock around render are well placed.
44-49: Loop delegation without a global lock is consistent with the design.
50-87: Pop flow cleanly handles results and avoids lock re-entrancy.Unlocking before handlers/goHome keeps deadlocks at bay.
89-115: Push/replace state transitions look consistent and safe.
120-127: exitActivity() under lock is clear and correct.
129-141: Deferred replace avoids delete-this hazards.
143-158: Navigation helpers are straightforward and consistent.
164-170: Remaining navigation helpers look good.
172-191: Push/pop request wiring is tidy and defensive.
193-198: State query helpers are clean and clear.
199-205: Render notifications are coalesced correctly.
209-231: RenderLock RAII implementation is straightforward.
159-162: goToSleep() does not risk re-entrance.The function is called from the main event loop (lines 348 and 354 in main.cpp) with
return;statements, soactivityManager.loop()is never invoked in that context. AftergoToSleep()renders the sleep screen via its nestedloop()call, execution proceeds directly todisplay.deepSleep()and device deep sleep, never returning to the main event loop. The nestedloop()call is safe and intentional.src/activities/network/CrossPointWebServerActivity.cpp (9)
35-57: onEnter now cleanly boots into mode selection with cancellation handling.
60-62: Base onExit() delegation is in the right place.
113-125: Calibre connect result handoff back to mode selection is clear.
136-140: WiFi selection result plumbing looks good.
167-174: Cancellation path now routes cleanly to home or re-selection.
195-198: AP start failure now exits cleanly.
245-252: Immediate render update + error fallback are well handled.
323-324: Back button exits consistently via onGoHome().Also applies to: 332-334
339-339: Render signature update matches the new RenderLock API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/activities/util/KeyboardEntryActivity.h`:
- Around line 15-19: Update the usage comment for KeyboardEntryActivity to
reflect ActivityManager-driven lifecycle: replace steps that mention manual
onEnter() and loop() with instructions to start the activity via ActivityManager
(e.g., call startActivityForResult with a KeyboardEntryActivity instance) and
handle the returned ActivityResult in the result handler/callback; reference
KeyboardEntryActivity, ActivityManager, startActivityForResult, and
ActivityResult in the wording so callers know to use the manager and result
handler rather than calling onEnter()/loop() directly.
Contributor
|
I'm in favor of moving in this direction. After looking through the changes, I have a few thoughts that I put into patches for you to consider:
I'm seeing an occasional crash after bouncing into and out of a few epub books and using the chapter select activity. I'll try to make sense of the crash dump. |
Contributor
Author
|
btw, do you have any public OPDS server that can be used for testing? I tested most activities except for OPDS, a bit struggle to setup one myself |
osteotek
previously approved these changes
Feb 26, 2026
Member
There is the Project Gutenberg OPDS - https://www.gutenberg.org/ebooks/search.opds/ |
osteotek
approved these changes
Feb 26, 2026
Contributor
Author
|
@znelson Would you mind having a look? Thanks! |
Contributor
|
Looks good! Played around with it some this evening and didn't hit any problems. A few possible small tweaks to consider: |
znelson
approved these changes
Feb 27, 2026
Contributor
Author
|
@znelson the The rest of your patch looks ok, feel free to push a PR for it, thanks! |
Contributor
|
This is quite invasive for existing PRs (or wanna be PRs) that implement new Activities the old way, expecially for developers not familiar with the Android framework. What about a migration guide?
|
Contributor
|
@drbourbon That's a good suggestion. I'll ask Claude to put something together, and tag you in a PR for feedback. |
ngxson
pushed a commit
that referenced
this pull request
Feb 27, 2026
## Summary **What is the goal of this PR?** Small tweaks to #1016: - Only Activity and ActivityManager can access activityResultHandler and activityResult - `[[maybe_unused]]` in RenderLock constructor - Only ActivityManager and RenderLock can access renderingMutex - Missing renderUpdate after failed wifi selection - Standardize on activities calling finish instead of activityManager.popActivity - Hold RenderLock while mutating state in EpubReaderActivity result handlers --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**NO**_
ngxson
pushed a commit
that referenced
this pull request
Feb 28, 2026
## Summary **What is the goal of this PR?** Added overview and migration guide for ActivityManager changes in #1016. Thanks for the suggestion, @drbourbon! --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES, fully written by Claude 4.6**_
anthonydgallo
pushed a commit
to anthonydgallo/crosspoint-reader
that referenced
this pull request
Feb 28, 2026
Synced with upstream daveallie/crosspoint-reader, integrating the major ActivityManager refactor (PR crosspoint-reader#1016) and 30+ other commits. Upstream features integrated: - ActivityManager: stack-based activity lifecycle with startActivityForResult() - Auto page turn for epub/txt readers - File deletion via ConfirmationActivity in MyLibrary - Thread-safe HalStorage (mutex-wrapped file operations) - Footnotes support in epub reader - QR code display for book sharing - Status bar settings activity - New translations: Dutch, Polish, Danish, Finnish, Belarusian, Italian - Kerning and ligature support in fonts - Improved hyphenation Fork features preserved: - App Store and Apps Menu with 20 preloaded SD card apps - All app activities: Calculator, Minesweeper, Flashcard, Image Viewer, Text Editor, Text Viewer, Random Quote, Art Gallery, Rosary - Enhanced file management (folder delete, rename, move) - hideOpdsBrowser and keyboardStyle settings - OTA updates pointing to anthonydgallo/crosspoint-reader - FILE_BROWSER theme auto-navigation to MyLibrary Fork adaptations for new architecture: - Added goToAppsMenu(), goToAppStore(), goToOpenApp() to ActivityManager - Migrated AppStoreActivity from ActivityWithSubactivity to Activity base - Migrated TextEditorAppActivity from ActivityWithSubactivity to Activity base - Rewrote RosaryActivity to use ActivityManager rendering (removed FreeRTOS task) - Updated all app activities to use finish() instead of onGoHome callback - Updated AppsMenuActivity to use activityManager.goToOpenApp() - Removed all ActivityWithSubactivity references https://claude.ai/code/session_018sxVXVzezGUiWJzrEkcT5J
lukestein
pushed a commit
to lukestein/crosspoint-reader
that referenced
this pull request
Feb 28, 2026
## Summary **What is the goal of this PR?** Added overview and migration guide for ActivityManager changes in crosspoint-reader#1016. Thanks for the suggestion, @drbourbon! --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES, fully written by Claude 4.6**_
This was referenced Feb 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ref comment: #1010 (review)
This PR introduces
ActivityManager, which mirrors the same concept of Activity in Android, where an activity represents a single screen of the UI. The manager is responsible for launching activities, and ensuring that only one activity is active at a time.Main differences from Android's ActivityManager:
Key changes
renderTaskshared across all activitiesstartActivityForResultandsetResultfinish()to destroy themself, but the actual deletion will be handled byActivityManagerto avoiddelete thispatternAs a bonus: the manager will automatically call
requestUpdate()when returning from another activityExample usage
BEFORE:
AFTER: (mirrors the
startActivityForResultandsetResultfrom android)TODO:
Intentis really necessary or it should be removed (note: it's inspired by Intent from Android API) ==> I decided to keep this pattern fr clarityActivityWithSubactivityto just simpleActivity--> We are using a stack for keeping track of sub-activity nowAI Usage
While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.
Did you use AI tools to help write this code? PARTIALLY, some repetitive migrations are done by Claude, but I'm the one how ultimately approve it