cottongin/crosspoint-reader-mod
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: move render() to Activity super class, use freeRTOS notification (#774)

Browse Source 
## Summary

Currently, each activity has to manage their own `displayTaskLoop` which
adds redundant boilerplate code. The loop is a wait loop which is also
not the best practice, as the `updateRequested` boolean is not protected
by a mutex.

In this PR:
- Move `displayTaskLoop` to the super `Activity` class
- Replace `updateRequested` with freeRTOS's [direct to task
notification](https://www.freertos.org/Documentation/02-Kernel/02-Kernel-features/03-Direct-to-task-notifications/01-Task-notifications)
- For `ActivityWithSubactivity`, whenever a sub-activity is present, the
parent's `render()` automatically goes inactive

With this change, activities now only need to expose `render()`
function, and anywhere in the code base can call `requestUpdate()` to
request a new rendering pass.

## Additional Context

In theory, this change may also make the battery life a bit better,
since one wait loop is removed. Although the equipment in my home lab
wasn't been able to verify it (the electric current is too noisy and
small). Would appreciate if anyone has any insights on this subject.

Update: I managed to hack [a small piece of
code](https://github.com/ngxson/crosspoint-reader/tree/xsn/measure_cpu_usage)
that allow tracking CPU idle time.

The CPU load does decrease a bit (1.47% down to 1.39%), which make
sense, because the display task is now sleeping most of the time unless
notified. This should translate to a slightly increase in battery life
in the long run.

```
PR:
[40012] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[40012] [IDLE] Idle time: 98.61% (CPU load: 1.39%)
[50017] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[50017] [IDLE] Idle time: 98.61% (CPU load: 1.39%)
[60022] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[60022] [IDLE] Idle time: 98.61% (CPU load: 1.39%)

master:
[20012] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[20012] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
[30017] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[30017] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
[40022] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[40022] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
```

---

### 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**


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Streamlined rendering architecture by consolidating update mechanisms
across all activities, improving efficiency and consistency.
* Modernized synchronization patterns for display updates to ensure
reliable, conflict-free rendering.

* **Bug Fixes**
* Enhanced rendering stability through improved locking mechanisms and
explicit update requests.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: znelson <znelson@users.noreply.github.com>
This commit is contained in:
Xuan-Son Nguyen
2026-02-16 11:11:15 +01:00
committed by GitHub
parent 0508bfc1f7
commit a616f42cb4
53 changed files with 458 additions and 1409 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
src/activities/reader/EpubReaderActivity.cpp
View File

@@ -57,11 +57,6 @@ void applyReaderOrientation(GfxRenderer& renderer, const uint8_t orientation) {
} // namespace
void EpubReaderActivity::taskTrampoline(void* param) {
auto* self = static_cast<EpubReaderActivity*>(param);
self->displayTaskLoop();
}
void EpubReaderActivity::onEnter() {
ActivityWithSubactivity::onEnter();
@@ -73,8 +68,6 @@ void EpubReaderActivity::onEnter() {
// NOTE: This affects layout math and must be applied before any render calls.
applyReaderOrientation(renderer, SETTINGS.orientation);
renderingMutex = xSemaphoreCreateMutex();
epub->setupCacheDir();
FsFile f;
@@ -108,14 +101,7 @@ void EpubReaderActivity::onEnter() {
RECENT_BOOKS.addBook(epub->getPath(), epub->getTitle(), epub->getAuthor(), epub->getThumbBmpPath());
// Trigger first update
updateRequired = true;
xTaskCreate(&EpubReaderActivity::taskTrampoline, "EpubReaderActivityTask",
8192, // Stack size
this, // Parameters
1, // Priority
&displayTaskHandle // Task handle
);
requestUpdate();
}
void EpubReaderActivity::onExit() {
@@ -124,14 +110,6 @@ void EpubReaderActivity::onExit() {
// Reset orientation back to portrait for the rest of the UI
renderer.setOrientation(GfxRenderer::Orientation::Portrait);
// Wait until not rendering to delete task to avoid killing mid-instruction to EPD
xSemaphoreTake(renderingMutex, portMAX_DELAY);
if (displayTaskHandle) {
vTaskDelete(displayTaskHandle);
displayTaskHandle = nullptr;
}
vSemaphoreDelete(renderingMutex);
renderingMutex = nullptr;
APP_STATE.readerActivityLoadCount = 0;
APP_STATE.saveToFile();
section.reset();
@@ -146,7 +124,7 @@ void EpubReaderActivity::loop() {
if (pendingSubactivityExit) {
pendingSubactivityExit = false;
exitActivity();
updateRequired = true;
requestUpdate();
skipNextButtonCheck = true; // Skip button processing to ignore stale events
}
// Deferred go home: process after subActivity->loop() returns to avoid race condition
@@ -186,8 +164,6 @@ void EpubReaderActivity::loop() {
// Enter reader menu activity.
if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) {
// Don't start activity transition while rendering
xSemaphoreTake(renderingMutex, portMAX_DELAY);
const int currentPage = section ? section->currentPage + 1 : 0;
const int totalPages = section ? section->pageCount : 0;
float bookProgress = 0.0f;
@@ -201,7 +177,6 @@ void EpubReaderActivity::loop() {
this->renderer, this->mappedInput, epub->getTitle(), currentPage, totalPages, bookProgressPercent,
SETTINGS.orientation, [this](const uint8_t orientation) { onReaderMenuBack(orientation); },
[this](EpubReaderMenuActivity::MenuAction action) { onReaderMenuConfirm(action); }));
xSemaphoreGive(renderingMutex);
}
// Long press BACK (1s+) goes to file selection
@@ -238,7 +213,7 @@ void EpubReaderActivity::loop() {
if (currentSpineIndex > 0 && currentSpineIndex >= epub->getSpineItemsCount()) {
currentSpineIndex = epub->getSpineItemsCount() - 1;
nextPageNumber = UINT16_MAX;
updateRequired = true;
requestUpdate();
return;
}
@@ -251,13 +226,13 @@ void EpubReaderActivity::loop() {
currentSpineIndex = nextTriggered ? currentSpineIndex + 1 : currentSpineIndex - 1;
section.reset();
xSemaphoreGive(renderingMutex);
updateRequired = true;
requestUpdate();
return;
}
// No current section, attempt to rerender the book
if (!section) {
updateRequired = true;
requestUpdate();
return;
}
@@ -272,7 +247,7 @@ void EpubReaderActivity::loop() {
section.reset();
xSemaphoreGive(renderingMutex);
}
updateRequired = true;
requestUpdate();
} else {
if (section->currentPage < section->pageCount - 1) {
section->currentPage++;
@@ -284,7 +259,7 @@ void EpubReaderActivity::loop() {
section.reset();
xSemaphoreGive(renderingMutex);
}
updateRequired = true;
requestUpdate();
}
}
@@ -293,7 +268,7 @@ void EpubReaderActivity::onReaderMenuBack(const uint8_t orientation) {
// Apply the user-selected orientation when the menu is dismissed.
// This ensures the menu can be navigated without immediately rotating the screen.
applyOrientation(orientation);
updateRequired = true;
requestUpdate();
}
// Translate an absolute percent into a spine index plus a normalized position
@@ -349,7 +324,7 @@ void EpubReaderActivity::jumpToPercent(int percent) {
pendingSpineProgress = 1.0f;
}
// Reset state so renderScreen() reloads and repositions on the target spine.
// Reset state so render() reloads and repositions on the target spine.
xSemaphoreTake(renderingMutex, portMAX_DELAY);
currentSpineIndex = targetSpineIndex;
nextPageNumber = 0;
@@ -367,8 +342,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
const int spineIdx = currentSpineIndex;
const std::string path = epub->getPath();
xSemaphoreTake(renderingMutex, portMAX_DELAY);
// 1. Close the menu
exitActivity();
@@ -377,7 +350,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
this->renderer, this->mappedInput, epub, path, spineIdx, currentP, totalP,
[this] {
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex) {
if (currentSpineIndex != newSpineIndex) {
@@ -386,7 +359,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex, const int newPage) {
if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) {
@@ -395,10 +368,9 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::GO_TO_PERCENT: {
@@ -409,7 +381,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f;
}
const int initialPercent = clampPercent(static_cast<int>(bookProgress + 0.5f));
xSemaphoreTake(renderingMutex, portMAX_DELAY);
exitActivity();
enterNewActivity(new EpubReaderPercentSelectionActivity(
renderer, mappedInput, initialPercent,
@@ -417,14 +388,13 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
// Apply the new position and exit back to the reader.
jumpToPercent(percent);
exitActivity();
updateRequired = true;
requestUpdate();
},
[this]() {
// Cancel selection and return to the reader.
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::GO_HOME: {
@@ -457,7 +427,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
}
case EpubReaderMenuActivity::MenuAction::SYNC: {
if (KOREADER_STORE.hasCredentials()) {
xSemaphoreTake(renderingMutex, portMAX_DELAY);
const int currentPage = section ? section->currentPage : 0;
const int totalPages = section ? section->pageCount : 0;
exitActivity();
@@ -476,7 +445,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
}
pendingSubactivityExit = true;
}));
xSemaphoreGive(renderingMutex);
}
break;
}
@@ -509,20 +477,8 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) {
xSemaphoreGive(renderingMutex);
}
void EpubReaderActivity::displayTaskLoop() {
while (true) {
if (updateRequired) {
updateRequired = false;
xSemaphoreTake(renderingMutex, portMAX_DELAY);
renderScreen();
xSemaphoreGive(renderingMutex);
}
vTaskDelay(10 / portTICK_PERIOD_MS);
}
}
// TODO: Failure handling
void EpubReaderActivity::renderScreen() {
void EpubReaderActivity::render(Activity::RenderLock&& lock) {
if (!epub) {
return;
}
@@ -643,7 +599,9 @@ void EpubReaderActivity::renderScreen() {
LOG_ERR("ERS", "Failed to load page from SD - clearing section cache");
section->clearCache();
section.reset();
return renderScreen();
requestUpdate(); // Try again after clearing cache
// TODO: prevent infinite loop if the page keeps failing to load for some reason
return;
}
const auto start = millis();
renderContents(std::move(p), orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft);