From ed8a0feac156a115805e1e93bc66132003a89777 Mon Sep 17 00:00:00 2001 From: Xuan-Son Nguyen Date: Mon, 16 Feb 2026 11:11:15 +0100 Subject: [PATCH] refactor: move render() to Activity super class, use freeRTOS notification (#774) 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. 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%) ``` --- 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** * **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. --------- Co-authored-by: znelson --- src/activities/Activity.cpp | 58 ++++++ src/activities/Activity.h | 48 ++++- src/activities/ActivityWithSubactivity.cpp | 32 ++- src/activities/ActivityWithSubactivity.h | 4 + .../browser/OpdsBookBrowserActivity.cpp | 76 +++----- .../browser/OpdsBookBrowserActivity.h | 12 +- src/activities/home/HomeActivity.cpp | 47 +---- src/activities/home/HomeActivity.h | 11 +- src/activities/home/MyLibraryActivity.cpp | 53 +---- src/activities/home/MyLibraryActivity.h | 12 +- src/activities/home/RecentBooksActivity.cpp | 48 +---- src/activities/home/RecentBooksActivity.h | 11 +- .../network/CalibreConnectActivity.cpp | 45 +---- .../network/CalibreConnectActivity.h | 10 +- .../network/CrossPointWebServerActivity.cpp | 71 ++----- .../network/CrossPointWebServerActivity.h | 10 +- .../network/NetworkModeSelectionActivity.cpp | 47 +---- .../network/NetworkModeSelectionActivity.h | 12 +- .../network/WifiSelectionActivity.cpp | 122 ++++-------- .../network/WifiSelectionActivity.h | 11 +- src/activities/reader/EpubReaderActivity.cpp | 182 +++++++----------- src/activities/reader/EpubReaderActivity.h | 12 +- .../EpubReaderChapterSelectionActivity.cpp | 50 +---- .../EpubReaderChapterSelectionActivity.h | 12 +- .../reader/EpubReaderMenuActivity.cpp | 43 +---- .../reader/EpubReaderMenuActivity.h | 12 +- .../EpubReaderPercentSelectionActivity.cpp | 39 +--- .../EpubReaderPercentSelectionActivity.h | 14 +- .../reader/KOReaderSyncActivity.cpp | 70 ++----- src/activities/reader/KOReaderSyncActivity.h | 12 +- src/activities/reader/TxtReaderActivity.cpp | 42 +--- src/activities/reader/TxtReaderActivity.h | 12 +- src/activities/reader/XtcReaderActivity.cpp | 50 +---- src/activities/reader/XtcReaderActivity.h | 12 +- .../XtcReaderChapterSelectionActivity.cpp | 48 +---- .../XtcReaderChapterSelectionActivity.h | 12 +- .../settings/ButtonRemapActivity.cpp | 102 ++++------ src/activities/settings/ButtonRemapActivity.h | 12 +- .../settings/CalibreSettingsActivity.cpp | 61 ++---- .../settings/CalibreSettingsActivity.h | 11 +- .../settings/ClearCacheActivity.cpp | 49 +---- src/activities/settings/ClearCacheActivity.h | 13 +- .../settings/KOReaderAuthActivity.cpp | 48 +---- .../settings/KOReaderAuthActivity.h | 12 +- .../settings/KOReaderSettingsActivity.cpp | 65 ++----- .../settings/KOReaderSettingsActivity.h | 10 +- src/activities/settings/OtaUpdateActivity.cpp | 81 +++----- src/activities/settings/OtaUpdateActivity.h | 10 +- src/activities/settings/SettingsActivity.cpp | 56 +----- src/activities/settings/SettingsActivity.h | 13 +- src/activities/util/KeyboardEntryActivity.cpp | 55 +----- src/activities/util/KeyboardEntryActivity.h | 11 +- src/network/OtaUpdater.cpp | 2 +- 53 files changed, 511 insertions(+), 1462 deletions(-) create mode 100644 src/activities/Activity.cpp diff --git a/src/activities/Activity.cpp b/src/activities/Activity.cpp new file mode 100644 index 00000000..6cd8493e --- /dev/null +++ b/src/activities/Activity.cpp @@ -0,0 +1,58 @@ +#include "Activity.h" + +void Activity::renderTaskTrampoline(void* param) { + auto* self = static_cast(param); + self->renderTaskLoop(); +} + +void Activity::renderTaskLoop() { + while (true) { + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); + { + RenderLock lock(*this); + render(std::move(lock)); + } + } +} + +void Activity::onEnter() { + xTaskCreate(&renderTaskTrampoline, name.c_str(), + 8192, // Stack size + this, // Parameters + 1, // Priority + &renderTaskHandle // Task handle + ); + assert(renderTaskHandle != nullptr && "Failed to create render task"); + LOG_DBG("ACT", "Entering activity: %s", name.c_str()); +} + +void Activity::onExit() { + RenderLock lock(*this); // Ensure we don't delete the task while it's rendering + if (renderTaskHandle) { + vTaskDelete(renderTaskHandle); + renderTaskHandle = nullptr; + } + + LOG_DBG("ACT", "Exiting activity: %s", name.c_str()); +} + +void Activity::requestUpdate() { + // Using direct notification to signal the render task to update + // Increment counter so multiple rapid calls won't be lost + if (renderTaskHandle) { + xTaskNotify(renderTaskHandle, 1, eIncrement); + } +} + +void Activity::requestUpdateAndWait() { + // FIXME @ngxson : properly implement this using freeRTOS notification + delay(100); +} + +// RenderLock + +Activity::RenderLock::RenderLock(Activity& activity) : activity(activity) { + xSemaphoreTake(activity.renderingMutex, portMAX_DELAY); +} + +Activity::RenderLock::~RenderLock() { xSemaphoreGive(activity.renderingMutex); } diff --git a/src/activities/Activity.h b/src/activities/Activity.h index 68e16815..3669dc46 100644 --- a/src/activities/Activity.h +++ b/src/activities/Activity.h @@ -1,12 +1,16 @@ #pragma once - +#include #include +#include +#include +#include +#include #include #include -class MappedInputManager; -class GfxRenderer; +#include "GfxRenderer.h" +#include "MappedInputManager.h" class Activity { protected: @@ -14,14 +18,44 @@ class Activity { GfxRenderer& renderer; MappedInputManager& mappedInput; + // Task to render and display the activity + TaskHandle_t renderTaskHandle = nullptr; + [[noreturn]] static void renderTaskTrampoline(void* param); + [[noreturn]] virtual void renderTaskLoop(); + + // Mutex to protect rendering operations from being deleted mid-render + SemaphoreHandle_t renderingMutex = nullptr; + public: explicit Activity(std::string name, GfxRenderer& renderer, MappedInputManager& mappedInput) - : name(std::move(name)), renderer(renderer), mappedInput(mappedInput) {} - virtual ~Activity() = default; - virtual void onEnter() { LOG_DBG("ACT", "Entering activity: %s", name.c_str()); } - virtual void onExit() { LOG_DBG("ACT", "Exiting activity: %s", name.c_str()); } + : name(std::move(name)), renderer(renderer), mappedInput(mappedInput), renderingMutex(xSemaphoreCreateMutex()) { + assert(renderingMutex != nullptr && "Failed to create rendering mutex"); + } + virtual ~Activity() { + vSemaphoreDelete(renderingMutex); + renderingMutex = nullptr; + }; + class RenderLock; + virtual void onEnter(); + virtual void onExit(); virtual void loop() {} + + virtual void render(RenderLock&&) {} + virtual void requestUpdate(); + virtual void requestUpdateAndWait(); + virtual bool skipLoopDelay() { return false; } virtual bool preventAutoSleep() { return false; } virtual bool isReaderActivity() const { return false; } + + // RAII helper to lock rendering mutex for the duration of a scope. + class RenderLock { + Activity& activity; + + public: + explicit RenderLock(Activity& activity); + RenderLock(const RenderLock&) = delete; + RenderLock& operator=(const RenderLock&) = delete; + ~RenderLock(); + }; }; diff --git a/src/activities/ActivityWithSubactivity.cpp b/src/activities/ActivityWithSubactivity.cpp index 61b1fc1e..40da93f1 100644 --- a/src/activities/ActivityWithSubactivity.cpp +++ b/src/activities/ActivityWithSubactivity.cpp @@ -1,13 +1,31 @@ #include "ActivityWithSubactivity.h" +void ActivityWithSubactivity::renderTaskLoop() { + while (true) { + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); + { + RenderLock lock(*this); + if (!subActivity) { + render(std::move(lock)); + } + // If subActivity is set, consume the notification but skip parent render + // Note: the sub-activity will call its render() from its own display task + } + } +} + void ActivityWithSubactivity::exitActivity() { + // No need to lock, since onExit() already acquires its own lock if (subActivity) { + LOG_DBG("ACT", "Exiting subactivity..."); subActivity->onExit(); subActivity.reset(); } } void ActivityWithSubactivity::enterNewActivity(Activity* activity) { + // Acquire lock to avoid 2 activities rendering at the same time during transition + RenderLock lock(*this); subActivity.reset(activity); subActivity->onEnter(); } @@ -18,7 +36,15 @@ void ActivityWithSubactivity::loop() { } } -void ActivityWithSubactivity::onExit() { - Activity::onExit(); - exitActivity(); +void ActivityWithSubactivity::requestUpdate() { + if (!subActivity) { + Activity::requestUpdate(); + } + // Sub-activity should call their own requestUpdate() from their loop() function +} + +void ActivityWithSubactivity::onExit() { + // No need to lock, onExit() already acquires its own lock + exitActivity(); + Activity::onExit(); } diff --git a/src/activities/ActivityWithSubactivity.h b/src/activities/ActivityWithSubactivity.h index 3189467d..72466b7e 100644 --- a/src/activities/ActivityWithSubactivity.h +++ b/src/activities/ActivityWithSubactivity.h @@ -8,11 +8,15 @@ class ActivityWithSubactivity : public Activity { std::unique_ptr subActivity = nullptr; void exitActivity(); void enterNewActivity(Activity* activity); + [[noreturn]] void renderTaskLoop() override; public: explicit ActivityWithSubactivity(std::string name, GfxRenderer& renderer, MappedInputManager& mappedInput) : Activity(std::move(name), renderer, mappedInput) {} void loop() override; + // Note: when a subactivity is active, parent requestUpdate() calls are ignored; + // the subactivity should request its own renders. This pauses parent rendering until exit. + void requestUpdate() override; void onExit() override; bool preventAutoSleep() override { return subActivity && subActivity->preventAutoSleep(); } bool skipLoopDelay() override { return subActivity && subActivity->skipLoopDelay(); } diff --git a/src/activities/browser/OpdsBookBrowserActivity.cpp b/src/activities/browser/OpdsBookBrowserActivity.cpp index 35c8c725..f20b6bf0 100644 --- a/src/activities/browser/OpdsBookBrowserActivity.cpp +++ b/src/activities/browser/OpdsBookBrowserActivity.cpp @@ -19,15 +19,9 @@ namespace { constexpr int PAGE_ITEMS = 23; } // namespace -void OpdsBookBrowserActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void OpdsBookBrowserActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); state = BrowserState::CHECK_WIFI; entries.clear(); navigationHistory.clear(); @@ -35,14 +29,7 @@ void OpdsBookBrowserActivity::onEnter() { selectorIndex = 0; errorMessage.clear(); statusMessage = "Checking WiFi..."; - updateRequired = true; - - xTaskCreate(&OpdsBookBrowserActivity::taskTrampoline, "OpdsBookBrowserTask", - 4096, // Stack size (larger for HTTP operations) - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); // Check WiFi and connect if needed, then fetch feed checkAndConnectWifi(); @@ -54,13 +41,6 @@ void OpdsBookBrowserActivity::onExit() { // Turn off WiFi when exiting WiFi.mode(WIFI_OFF); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; entries.clear(); navigationHistory.clear(); } @@ -81,7 +61,7 @@ void OpdsBookBrowserActivity::loop() { LOG_DBG("OPDS", "Retry: WiFi connected, retrying fetch"); state = BrowserState::LOADING; statusMessage = "Loading..."; - updateRequired = true; + requestUpdate(); fetchFeed(currentPath); } else { // WiFi not connected - launch WiFi selection @@ -134,40 +114,28 @@ void OpdsBookBrowserActivity::loop() { if (!entries.empty()) { buttonNavigator.onNextRelease([this] { selectorIndex = ButtonNavigator::nextIndex(selectorIndex, entries.size()); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this] { selectorIndex = ButtonNavigator::previousIndex(selectorIndex, entries.size()); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this] { selectorIndex = ButtonNavigator::nextPageIndex(selectorIndex, entries.size(), PAGE_ITEMS); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this] { selectorIndex = ButtonNavigator::previousPageIndex(selectorIndex, entries.size(), PAGE_ITEMS); - updateRequired = true; + requestUpdate(); }); } } } -void OpdsBookBrowserActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void OpdsBookBrowserActivity::render() const { +void OpdsBookBrowserActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); @@ -260,7 +228,7 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) { if (strlen(serverUrl) == 0) { state = BrowserState::ERROR; errorMessage = "No server URL configured"; - updateRequired = true; + requestUpdate(); return; } @@ -274,7 +242,7 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) { if (!HttpDownloader::fetchUrl(url, stream)) { state = BrowserState::ERROR; errorMessage = "Failed to fetch feed"; - updateRequired = true; + requestUpdate(); return; } } @@ -282,7 +250,7 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) { if (!parser) { state = BrowserState::ERROR; errorMessage = "Failed to parse feed"; - updateRequired = true; + requestUpdate(); return; } @@ -293,12 +261,12 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) { if (entries.empty()) { state = BrowserState::ERROR; errorMessage = "No entries found"; - updateRequired = true; + requestUpdate(); return; } state = BrowserState::BROWSING; - updateRequired = true; + requestUpdate(); } void OpdsBookBrowserActivity::navigateToEntry(const OpdsEntry& entry) { @@ -310,7 +278,7 @@ void OpdsBookBrowserActivity::navigateToEntry(const OpdsEntry& entry) { statusMessage = "Loading..."; entries.clear(); selectorIndex = 0; - updateRequired = true; + requestUpdate(); fetchFeed(currentPath); } @@ -328,7 +296,7 @@ void OpdsBookBrowserActivity::navigateBack() { statusMessage = "Loading..."; entries.clear(); selectorIndex = 0; - updateRequired = true; + requestUpdate(); fetchFeed(currentPath); } @@ -339,7 +307,7 @@ void OpdsBookBrowserActivity::downloadBook(const OpdsEntry& book) { statusMessage = book.title; downloadProgress = 0; downloadTotal = 0; - updateRequired = true; + requestUpdate(); // Build full download URL std::string downloadUrl = UrlUtils::buildUrl(SETTINGS.opdsServerUrl, book.href); @@ -357,7 +325,7 @@ void OpdsBookBrowserActivity::downloadBook(const OpdsEntry& book) { HttpDownloader::downloadToFile(downloadUrl, filename, [this](const size_t downloaded, const size_t total) { downloadProgress = downloaded; downloadTotal = total; - updateRequired = true; + requestUpdate(); }); if (result == HttpDownloader::OK) { @@ -369,11 +337,11 @@ void OpdsBookBrowserActivity::downloadBook(const OpdsEntry& book) { LOG_DBG("OPDS", "Cleared cache for: %s", filename.c_str()); state = BrowserState::BROWSING; - updateRequired = true; + requestUpdate(); } else { state = BrowserState::ERROR; errorMessage = "Download failed"; - updateRequired = true; + requestUpdate(); } } @@ -382,7 +350,7 @@ void OpdsBookBrowserActivity::checkAndConnectWifi() { if (WiFi.status() == WL_CONNECTED && WiFi.localIP() != IPAddress(0, 0, 0, 0)) { state = BrowserState::LOADING; statusMessage = "Loading..."; - updateRequired = true; + requestUpdate(); fetchFeed(currentPath); return; } @@ -393,7 +361,7 @@ void OpdsBookBrowserActivity::checkAndConnectWifi() { void OpdsBookBrowserActivity::launchWifiSelection() { state = BrowserState::WIFI_SELECTION; - updateRequired = true; + requestUpdate(); enterNewActivity(new WifiSelectionActivity(renderer, mappedInput, [this](const bool connected) { onWifiSelectionComplete(connected); })); @@ -406,7 +374,7 @@ void OpdsBookBrowserActivity::onWifiSelectionComplete(const bool connected) { LOG_DBG("OPDS", "WiFi connected via selection, fetching feed"); state = BrowserState::LOADING; statusMessage = "Loading..."; - updateRequired = true; + requestUpdate(); fetchFeed(currentPath); } else { LOG_DBG("OPDS", "WiFi selection cancelled/failed"); @@ -416,6 +384,6 @@ void OpdsBookBrowserActivity::onWifiSelectionComplete(const bool connected) { WiFi.mode(WIFI_OFF); state = BrowserState::ERROR; errorMessage = "WiFi connection failed"; - updateRequired = true; + requestUpdate(); } } diff --git a/src/activities/browser/OpdsBookBrowserActivity.h b/src/activities/browser/OpdsBookBrowserActivity.h index 879a5a39..3ee94f0a 100644 --- a/src/activities/browser/OpdsBookBrowserActivity.h +++ b/src/activities/browser/OpdsBookBrowserActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include #include @@ -34,13 +31,10 @@ class OpdsBookBrowserActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; - bool updateRequired = false; - BrowserState state = BrowserState::LOADING; std::vector entries; std::vector navigationHistory; // Stack of previous feed paths for back navigation @@ -53,10 +47,6 @@ class OpdsBookBrowserActivity final : public ActivityWithSubactivity { const std::function onGoHome; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; - void checkAndConnectWifi(); void launchWifiSelection(); void onWifiSelectionComplete(bool connected); diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index 290c5c29..90408eb7 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -20,11 +20,6 @@ #include "fontIds.h" #include "util/StringUtils.h" -void HomeActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - int HomeActivity::getMenuItemCount() const { int count = 4; // My Library, Recents, File transfer, Settings if (!recentBooks.empty()) { @@ -119,7 +114,7 @@ void HomeActivity::loadRecentCovers(int coverHeight) { } coverRendered = false; - updateRequired = true; + requestUpdate(); } } progress++; @@ -132,8 +127,6 @@ void HomeActivity::loadRecentCovers(int coverHeight) { void HomeActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - // Check if OPDS browser URL is configured hasOpdsUrl = strlen(SETTINGS.opdsServerUrl) > 0; @@ -143,28 +136,12 @@ void HomeActivity::onEnter() { loadRecentBooks(metrics.homeRecentBooksCount); // Trigger first update - updateRequired = true; - - xTaskCreate(&HomeActivity::taskTrampoline, "HomeActivityTask", - 8192, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void HomeActivity::onExit() { Activity::onExit(); - // 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; - // Free the stored cover buffer if any freeCoverBuffer(); } @@ -216,12 +193,12 @@ void HomeActivity::loop() { buttonNavigator.onNext([this, menuCount] { selectorIndex = ButtonNavigator::nextIndex(selectorIndex, menuCount); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this, menuCount] { selectorIndex = ButtonNavigator::previousIndex(selectorIndex, menuCount); - updateRequired = true; + requestUpdate(); }); if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { @@ -250,19 +227,7 @@ void HomeActivity::loop() { } } -void HomeActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void HomeActivity::render() { +void HomeActivity::render(Activity::RenderLock&&) { auto metrics = UITheme::getInstance().getMetrics(); const auto pageWidth = renderer.getScreenWidth(); const auto pageHeight = renderer.getScreenHeight(); @@ -298,7 +263,7 @@ void HomeActivity::render() { if (!firstRenderDone) { firstRenderDone = true; - updateRequired = true; + requestUpdate(); } else if (!recentsLoaded && !recentsLoading) { recentsLoading = true; loadRecentCovers(metrics.homeCoverHeight); diff --git a/src/activities/home/HomeActivity.h b/src/activities/home/HomeActivity.h index 8ec68777..5359bf79 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -1,8 +1,4 @@ #pragma once -#include -#include -#include - #include #include @@ -14,11 +10,8 @@ struct RecentBook; struct Rect; class HomeActivity final : public Activity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; int selectorIndex = 0; - bool updateRequired = false; bool recentsLoading = false; bool recentsLoaded = false; bool firstRenderDone = false; @@ -34,9 +27,6 @@ class HomeActivity final : public Activity { const std::function onFileTransferOpen; const std::function onOpdsBrowserOpen; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); int getMenuItemCount() const; bool storeCoverBuffer(); // Store frame buffer for cover image bool restoreCoverBuffer(); // Restore frame buffer from stored cover @@ -60,4 +50,5 @@ class HomeActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/home/MyLibraryActivity.cpp b/src/activities/home/MyLibraryActivity.cpp index 9d2f4073..54e69adf 100644 --- a/src/activities/home/MyLibraryActivity.cpp +++ b/src/activities/home/MyLibraryActivity.cpp @@ -66,11 +66,6 @@ void sortFileList(std::vector& strs) { }); } -void MyLibraryActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void MyLibraryActivity::loadFiles() { files.clear(); @@ -109,33 +104,14 @@ void MyLibraryActivity::loadFiles() { void MyLibraryActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - loadFiles(); - selectorIndex = 0; - updateRequired = true; - xTaskCreate(&MyLibraryActivity::taskTrampoline, "MyLibraryActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void MyLibraryActivity::onExit() { Activity::onExit(); - - // 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; - files.clear(); } @@ -146,7 +122,6 @@ void MyLibraryActivity::loop() { basepath = "/"; loadFiles(); selectorIndex = 0; - updateRequired = true; return; } @@ -162,7 +137,7 @@ void MyLibraryActivity::loop() { basepath += files[selectorIndex].substr(0, files[selectorIndex].length() - 1); loadFiles(); selectorIndex = 0; - updateRequired = true; + requestUpdate(); } else { onSelectBook(basepath + files[selectorIndex]); return; @@ -183,7 +158,7 @@ void MyLibraryActivity::loop() { const std::string dirName = oldPath.substr(pos + 1) + "/"; selectorIndex = findEntry(dirName); - updateRequired = true; + requestUpdate(); } else { onGoHome(); } @@ -194,38 +169,26 @@ void MyLibraryActivity::loop() { buttonNavigator.onNextRelease([this, listSize] { selectorIndex = ButtonNavigator::nextIndex(static_cast(selectorIndex), listSize); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this, listSize] { selectorIndex = ButtonNavigator::previousIndex(static_cast(selectorIndex), listSize); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this, listSize, pageItems] { selectorIndex = ButtonNavigator::nextPageIndex(static_cast(selectorIndex), listSize, pageItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this, listSize, pageItems] { selectorIndex = ButtonNavigator::previousPageIndex(static_cast(selectorIndex), listSize, pageItems); - updateRequired = true; + requestUpdate(); }); } -void MyLibraryActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void MyLibraryActivity::render() const { +void MyLibraryActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/home/MyLibraryActivity.h b/src/activities/home/MyLibraryActivity.h index 0713524d..748b7eea 100644 --- a/src/activities/home/MyLibraryActivity.h +++ b/src/activities/home/MyLibraryActivity.h @@ -1,8 +1,4 @@ #pragma once -#include -#include -#include - #include #include #include @@ -13,12 +9,9 @@ class MyLibraryActivity final : public Activity { private: - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; size_t selectorIndex = 0; - bool updateRequired = false; // Files state std::string basepath = "/"; @@ -28,10 +21,6 @@ class MyLibraryActivity final : public Activity { const std::function onSelectBook; const std::function onGoHome; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; - // Data loading void loadFiles(); size_t findEntry(const std::string& name) const; @@ -48,4 +37,5 @@ class MyLibraryActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/home/RecentBooksActivity.cpp b/src/activities/home/RecentBooksActivity.cpp index 4ade2ae8..74bbf4fd 100644 --- a/src/activities/home/RecentBooksActivity.cpp +++ b/src/activities/home/RecentBooksActivity.cpp @@ -15,11 +15,6 @@ namespace { constexpr unsigned long GO_HOME_MS = 1000; } // namespace -void RecentBooksActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void RecentBooksActivity::loadRecentBooks() { recentBooks.clear(); const auto& books = RECENT_BOOKS.getBooks(); @@ -37,34 +32,15 @@ void RecentBooksActivity::loadRecentBooks() { void RecentBooksActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - // Load data loadRecentBooks(); selectorIndex = 0; - updateRequired = true; - - xTaskCreate(&RecentBooksActivity::taskTrampoline, "RecentBooksActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void RecentBooksActivity::onExit() { Activity::onExit(); - - // 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; - recentBooks.clear(); } @@ -87,38 +63,26 @@ void RecentBooksActivity::loop() { buttonNavigator.onNextRelease([this, listSize] { selectorIndex = ButtonNavigator::nextIndex(static_cast(selectorIndex), listSize); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this, listSize] { selectorIndex = ButtonNavigator::previousIndex(static_cast(selectorIndex), listSize); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this, listSize, pageItems] { selectorIndex = ButtonNavigator::nextPageIndex(static_cast(selectorIndex), listSize, pageItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this, listSize, pageItems] { selectorIndex = ButtonNavigator::previousPageIndex(static_cast(selectorIndex), listSize, pageItems); - updateRequired = true; + requestUpdate(); }); } -void RecentBooksActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void RecentBooksActivity::render() const { +void RecentBooksActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/home/RecentBooksActivity.h b/src/activities/home/RecentBooksActivity.h index fee89981..3c9819c1 100644 --- a/src/activities/home/RecentBooksActivity.h +++ b/src/activities/home/RecentBooksActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -13,12 +10,9 @@ class RecentBooksActivity final : public Activity { private: - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; size_t selectorIndex = 0; - bool updateRequired = false; // Recent tab state std::vector recentBooks; @@ -27,10 +21,6 @@ class RecentBooksActivity final : public Activity { const std::function onSelectBook; const std::function onGoHome; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; - // Data loading void loadRecentBooks(); @@ -42,4 +32,5 @@ class RecentBooksActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/network/CalibreConnectActivity.cpp b/src/activities/network/CalibreConnectActivity.cpp index dd5e14bb..fdf4db6c 100644 --- a/src/activities/network/CalibreConnectActivity.cpp +++ b/src/activities/network/CalibreConnectActivity.cpp @@ -14,16 +14,10 @@ namespace { constexpr const char* HOSTNAME = "crosspoint"; } // namespace -void CalibreConnectActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void CalibreConnectActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - updateRequired = true; + requestUpdate(); state = CalibreConnectState::WIFI_SELECTION; connectedIP.clear(); connectedSSID.clear(); @@ -35,13 +29,6 @@ void CalibreConnectActivity::onEnter() { lastCompleteAt = 0; exitRequested = false; - xTaskCreate(&CalibreConnectActivity::taskTrampoline, "CalibreConnectTask", - 2048, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); - if (WiFi.status() != WL_CONNECTED) { enterNewActivity(new WifiSelectionActivity(renderer, mappedInput, [this](const bool connected) { onWifiSelectionComplete(connected); })); @@ -63,14 +50,6 @@ void CalibreConnectActivity::onExit() { delay(30); WiFi.mode(WIFI_OFF); delay(30); - - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; } void CalibreConnectActivity::onWifiSelectionComplete(const bool connected) { @@ -92,7 +71,7 @@ void CalibreConnectActivity::onWifiSelectionComplete(const bool connected) { void CalibreConnectActivity::startWebServer() { state = CalibreConnectState::SERVER_STARTING; - updateRequired = true; + requestUpdate(); if (MDNS.begin(HOSTNAME)) { // mDNS is optional for the Calibre plugin but still helpful for users. @@ -104,10 +83,10 @@ void CalibreConnectActivity::startWebServer() { if (webServer->isRunning()) { state = CalibreConnectState::SERVER_RUNNING; - updateRequired = true; + requestUpdate(); } else { state = CalibreConnectState::ERROR; - updateRequired = true; + requestUpdate(); } } @@ -178,7 +157,7 @@ void CalibreConnectActivity::loop() { changed = true; } if (changed) { - updateRequired = true; + requestUpdate(); } } @@ -188,19 +167,7 @@ void CalibreConnectActivity::loop() { } } -void CalibreConnectActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void CalibreConnectActivity::render() const { +void CalibreConnectActivity::render(Activity::RenderLock&&) { if (state == CalibreConnectState::SERVER_RUNNING) { renderer.clearScreen(); renderServerRunning(); diff --git a/src/activities/network/CalibreConnectActivity.h b/src/activities/network/CalibreConnectActivity.h index 08cf4bb4..d1d2bfcf 100644 --- a/src/activities/network/CalibreConnectActivity.h +++ b/src/activities/network/CalibreConnectActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -17,9 +14,6 @@ enum class CalibreConnectState { WIFI_SELECTION, SERVER_STARTING, SERVER_RUNNING * but renders Calibre-specific instructions instead of the web transfer UI. */ class CalibreConnectActivity final : public ActivityWithSubactivity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; CalibreConnectState state = CalibreConnectState::WIFI_SELECTION; const std::function onComplete; @@ -34,9 +28,6 @@ class CalibreConnectActivity final : public ActivityWithSubactivity { unsigned long lastCompleteAt = 0; bool exitRequested = false; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; void renderServerRunning() const; void onWifiSelectionComplete(bool connected); @@ -50,6 +41,7 @@ class CalibreConnectActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; bool skipLoopDelay() override { return webServer && webServer->isRunning(); } bool preventAutoSleep() override { return webServer && webServer->isRunning(); } }; diff --git a/src/activities/network/CrossPointWebServerActivity.cpp b/src/activities/network/CrossPointWebServerActivity.cpp index 6c52d791..076e4e29 100644 --- a/src/activities/network/CrossPointWebServerActivity.cpp +++ b/src/activities/network/CrossPointWebServerActivity.cpp @@ -29,17 +29,10 @@ DNSServer* dnsServer = nullptr; constexpr uint16_t DNS_PORT = 53; } // namespace -void CrossPointWebServerActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void CrossPointWebServerActivity::onEnter() { ActivityWithSubactivity::onEnter(); - LOG_DBG("WEBACT] [MEM", "Free heap at onEnter: %d bytes", ESP.getFreeHeap()); - - renderingMutex = xSemaphoreCreateMutex(); + LOG_DBG("WEBACT", "Free heap at onEnter: %d bytes", ESP.getFreeHeap()); // Reset state state = WebServerActivityState::MODE_SELECTION; @@ -48,14 +41,7 @@ void CrossPointWebServerActivity::onEnter() { connectedIP.clear(); connectedSSID.clear(); lastHandleClientTime = 0; - updateRequired = true; - - xTaskCreate(&CrossPointWebServerActivity::taskTrampoline, "WebServerActivityTask", - 2048, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); // Launch network mode selection subactivity LOG_DBG("WEBACT", "Launching NetworkModeSelectionActivity..."); @@ -68,7 +54,7 @@ void CrossPointWebServerActivity::onEnter() { void CrossPointWebServerActivity::onExit() { ActivityWithSubactivity::onExit(); - LOG_DBG("WEBACT] [MEM", "Free heap at onExit start: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WEBACT", "Free heap at onExit start: %d bytes", ESP.getFreeHeap()); state = WebServerActivityState::SHUTTING_DOWN; @@ -103,27 +89,7 @@ void CrossPointWebServerActivity::onExit() { WiFi.mode(WIFI_OFF); delay(30); // Allow WiFi hardware to power down - LOG_DBG("WEBACT] [MEM", "Free heap after WiFi disconnect: %d bytes", ESP.getFreeHeap()); - - // Acquire mutex before deleting task - LOG_DBG("WEBACT", "Acquiring rendering mutex before task deletion..."); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - - // Delete the display task - LOG_DBG("WEBACT", "Deleting display task..."); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - LOG_DBG("WEBACT", "Display task deleted"); - } - - // Delete the mutex - LOG_DBG("WEBACT", "Deleting mutex..."); - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; - LOG_DBG("WEBACT", "Mutex deleted"); - - LOG_DBG("WEBACT] [MEM", "Free heap at onExit end: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WEBACT", "Free heap at onExit end: %d bytes", ESP.getFreeHeap()); } void CrossPointWebServerActivity::onNetworkModeSelected(const NetworkMode mode) { @@ -165,7 +131,7 @@ void CrossPointWebServerActivity::onNetworkModeSelected(const NetworkMode mode) } else { // AP mode - start access point state = WebServerActivityState::AP_STARTING; - updateRequired = true; + requestUpdate(); startAccessPoint(); } } @@ -200,7 +166,7 @@ void CrossPointWebServerActivity::onWifiSelectionComplete(const bool connected) void CrossPointWebServerActivity::startAccessPoint() { LOG_DBG("WEBACT", "Starting Access Point mode..."); - LOG_DBG("WEBACT] [MEM", "Free heap before AP start: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WEBACT", "Free heap before AP start: %d bytes", ESP.getFreeHeap()); // Configure and start the AP WiFi.mode(WIFI_AP); @@ -248,7 +214,7 @@ void CrossPointWebServerActivity::startAccessPoint() { dnsServer->start(DNS_PORT, "*", apIP); LOG_DBG("WEBACT", "DNS server started for captive portal"); - LOG_DBG("WEBACT] [MEM", "Free heap after AP start: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WEBACT", "Free heap after AP start: %d bytes", ESP.getFreeHeap()); // Start the web server startWebServer(); @@ -267,9 +233,10 @@ void CrossPointWebServerActivity::startWebServer() { // Force an immediate render since we're transitioning from a subactivity // that had its own rendering task. We need to make sure our display is shown. - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); + { + RenderLock lock(*this); + render(std::move(lock)); + } LOG_DBG("WEBACT", "Rendered File Transfer screen"); } else { LOG_ERR("WEBACT", "ERROR: Failed to start web server!"); @@ -312,7 +279,7 @@ void CrossPointWebServerActivity::loop() { LOG_DBG("WEBACT", "WiFi disconnected! Status: %d", wifiStatus); // Show error and exit gracefully state = WebServerActivityState::SHUTTING_DOWN; - updateRequired = true; + requestUpdate(); return; } // Log weak signal warnings @@ -368,19 +335,7 @@ void CrossPointWebServerActivity::loop() { } } -void CrossPointWebServerActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void CrossPointWebServerActivity::render() const { +void CrossPointWebServerActivity::render(Activity::RenderLock&&) { // Only render our own UI when server is running // Subactivities handle their own rendering if (state == WebServerActivityState::SERVER_RUNNING) { diff --git a/src/activities/network/CrossPointWebServerActivity.h b/src/activities/network/CrossPointWebServerActivity.h index a1189a57..145c11ea 100644 --- a/src/activities/network/CrossPointWebServerActivity.h +++ b/src/activities/network/CrossPointWebServerActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -31,9 +28,6 @@ enum class WebServerActivityState { * - Cleans up the server and shuts down WiFi on exit */ class CrossPointWebServerActivity final : public ActivityWithSubactivity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; WebServerActivityState state = WebServerActivityState::MODE_SELECTION; const std::function onGoBack; @@ -51,9 +45,6 @@ class CrossPointWebServerActivity final : public ActivityWithSubactivity { // Performance monitoring unsigned long lastHandleClientTime = 0; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; void renderServerRunning() const; void onNetworkModeSelected(NetworkMode mode); @@ -69,6 +60,7 @@ class CrossPointWebServerActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; bool skipLoopDelay() override { return webServer && webServer->isRunning(); } bool preventAutoSleep() override { return webServer && webServer->isRunning(); } }; diff --git a/src/activities/network/NetworkModeSelectionActivity.cpp b/src/activities/network/NetworkModeSelectionActivity.cpp index bee13d8c..abe984ff 100644 --- a/src/activities/network/NetworkModeSelectionActivity.cpp +++ b/src/activities/network/NetworkModeSelectionActivity.cpp @@ -16,42 +16,17 @@ const char* MENU_DESCRIPTIONS[MENU_ITEM_COUNT] = { }; } // namespace -void NetworkModeSelectionActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void NetworkModeSelectionActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - // Reset selection selectedIndex = 0; // Trigger first update - updateRequired = true; - - xTaskCreate(&NetworkModeSelectionActivity::taskTrampoline, "NetworkModeTask", - 2048, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void NetworkModeSelectionActivity::onExit() { - Activity::onExit(); - - // Wait until not rendering to delete task - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} +void NetworkModeSelectionActivity::onExit() { Activity::onExit(); } void NetworkModeSelectionActivity::loop() { // Handle back button - cancel @@ -75,28 +50,16 @@ void NetworkModeSelectionActivity::loop() { // Handle navigation buttonNavigator.onNext([this] { selectedIndex = ButtonNavigator::nextIndex(selectedIndex, MENU_ITEM_COUNT); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this] { selectedIndex = ButtonNavigator::previousIndex(selectedIndex, MENU_ITEM_COUNT); - updateRequired = true; + requestUpdate(); }); } -void NetworkModeSelectionActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void NetworkModeSelectionActivity::render() const { +void NetworkModeSelectionActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/network/NetworkModeSelectionActivity.h b/src/activities/network/NetworkModeSelectionActivity.h index 5441e1af..89fe3499 100644 --- a/src/activities/network/NetworkModeSelectionActivity.h +++ b/src/activities/network/NetworkModeSelectionActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include @@ -21,19 +18,13 @@ enum class NetworkMode { JOIN_NETWORK, CONNECT_CALIBRE, CREATE_HOTSPOT }; * The onCancel callback is called if the user presses back. */ class NetworkModeSelectionActivity final : public Activity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; int selectedIndex = 0; - bool updateRequired = false; + const std::function onModeSelected; const std::function onCancel; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; - public: explicit NetworkModeSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::function& onModeSelected, @@ -42,4 +33,5 @@ class NetworkModeSelectionActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/network/WifiSelectionActivity.cpp b/src/activities/network/WifiSelectionActivity.cpp index 547acc39..793f2492 100644 --- a/src/activities/network/WifiSelectionActivity.cpp +++ b/src/activities/network/WifiSelectionActivity.cpp @@ -12,21 +12,15 @@ #include "components/UITheme.h" #include "fontIds.h" -void WifiSelectionActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void WifiSelectionActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - // Load saved WiFi credentials - SD card operations need lock as we use SPI // for both - xSemaphoreTake(renderingMutex, portMAX_DELAY); - WIFI_STORE.loadFromFile(); - xSemaphoreGive(renderingMutex); + { + RenderLock lock(*this); + WIFI_STORE.loadFromFile(); + } // Reset state selectedNetworkIndex = 0; @@ -49,13 +43,8 @@ void WifiSelectionActivity::onEnter() { mac[5]); cachedMacAddress = std::string(macStr); - // Task creation - xTaskCreate(&WifiSelectionActivity::taskTrampoline, "WifiSelectionTask", - 4096, // Stack size (larger for WiFi operations) - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + // Trigger first update to show scanning message + requestUpdate(); // Attempt to auto-connect to the last network if (allowAutoConnect) { @@ -70,7 +59,7 @@ void WifiSelectionActivity::onEnter() { usedSavedPassword = true; autoConnecting = true; attemptConnection(); - updateRequired = true; + requestUpdate(); return; } } @@ -83,45 +72,25 @@ void WifiSelectionActivity::onEnter() { void WifiSelectionActivity::onExit() { Activity::onExit(); - LOG_DBG("WIFI] [MEM", "Free heap at onExit start: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WIFI", "Free heap at onExit start: %d bytes", ESP.getFreeHeap()); // Stop any ongoing WiFi scan LOG_DBG("WIFI", "Deleting WiFi scan..."); WiFi.scanDelete(); - LOG_DBG("WIFI] [MEM", "Free heap after scanDelete: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WIFI", "Free heap after scanDelete: %d bytes", ESP.getFreeHeap()); // Note: We do NOT disconnect WiFi here - the parent activity // (CrossPointWebServerActivity) manages WiFi connection state. We just clean // up the scan and task. - // Acquire mutex before deleting task to ensure task isn't using it - // This prevents hangs/crashes if the task holds the mutex when deleted - LOG_DBG("WIFI", "Acquiring rendering mutex before task deletion..."); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - - // Delete the display task (we now hold the mutex, so task is blocked if it - // needs it) - LOG_DBG("WIFI", "Deleting display task..."); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - LOG_DBG("WIFI", "Display task deleted"); - } - - // Now safe to delete the mutex since we own it - LOG_DBG("WIFI", "Deleting mutex..."); - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; - LOG_DBG("WIFI", "Mutex deleted"); - - LOG_DBG("WIFI] [MEM", "Free heap at onExit end: %d bytes", ESP.getFreeHeap()); + LOG_DBG("WIFI", "Free heap at onExit end: %d bytes", ESP.getFreeHeap()); } void WifiSelectionActivity::startWifiScan() { autoConnecting = false; state = WifiSelectionState::SCANNING; networks.clear(); - updateRequired = true; + requestUpdate(); // Set WiFi mode to station WiFi.mode(WIFI_STA); @@ -142,7 +111,7 @@ void WifiSelectionActivity::processWifiScanResults() { if (scanResult == WIFI_SCAN_FAILED) { state = WifiSelectionState::NETWORK_LIST; - updateRequired = true; + requestUpdate(); return; } @@ -191,7 +160,7 @@ void WifiSelectionActivity::processWifiScanResults() { WiFi.scanDelete(); state = WifiSelectionState::NETWORK_LIST; selectedNetworkIndex = 0; - updateRequired = true; + requestUpdate(); } void WifiSelectionActivity::selectNetwork(const int index) { @@ -221,7 +190,6 @@ void WifiSelectionActivity::selectNetwork(const int index) { // Show password entry state = WifiSelectionState::PASSWORD_ENTRY; // Don't allow screen updates while changing activity - xSemaphoreTake(renderingMutex, portMAX_DELAY); enterNewActivity(new KeyboardEntryActivity( renderer, mappedInput, "Enter WiFi Password", "", // No initial text @@ -234,11 +202,9 @@ void WifiSelectionActivity::selectNetwork(const int index) { }, [this] { state = WifiSelectionState::NETWORK_LIST; - updateRequired = true; exitActivity(); + requestUpdate(); })); - updateRequired = true; - xSemaphoreGive(renderingMutex); } else { // Connect directly for open networks attemptConnection(); @@ -250,7 +216,7 @@ void WifiSelectionActivity::attemptConnection() { connectionStartTime = millis(); connectedIP.clear(); connectionError.clear(); - updateRequired = true; + requestUpdate(); WiFi.mode(WIFI_STA); @@ -287,7 +253,7 @@ void WifiSelectionActivity::checkConnectionStatus() { if (!usedSavedPassword && !enteredPassword.empty()) { state = WifiSelectionState::SAVE_PROMPT; savePromptSelection = 0; // Default to "Yes" - updateRequired = true; + requestUpdate(); } else { // Using saved password or open network - complete immediately LOG_DBG("WIFI", @@ -304,7 +270,7 @@ void WifiSelectionActivity::checkConnectionStatus() { connectionError = "Error: Network not found"; } state = WifiSelectionState::CONNECTION_FAILED; - updateRequired = true; + requestUpdate(); return; } @@ -313,7 +279,7 @@ void WifiSelectionActivity::checkConnectionStatus() { WiFi.disconnect(); connectionError = "Error: Connection timeout"; state = WifiSelectionState::CONNECTION_FAILED; - updateRequired = true; + requestUpdate(); return; } } @@ -348,20 +314,19 @@ void WifiSelectionActivity::loop() { mappedInput.wasPressed(MappedInputManager::Button::Left)) { if (savePromptSelection > 0) { savePromptSelection--; - updateRequired = true; + requestUpdate(); } } else if (mappedInput.wasPressed(MappedInputManager::Button::Down) || mappedInput.wasPressed(MappedInputManager::Button::Right)) { if (savePromptSelection < 1) { savePromptSelection++; - updateRequired = true; + requestUpdate(); } } else if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { if (savePromptSelection == 0) { // User chose "Yes" - save the password - xSemaphoreTake(renderingMutex, portMAX_DELAY); + RenderLock lock(*this); WIFI_STORE.addCredential(selectedSSID, enteredPassword); - xSemaphoreGive(renderingMutex); } // Complete - parent will start web server onComplete(true); @@ -378,20 +343,19 @@ void WifiSelectionActivity::loop() { mappedInput.wasPressed(MappedInputManager::Button::Left)) { if (forgetPromptSelection > 0) { forgetPromptSelection--; - updateRequired = true; + requestUpdate(); } } else if (mappedInput.wasPressed(MappedInputManager::Button::Down) || mappedInput.wasPressed(MappedInputManager::Button::Right)) { if (forgetPromptSelection < 1) { forgetPromptSelection++; - updateRequired = true; + requestUpdate(); } } else if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { if (forgetPromptSelection == 1) { + RenderLock lock(*this); // User chose "Forget network" - forget the network - xSemaphoreTake(renderingMutex, portMAX_DELAY); WIFI_STORE.removeCredential(selectedSSID); - xSemaphoreGive(renderingMutex); // Update the network list to reflect the change const auto network = find_if(networks.begin(), networks.end(), [this](const WifiNetworkInfo& net) { return net.ssid == selectedSSID; }); @@ -430,7 +394,7 @@ void WifiSelectionActivity::loop() { // Go back to network list on failure for non-saved credentials state = WifiSelectionState::NETWORK_LIST; } - updateRequired = true; + requestUpdate(); return; } } @@ -465,7 +429,7 @@ void WifiSelectionActivity::loop() { selectedSSID = networks[selectedNetworkIndex].ssid; state = WifiSelectionState::FORGET_PROMPT; forgetPromptSelection = 0; // Default to "Cancel" - updateRequired = true; + requestUpdate(); return; } } @@ -473,12 +437,12 @@ void WifiSelectionActivity::loop() { // Handle navigation buttonNavigator.onNext([this] { selectedNetworkIndex = ButtonNavigator::nextIndex(selectedNetworkIndex, networks.size()); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this] { selectedNetworkIndex = ButtonNavigator::previousIndex(selectedNetworkIndex, networks.size()); - updateRequired = true; + requestUpdate(); }); } } @@ -500,32 +464,14 @@ std::string WifiSelectionActivity::getSignalStrengthIndicator(const int32_t rssi return " "; // Very weak } -void WifiSelectionActivity::displayTaskLoop() { - while (true) { - // If a subactivity is active, yield CPU time but don't render - if (subActivity) { - vTaskDelay(10 / portTICK_PERIOD_MS); - continue; - } - - // Don't render if we're in PASSWORD_ENTRY state - we're just transitioning - // from the keyboard subactivity back to the main activity - if (state == WifiSelectionState::PASSWORD_ENTRY) { - vTaskDelay(10 / portTICK_PERIOD_MS); - continue; - } - - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); +void WifiSelectionActivity::render(Activity::RenderLock&&) { + // Don't render if we're in PASSWORD_ENTRY state - we're just transitioning + // from the keyboard subactivity back to the main activity + if (state == WifiSelectionState::PASSWORD_ENTRY) { + requestUpdateAndWait(); + return; } -} -void WifiSelectionActivity::render() const { renderer.clearScreen(); switch (state) { diff --git a/src/activities/network/WifiSelectionActivity.h b/src/activities/network/WifiSelectionActivity.h index 32eb36db..34641f3c 100644 --- a/src/activities/network/WifiSelectionActivity.h +++ b/src/activities/network/WifiSelectionActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -45,10 +42,8 @@ enum class WifiSelectionState { * The onComplete callback receives true if connected successfully, false if cancelled. */ class WifiSelectionActivity final : public ActivityWithSubactivity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; - bool updateRequired = false; + WifiSelectionState state = WifiSelectionState::SCANNING; int selectedNetworkIndex = 0; std::vector networks; @@ -85,9 +80,6 @@ class WifiSelectionActivity final : public ActivityWithSubactivity { static constexpr unsigned long CONNECTION_TIMEOUT_MS = 15000; unsigned long connectionStartTime = 0; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; void renderNetworkList() const; void renderPasswordEntry() const; void renderConnecting() const; @@ -112,6 +104,7 @@ class WifiSelectionActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; // Get the IP address after successful connection const std::string& getConnectedIP() const { return connectedIP; } diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 18e0d581..c101d384 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -67,11 +67,6 @@ void applyReaderOrientation(GfxRenderer& renderer, const uint8_t orientation) { } // namespace -void EpubReaderActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void EpubReaderActivity::onEnter() { ActivityWithSubactivity::onEnter(); @@ -83,8 +78,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; @@ -179,14 +172,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() { @@ -195,14 +181,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(); @@ -217,7 +195,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 @@ -257,8 +235,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; @@ -276,7 +252,6 @@ void EpubReaderActivity::loop() { SETTINGS.orientation, hasDictionary, isBookmarked, epub->getCachePath(), [this](const uint8_t orientation) { onReaderMenuBack(orientation); }, [this](EpubReaderMenuActivity::MenuAction action) { onReaderMenuConfirm(action); })); - xSemaphoreGive(renderingMutex); } // Long press BACK (1s+) goes to file selection @@ -313,7 +288,7 @@ void EpubReaderActivity::loop() { if (currentSpineIndex > 0 && currentSpineIndex >= epub->getSpineItemsCount()) { currentSpineIndex = epub->getSpineItemsCount() - 1; nextPageNumber = UINT16_MAX; - updateRequired = true; + requestUpdate(); return; } @@ -326,13 +301,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; } @@ -347,7 +322,7 @@ void EpubReaderActivity::loop() { section.reset(); xSemaphoreGive(renderingMutex); } - updateRequired = true; + requestUpdate(); } else { if (section->currentPage < section->pageCount - 1) { section->currentPage++; @@ -359,7 +334,7 @@ void EpubReaderActivity::loop() { section.reset(); xSemaphoreGive(renderingMutex); } - updateRequired = true; + requestUpdate(); } } @@ -370,7 +345,7 @@ void EpubReaderActivity::onReaderMenuBack(const uint8_t orientation) { applyOrientation(orientation); // Force a half refresh on the next render to clear menu/popup artifacts pagesUntilFullRefresh = 1; - updateRequired = true; + requestUpdate(); } // Translate an absolute percent into a spine index plus a normalized position @@ -426,7 +401,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; @@ -506,7 +481,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction // and next menu open will reflect the updated state. exitActivity(); pagesUntilFullRefresh = 1; - updateRequired = true; + requestUpdate(); break; } case EpubReaderMenuActivity::MenuAction::REMOVE_BOOKMARK: { @@ -519,7 +494,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction vTaskDelay(750 / portTICK_PERIOD_MS); exitActivity(); pagesUntilFullRefresh = 1; - updateRequired = true; + requestUpdate(); break; } case EpubReaderMenuActivity::MenuAction::GO_TO_BOOKMARK: { @@ -533,13 +508,12 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int spineIdx = currentSpineIndex; const std::string path = epub->getPath(); - xSemaphoreTake(renderingMutex, portMAX_DELAY); exitActivity(); enterNewActivity(new EpubReaderChapterSelectionActivity( this->renderer, this->mappedInput, epub, path, spineIdx, currentP, totalP, [this] { exitActivity(); - updateRequired = true; + requestUpdate(); }, [this](const int newSpineIndex) { if (currentSpineIndex != newSpineIndex) { @@ -548,7 +522,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)) { @@ -557,21 +531,19 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction section.reset(); } exitActivity(); - updateRequired = true; + requestUpdate(); })); - xSemaphoreGive(renderingMutex); } // If no TOC either, just return to reader (menu already closed by callback) break; } - xSemaphoreTake(renderingMutex, portMAX_DELAY); exitActivity(); enterNewActivity(new EpubReaderBookmarkSelectionActivity( this->renderer, this->mappedInput, epub, std::move(bookmarks), epub->getCachePath(), [this] { exitActivity(); - updateRequired = true; + requestUpdate(); }, [this](const int newSpineIndex, const int newPage) { if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) { @@ -580,9 +552,8 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction section.reset(); } exitActivity(); - updateRequired = true; + requestUpdate(); })); - xSemaphoreGive(renderingMutex); break; } case EpubReaderMenuActivity::MenuAction::DELETE_DICT_CACHE: { @@ -608,8 +579,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(); @@ -618,7 +587,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) { @@ -627,7 +596,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)) { @@ -636,10 +605,9 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction section.reset(); } exitActivity(); - updateRequired = true; + requestUpdate(); })); - xSemaphoreGive(renderingMutex); break; } case EpubReaderMenuActivity::MenuAction::GO_TO_PERCENT: { @@ -650,7 +618,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f; } const int initialPercent = clampPercent(static_cast(bookProgress + 0.5f)); - xSemaphoreTake(renderingMutex, portMAX_DELAY); exitActivity(); enterNewActivity(new EpubReaderPercentSelectionActivity( renderer, mappedInput, initialPercent, @@ -658,59 +625,65 @@ 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::LOOKUP: { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - - // Compute margins (same logic as renderScreen) + // Gather data we need while holding the render lock int orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft; - renderer.getOrientedViewableTRBL(&orientedMarginTop, &orientedMarginRight, &orientedMarginBottom, - &orientedMarginLeft); - orientedMarginTop += SETTINGS.screenMargin; - orientedMarginLeft += SETTINGS.screenMargin; - orientedMarginRight += SETTINGS.screenMargin; - orientedMarginBottom += SETTINGS.screenMargin; - - if (SETTINGS.statusBar != CrossPointSettings::STATUS_BAR_MODE::NONE) { - auto metrics = UITheme::getInstance().getMetrics(); - const bool showProgressBar = - SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::BOOK_PROGRESS_BAR || - SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::ONLY_BOOK_PROGRESS_BAR || - SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::CHAPTER_PROGRESS_BAR; - orientedMarginBottom += statusBarMargin - SETTINGS.screenMargin + - (showProgressBar ? (metrics.bookProgressBarHeight + progressBarMarginTop) : 0); - } - - // Load the current page - auto pageForLookup = section ? section->loadPageFromSectionFile() : nullptr; - const int readerFontId = SETTINGS.getReaderFontId(); - const std::string bookCachePath = epub->getCachePath(); - const uint8_t currentOrientation = SETTINGS.orientation; - - // Get first word of next page for cross-page hyphenation + std::unique_ptr pageForLookup; + int readerFontId; + std::string bookCachePath; + uint8_t currentOrientation; std::string nextPageFirstWord; - if (section && section->currentPage < section->pageCount - 1) { - int savedPage = section->currentPage; - section->currentPage = savedPage + 1; - auto nextPage = section->loadPageFromSectionFile(); - section->currentPage = savedPage; - if (nextPage && !nextPage->elements.empty()) { - const auto* firstLine = static_cast(nextPage->elements[0].get()); - if (firstLine->getBlock() && !firstLine->getBlock()->getWords().empty()) { - nextPageFirstWord = firstLine->getBlock()->getWords().front(); + { + RenderLock lock(*this); + + // Compute margins (same logic as render) + renderer.getOrientedViewableTRBL(&orientedMarginTop, &orientedMarginRight, &orientedMarginBottom, + &orientedMarginLeft); + orientedMarginTop += SETTINGS.screenMargin; + orientedMarginLeft += SETTINGS.screenMargin; + orientedMarginRight += SETTINGS.screenMargin; + orientedMarginBottom += SETTINGS.screenMargin; + + if (SETTINGS.statusBar != CrossPointSettings::STATUS_BAR_MODE::NONE) { + auto metrics = UITheme::getInstance().getMetrics(); + const bool showProgressBar = + SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::BOOK_PROGRESS_BAR || + SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::ONLY_BOOK_PROGRESS_BAR || + SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::CHAPTER_PROGRESS_BAR; + orientedMarginBottom += statusBarMargin - SETTINGS.screenMargin + + (showProgressBar ? (metrics.bookProgressBarHeight + progressBarMarginTop) : 0); + } + + // Load the current page + pageForLookup = section ? section->loadPageFromSectionFile() : nullptr; + readerFontId = SETTINGS.getReaderFontId(); + bookCachePath = epub->getCachePath(); + currentOrientation = SETTINGS.orientation; + + // Get first word of next page for cross-page hyphenation + if (section && section->currentPage < section->pageCount - 1) { + int savedPage = section->currentPage; + section->currentPage = savedPage + 1; + auto nextPage = section->loadPageFromSectionFile(); + section->currentPage = savedPage; + if (nextPage && !nextPage->elements.empty()) { + const auto* firstLine = static_cast(nextPage->elements[0].get()); + if (firstLine->getBlock() && !firstLine->getBlock()->getWords().empty()) { + nextPageFirstWord = firstLine->getBlock()->getWords().front(); + } } } } - + // Lock released — safe to call enterNewActivity which takes its own lock exitActivity(); if (pageForLookup) { @@ -718,18 +691,13 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction renderer, mappedInput, std::move(pageForLookup), readerFontId, orientedMarginLeft, orientedMarginTop, bookCachePath, currentOrientation, [this]() { pendingSubactivityExit = true; }, nextPageFirstWord)); } - - xSemaphoreGive(renderingMutex); break; } case EpubReaderMenuActivity::MenuAction::LOOKED_UP_WORDS: { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - exitActivity(); enterNewActivity(new LookedUpWordsActivity( renderer, mappedInput, epub->getCachePath(), SETTINGS.getReaderFontId(), SETTINGS.orientation, [this]() { pendingSubactivityExit = true; }, [this]() { pendingSubactivityExit = true; })); - xSemaphoreGive(renderingMutex); break; } case EpubReaderMenuActivity::MenuAction::GO_HOME: { @@ -765,7 +733,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(); @@ -784,7 +751,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction } pendingSubactivityExit = true; })); - xSemaphoreGive(renderingMutex); } break; } @@ -821,20 +787,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; } @@ -960,7 +914,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); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 56065c7c..3d4240af 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -1,9 +1,6 @@ #pragma once #include #include -#include -#include -#include #include "DictionaryWordSelectActivity.h" #include "EpubReaderMenuActivity.h" @@ -13,8 +10,6 @@ class EpubReaderActivity final : public ActivityWithSubactivity { std::shared_ptr epub; std::unique_ptr
section = nullptr; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; int currentSpineIndex = 0; int nextPageNumber = 0; int pagesUntilFullRefresh = 0; @@ -25,7 +20,6 @@ class EpubReaderActivity final : public ActivityWithSubactivity { bool pendingPercentJump = false; // Normalized 0.0-1.0 progress within the target spine item, computed from book percentage. float pendingSpineProgress = 0.0f; - bool updateRequired = false; bool pendingSubactivityExit = false; // Defer subactivity exit to avoid use-after-free bool pendingGoHome = false; // Defer go home to avoid race condition with display task bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit @@ -33,9 +27,6 @@ class EpubReaderActivity final : public ActivityWithSubactivity { const std::function onGoBack; const std::function onGoHome; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); void renderContents(std::unique_ptr page, int orientedMarginTop, int orientedMarginRight, int orientedMarginBottom, int orientedMarginLeft); void renderStatusBar(int orientedMarginRight, int orientedMarginBottom, int orientedMarginLeft) const; @@ -56,10 +47,11 @@ class EpubReaderActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&& lock) override; // Defer low-power mode and auto-sleep while a section is loading/building. // !section covers the period before the Section object is created (including // cover prerendering in onEnter). loadingSection covers the full !section block - // in renderScreen (including createSectionFile), during which section is non-null + // in render (including createSectionFile), during which section is non-null // but the section file is still being built. bool preventAutoSleep() override { return !section || loadingSection; } }; diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp index 9a11b1a3..ba62015b 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp @@ -24,11 +24,6 @@ int EpubReaderChapterSelectionActivity::getPageItems() const { return std::max(1, availableHeight / lineHeight); } -void EpubReaderChapterSelectionActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void EpubReaderChapterSelectionActivity::onEnter() { ActivityWithSubactivity::onEnter(); @@ -36,35 +31,16 @@ void EpubReaderChapterSelectionActivity::onEnter() { return; } - renderingMutex = xSemaphoreCreateMutex(); - selectorIndex = epub->getTocIndexForSpineIndex(currentSpineIndex); if (selectorIndex == -1) { selectorIndex = 0; } // Trigger first update - updateRequired = true; - xTaskCreate(&EpubReaderChapterSelectionActivity::taskTrampoline, "EpubReaderChapterSelectionActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void EpubReaderChapterSelectionActivity::onExit() { - ActivityWithSubactivity::onExit(); - - // 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; -} +void EpubReaderChapterSelectionActivity::onExit() { ActivityWithSubactivity::onExit(); } void EpubReaderChapterSelectionActivity::loop() { if (subActivity) { @@ -88,38 +64,26 @@ void EpubReaderChapterSelectionActivity::loop() { buttonNavigator.onNextRelease([this, totalItems] { selectorIndex = ButtonNavigator::nextIndex(selectorIndex, totalItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this, totalItems] { selectorIndex = ButtonNavigator::previousIndex(selectorIndex, totalItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this, totalItems, pageItems] { selectorIndex = ButtonNavigator::nextPageIndex(selectorIndex, totalItems, pageItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this, totalItems, pageItems] { selectorIndex = ButtonNavigator::previousPageIndex(selectorIndex, totalItems, pageItems); - updateRequired = true; + requestUpdate(); }); } -void EpubReaderChapterSelectionActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void EpubReaderChapterSelectionActivity::renderScreen() { +void EpubReaderChapterSelectionActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.h b/src/activities/reader/EpubReaderChapterSelectionActivity.h index 325d562a..28a6f165 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.h +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include @@ -12,14 +9,12 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity { std::shared_ptr epub; std::string epubPath; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; int currentSpineIndex = 0; int currentPage = 0; int totalPagesInSpine = 0; int selectorIndex = 0; - bool updateRequired = false; + const std::function onGoBack; const std::function onSelectSpineIndex; const std::function onSyncPosition; @@ -31,10 +26,6 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity // Total TOC items count int getTotalItems() const; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); - public: explicit EpubReaderChapterSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::shared_ptr& epub, const std::string& epubPath, @@ -54,4 +45,5 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 3aacef2f..a9281093 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -8,39 +8,10 @@ void EpubReaderMenuActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - updateRequired = true; - - xTaskCreate(&EpubReaderMenuActivity::taskTrampoline, "EpubMenuTask", 4096, this, 1, &displayTaskHandle); + requestUpdate(); } -void EpubReaderMenuActivity::onExit() { - ActivityWithSubactivity::onExit(); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} - -void EpubReaderMenuActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - -void EpubReaderMenuActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} +void EpubReaderMenuActivity::onExit() { ActivityWithSubactivity::onExit(); } void EpubReaderMenuActivity::loop() { if (subActivity) { @@ -51,12 +22,12 @@ void EpubReaderMenuActivity::loop() { // Handle navigation buttonNavigator.onNext([this] { selectedIndex = ButtonNavigator::nextIndex(selectedIndex, static_cast(menuItems.size())); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this] { selectedIndex = ButtonNavigator::previousIndex(selectedIndex, static_cast(menuItems.size())); - updateRequired = true; + requestUpdate(); }); // Use local variables for items we need to check after potential deletion @@ -65,7 +36,7 @@ void EpubReaderMenuActivity::loop() { if (selectedAction == MenuAction::ROTATE_SCREEN) { // Cycle orientation preview locally; actual rotation happens on menu exit. pendingOrientation = (pendingOrientation + 1) % orientationLabels.size(); - updateRequired = true; + requestUpdate(); return; } if (selectedAction == MenuAction::LETTERBOX_FILL) { @@ -73,7 +44,7 @@ void EpubReaderMenuActivity::loop() { int idx = (letterboxFillToIndex() + 1) % LETTERBOX_FILL_OPTION_COUNT; pendingLetterboxFill = indexToLetterboxFill(idx); saveLetterboxFill(); - updateRequired = true; + requestUpdate(); return; } @@ -92,7 +63,7 @@ void EpubReaderMenuActivity::loop() { } } -void EpubReaderMenuActivity::renderScreen() { +void EpubReaderMenuActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); const auto orientation = renderer.getOrientation(); diff --git a/src/activities/reader/EpubReaderMenuActivity.h b/src/activities/reader/EpubReaderMenuActivity.h index f4b511e5..6c13d86e 100644 --- a/src/activities/reader/EpubReaderMenuActivity.h +++ b/src/activities/reader/EpubReaderMenuActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include #include @@ -55,6 +52,7 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: struct MenuItem { @@ -65,9 +63,7 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { std::vector menuItems; int selectedIndex = 0; - bool updateRequired = false; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; + ButtonNavigator buttonNavigator; std::string title = "Reader Menu"; uint8_t pendingOrientation = 0; @@ -83,6 +79,7 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { const std::function onBack; const std::function onAction; +<<<<<<< HEAD // Map the internal override value to an index into letterboxFillLabels. int letterboxFillToIndex() const { @@ -128,7 +125,4 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { return items; } - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); }; diff --git a/src/activities/reader/EpubReaderPercentSelectionActivity.cpp b/src/activities/reader/EpubReaderPercentSelectionActivity.cpp index ec7293d8..060f0ceb 100644 --- a/src/activities/reader/EpubReaderPercentSelectionActivity.cpp +++ b/src/activities/reader/EpubReaderPercentSelectionActivity.cpp @@ -15,41 +15,10 @@ constexpr int kLargeStep = 10; void EpubReaderPercentSelectionActivity::onEnter() { ActivityWithSubactivity::onEnter(); // Set up rendering task and mark first frame dirty. - renderingMutex = xSemaphoreCreateMutex(); - updateRequired = true; - xTaskCreate(&EpubReaderPercentSelectionActivity::taskTrampoline, "EpubPercentSlider", 4096, this, 1, - &displayTaskHandle); + requestUpdate(); } -void EpubReaderPercentSelectionActivity::onExit() { - ActivityWithSubactivity::onExit(); - // Ensure the render task is stopped before freeing the mutex. - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} - -void EpubReaderPercentSelectionActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - -void EpubReaderPercentSelectionActivity::displayTaskLoop() { - while (true) { - // Render only when the view is dirty and no subactivity is running. - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} +void EpubReaderPercentSelectionActivity::onExit() { ActivityWithSubactivity::onExit(); } void EpubReaderPercentSelectionActivity::adjustPercent(const int delta) { // Apply delta and clamp within 0-100. @@ -59,7 +28,7 @@ void EpubReaderPercentSelectionActivity::adjustPercent(const int delta) { } else if (percent > 100) { percent = 100; } - updateRequired = true; + requestUpdate(); } void EpubReaderPercentSelectionActivity::loop() { @@ -86,7 +55,7 @@ void EpubReaderPercentSelectionActivity::loop() { buttonNavigator.onPressAndContinuous({MappedInputManager::Button::Down}, [this] { adjustPercent(-kLargeStep); }); } -void EpubReaderPercentSelectionActivity::renderScreen() { +void EpubReaderPercentSelectionActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); // Title and numeric percent value. diff --git a/src/activities/reader/EpubReaderPercentSelectionActivity.h b/src/activities/reader/EpubReaderPercentSelectionActivity.h index 8d3ec96f..08502d88 100644 --- a/src/activities/reader/EpubReaderPercentSelectionActivity.h +++ b/src/activities/reader/EpubReaderPercentSelectionActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include @@ -23,15 +20,12 @@ class EpubReaderPercentSelectionActivity final : public ActivityWithSubactivity void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: // Current percent value (0-100) shown on the slider. int percent = 0; - // Render dirty flag for the task loop. - bool updateRequired = false; - // FreeRTOS task and mutex for rendering. - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; + ButtonNavigator buttonNavigator; // Callback invoked when the user confirms a percent. @@ -39,10 +33,6 @@ class EpubReaderPercentSelectionActivity final : public ActivityWithSubactivity // Callback invoked when the user cancels the slider. const std::function onCancel; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - // Render the slider UI. - void renderScreen(); // Change the current percent by a delta and clamp within bounds. void adjustPercent(int delta); }; diff --git a/src/activities/reader/KOReaderSyncActivity.cpp b/src/activities/reader/KOReaderSyncActivity.cpp index c32f9650..54c4dabe 100644 --- a/src/activities/reader/KOReaderSyncActivity.cpp +++ b/src/activities/reader/KOReaderSyncActivity.cpp @@ -40,11 +40,6 @@ void syncTimeWithNTP() { } } // namespace -void KOReaderSyncActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void KOReaderSyncActivity::onWifiSelectionComplete(const bool success) { exitActivity(); @@ -60,7 +55,7 @@ void KOReaderSyncActivity::onWifiSelectionComplete(const bool success) { state = SYNCING; statusMessage = "Syncing time..."; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); // Sync time with NTP before making API requests syncTimeWithNTP(); @@ -68,7 +63,7 @@ void KOReaderSyncActivity::onWifiSelectionComplete(const bool success) { xSemaphoreTake(renderingMutex, portMAX_DELAY); statusMessage = "Calculating document hash..."; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); performSync(); } @@ -85,7 +80,7 @@ void KOReaderSyncActivity::performSync() { state = SYNC_FAILED; statusMessage = "Failed to calculate document hash"; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } @@ -94,8 +89,7 @@ void KOReaderSyncActivity::performSync() { xSemaphoreTake(renderingMutex, portMAX_DELAY); statusMessage = "Fetching remote progress..."; xSemaphoreGive(renderingMutex); - updateRequired = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + requestUpdateAndWait(); // Fetch remote progress const auto result = KOReaderSyncClient::getProgress(documentHash, remoteProgress); @@ -106,7 +100,7 @@ void KOReaderSyncActivity::performSync() { state = NO_REMOTE_PROGRESS; hasRemoteProgress = false; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } @@ -115,7 +109,7 @@ void KOReaderSyncActivity::performSync() { state = SYNC_FAILED; statusMessage = KOReaderSyncClient::errorString(result); xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } @@ -132,7 +126,7 @@ void KOReaderSyncActivity::performSync() { state = SHOWING_RESULT; selectedOption = 0; // Default to "Apply" xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); } void KOReaderSyncActivity::performUpload() { @@ -140,8 +134,8 @@ void KOReaderSyncActivity::performUpload() { state = UPLOADING; statusMessage = "Uploading progress..."; xSemaphoreGive(renderingMutex); - updateRequired = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + requestUpdate(); + requestUpdateAndWait(); // Convert current position to KOReader format CrossPointPosition localPos = {currentSpineIndex, currentPage, totalPagesInSpine}; @@ -159,32 +153,23 @@ void KOReaderSyncActivity::performUpload() { state = SYNC_FAILED; statusMessage = KOReaderSyncClient::errorString(result); xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } xSemaphoreTake(renderingMutex, portMAX_DELAY); state = UPLOAD_COMPLETE; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); } void KOReaderSyncActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - - xTaskCreate(&KOReaderSyncActivity::taskTrampoline, "KOSyncTask", - 4096, // Stack size (larger for network operations) - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); - // Check for credentials first if (!KOREADER_STORE.hasCredentials()) { state = NO_CREDENTIALS; - updateRequired = true; + requestUpdate(); return; } @@ -197,7 +182,7 @@ void KOReaderSyncActivity::onEnter() { LOG_DBG("KOSync", "Already connected to WiFi"); state = SYNCING; statusMessage = "Syncing time..."; - updateRequired = true; + requestUpdate(); // Perform sync directly (will be handled in loop) xTaskCreate( @@ -208,7 +193,7 @@ void KOReaderSyncActivity::onEnter() { xSemaphoreTake(self->renderingMutex, portMAX_DELAY); self->statusMessage = "Calculating document hash..."; xSemaphoreGive(self->renderingMutex); - self->updateRequired = true; + self->requestUpdate(); self->performSync(); vTaskDelete(nullptr); }, @@ -230,30 +215,9 @@ void KOReaderSyncActivity::onExit() { delay(100); WiFi.mode(WIFI_OFF); delay(100); - - // Wait until not rendering to delete task - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; } -void KOReaderSyncActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void KOReaderSyncActivity::render() { +void KOReaderSyncActivity::render(Activity::RenderLock&&) { if (subActivity) { return; } @@ -388,11 +352,11 @@ void KOReaderSyncActivity::loop() { if (mappedInput.wasPressed(MappedInputManager::Button::Up) || mappedInput.wasPressed(MappedInputManager::Button::Left)) { selectedOption = (selectedOption + 1) % 2; // Wrap around among 2 options - updateRequired = true; + requestUpdate(); } else if (mappedInput.wasPressed(MappedInputManager::Button::Down) || mappedInput.wasPressed(MappedInputManager::Button::Right)) { selectedOption = (selectedOption + 1) % 2; // Wrap around among 2 options - updateRequired = true; + requestUpdate(); } if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { diff --git a/src/activities/reader/KOReaderSyncActivity.h b/src/activities/reader/KOReaderSyncActivity.h index b1a8dca0..bd29bc0a 100644 --- a/src/activities/reader/KOReaderSyncActivity.h +++ b/src/activities/reader/KOReaderSyncActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include #include @@ -45,6 +42,7 @@ class KOReaderSyncActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; bool preventAutoSleep() override { return state == CONNECTING || state == SYNCING; } private: @@ -66,10 +64,6 @@ class KOReaderSyncActivity final : public ActivityWithSubactivity { int currentPage; int totalPagesInSpine; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; - State state = WIFI_SELECTION; std::string statusMessage; std::string documentHash; @@ -91,8 +85,4 @@ class KOReaderSyncActivity final : public ActivityWithSubactivity { void onWifiSelectionComplete(bool success); void performSync(); void performUpload(); - - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); }; diff --git a/src/activities/reader/TxtReaderActivity.cpp b/src/activities/reader/TxtReaderActivity.cpp index 010a1774..98aa877b 100644 --- a/src/activities/reader/TxtReaderActivity.cpp +++ b/src/activities/reader/TxtReaderActivity.cpp @@ -25,11 +25,6 @@ constexpr uint32_t CACHE_MAGIC = 0x54585449; // "TXTI" constexpr uint8_t CACHE_VERSION = 2; // Increment when cache format changes } // namespace -void TxtReaderActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void TxtReaderActivity::onEnter() { ActivityWithSubactivity::onEnter(); @@ -55,8 +50,6 @@ void TxtReaderActivity::onEnter() { break; } - renderingMutex = xSemaphoreCreateMutex(); - txt->setupCacheDir(); // Prerender covers and thumbnails on first open so Home and Sleep screens are instant. @@ -106,14 +99,7 @@ void TxtReaderActivity::onEnter() { RECENT_BOOKS.addBook(filePath, fileName, "", txt->getThumbBmpPath()); // Trigger first update - updateRequired = true; - - xTaskCreate(&TxtReaderActivity::taskTrampoline, "TxtReaderActivityTask", - 6144, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void TxtReaderActivity::onExit() { @@ -122,14 +108,6 @@ void TxtReaderActivity::onExit() { // Reset orientation back to portrait for the rest of the UI renderer.setOrientation(GfxRenderer::Orientation::Portrait); - // Wait until not rendering to delete task - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; pageOffsets.clear(); currentPageLines.clear(); APP_STATE.readerActivityLoadCount = 0; @@ -175,22 +153,10 @@ void TxtReaderActivity::loop() { if (prevTriggered && currentPage > 0) { currentPage--; - updateRequired = true; + requestUpdate(); } else if (nextTriggered && currentPage < totalPages - 1) { currentPage++; - updateRequired = true; - } -} - -void TxtReaderActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); + requestUpdate(); } } @@ -413,7 +379,7 @@ bool TxtReaderActivity::loadPageAtOffset(size_t offset, std::vector return !outLines.empty(); } -void TxtReaderActivity::renderScreen() { +void TxtReaderActivity::render(Activity::RenderLock&&) { if (!txt) { return; } diff --git a/src/activities/reader/TxtReaderActivity.h b/src/activities/reader/TxtReaderActivity.h index 74618808..ca061ce2 100644 --- a/src/activities/reader/TxtReaderActivity.h +++ b/src/activities/reader/TxtReaderActivity.h @@ -1,9 +1,6 @@ #pragma once #include -#include -#include -#include #include @@ -12,12 +9,11 @@ class TxtReaderActivity final : public ActivityWithSubactivity { std::unique_ptr txt; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; + int currentPage = 0; int totalPages = 1; int pagesUntilFullRefresh = 0; - bool updateRequired = false; + const std::function onGoBack; const std::function onGoHome; @@ -33,9 +29,6 @@ class TxtReaderActivity final : public ActivityWithSubactivity { int cachedScreenMargin = 0; uint8_t cachedParagraphAlignment = CrossPointSettings::LEFT_ALIGN; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); void renderPage(); void renderStatusBar(int orientedMarginRight, int orientedMarginBottom, int orientedMarginLeft) const; @@ -57,6 +50,7 @@ class TxtReaderActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; // Defer low-power mode and auto-sleep while the reader is initializing // (cover prerendering, page index building on first open). bool preventAutoSleep() override { return !initialized; } diff --git a/src/activities/reader/XtcReaderActivity.cpp b/src/activities/reader/XtcReaderActivity.cpp index d042ab3e..51c8fb19 100644 --- a/src/activities/reader/XtcReaderActivity.cpp +++ b/src/activities/reader/XtcReaderActivity.cpp @@ -26,11 +26,6 @@ constexpr unsigned long skipPageMs = 700; constexpr unsigned long goHomeMs = 1000; } // namespace -void XtcReaderActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void XtcReaderActivity::onEnter() { ActivityWithSubactivity::onEnter(); @@ -38,8 +33,6 @@ void XtcReaderActivity::onEnter() { return; } - renderingMutex = xSemaphoreCreateMutex(); - xtc->setupCacheDir(); // Load saved progress @@ -93,27 +86,12 @@ void XtcReaderActivity::onEnter() { RECENT_BOOKS.addBook(xtc->getPath(), xtc->getTitle(), xtc->getAuthor(), xtc->getThumbBmpPath()); // Trigger first update - updateRequired = true; - - xTaskCreate(&XtcReaderActivity::taskTrampoline, "XtcReaderActivityTask", - 4096, // Stack size (smaller than EPUB since no parsing needed) - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void XtcReaderActivity::onExit() { ActivityWithSubactivity::onExit(); - // Wait until not rendering to delete task - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; APP_STATE.readerActivityLoadCount = 0; APP_STATE.saveToFile(); xtc.reset(); @@ -129,20 +107,18 @@ void XtcReaderActivity::loop() { // Enter chapter selection activity if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { if (xtc && xtc->hasChapters() && !xtc->getChapters().empty()) { - xSemaphoreTake(renderingMutex, portMAX_DELAY); exitActivity(); enterNewActivity(new XtcReaderChapterSelectionActivity( this->renderer, this->mappedInput, xtc, currentPage, [this] { exitActivity(); - updateRequired = true; + requestUpdate(); }, [this](const uint32_t newPage) { currentPage = newPage; exitActivity(); - updateRequired = true; + requestUpdate(); })); - xSemaphoreGive(renderingMutex); } } @@ -179,7 +155,7 @@ void XtcReaderActivity::loop() { // Handle end of book if (currentPage >= xtc->getPageCount()) { currentPage = xtc->getPageCount() - 1; - updateRequired = true; + requestUpdate(); return; } @@ -192,29 +168,17 @@ void XtcReaderActivity::loop() { } else { currentPage = 0; } - updateRequired = true; + requestUpdate(); } else if (nextTriggered) { currentPage += skipAmount; if (currentPage >= xtc->getPageCount()) { currentPage = xtc->getPageCount(); // Allow showing "End of book" } - updateRequired = true; + requestUpdate(); } } -void XtcReaderActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void XtcReaderActivity::renderScreen() { +void XtcReaderActivity::render(Activity::RenderLock&&) { if (!xtc) { return; } diff --git a/src/activities/reader/XtcReaderActivity.h b/src/activities/reader/XtcReaderActivity.h index 579e1777..c9e8997c 100644 --- a/src/activities/reader/XtcReaderActivity.h +++ b/src/activities/reader/XtcReaderActivity.h @@ -8,25 +8,18 @@ #pragma once #include -#include -#include -#include #include "activities/ActivityWithSubactivity.h" class XtcReaderActivity final : public ActivityWithSubactivity { std::shared_ptr xtc; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; + uint32_t currentPage = 0; int pagesUntilFullRefresh = 0; - bool updateRequired = false; + const std::function onGoBack; const std::function onGoHome; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); void renderPage(); void saveProgress() const; void loadProgress(); @@ -41,4 +34,5 @@ class XtcReaderActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/reader/XtcReaderChapterSelectionActivity.cpp b/src/activities/reader/XtcReaderChapterSelectionActivity.cpp index 378924f0..5951cabb 100644 --- a/src/activities/reader/XtcReaderChapterSelectionActivity.cpp +++ b/src/activities/reader/XtcReaderChapterSelectionActivity.cpp @@ -37,11 +37,6 @@ int XtcReaderChapterSelectionActivity::findChapterIndexForPage(uint32_t page) co return 0; } -void XtcReaderChapterSelectionActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void XtcReaderChapterSelectionActivity::onEnter() { Activity::onEnter(); @@ -49,29 +44,12 @@ void XtcReaderChapterSelectionActivity::onEnter() { return; } - renderingMutex = xSemaphoreCreateMutex(); selectorIndex = findChapterIndexForPage(currentPage); - updateRequired = true; - xTaskCreate(&XtcReaderChapterSelectionActivity::taskTrampoline, "XtcReaderChapterSelectionActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void XtcReaderChapterSelectionActivity::onExit() { - Activity::onExit(); - - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} +void XtcReaderChapterSelectionActivity::onExit() { Activity::onExit(); } void XtcReaderChapterSelectionActivity::loop() { const int pageItems = getPageItems(); @@ -88,38 +66,26 @@ void XtcReaderChapterSelectionActivity::loop() { buttonNavigator.onNextRelease([this, totalItems] { selectorIndex = ButtonNavigator::nextIndex(selectorIndex, totalItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this, totalItems] { selectorIndex = ButtonNavigator::previousIndex(selectorIndex, totalItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this, totalItems, pageItems] { selectorIndex = ButtonNavigator::nextPageIndex(selectorIndex, totalItems, pageItems); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this, totalItems, pageItems] { selectorIndex = ButtonNavigator::previousPageIndex(selectorIndex, totalItems, pageItems); - updateRequired = true; + requestUpdate(); }); } -void XtcReaderChapterSelectionActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - renderScreen(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void XtcReaderChapterSelectionActivity::renderScreen() { +void XtcReaderChapterSelectionActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/reader/XtcReaderChapterSelectionActivity.h b/src/activities/reader/XtcReaderChapterSelectionActivity.h index c4de4f0b..b45217a1 100644 --- a/src/activities/reader/XtcReaderChapterSelectionActivity.h +++ b/src/activities/reader/XtcReaderChapterSelectionActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include @@ -11,22 +8,16 @@ class XtcReaderChapterSelectionActivity final : public Activity { std::shared_ptr xtc; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; uint32_t currentPage = 0; int selectorIndex = 0; - bool updateRequired = false; + const std::function onGoBack; const std::function onSelectPage; int getPageItems() const; int findChapterIndexForPage(uint32_t page) const; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void renderScreen(); - public: explicit XtcReaderChapterSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::shared_ptr& xtc, uint32_t currentPage, @@ -40,4 +31,5 @@ class XtcReaderChapterSelectionActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; }; diff --git a/src/activities/settings/ButtonRemapActivity.cpp b/src/activities/settings/ButtonRemapActivity.cpp index 43184735..a3c5d592 100644 --- a/src/activities/settings/ButtonRemapActivity.cpp +++ b/src/activities/settings/ButtonRemapActivity.cpp @@ -16,15 +16,9 @@ constexpr uint8_t kUnassigned = 0xFF; constexpr unsigned long kErrorDisplayMs = 1500; } // namespace -void ButtonRemapActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void ButtonRemapActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); // Start with all roles unassigned to avoid duplicate blocking. currentStep = 0; tempMapping[0] = kUnassigned; @@ -33,25 +27,20 @@ void ButtonRemapActivity::onEnter() { tempMapping[3] = kUnassigned; errorMessage.clear(); errorUntil = 0; - updateRequired = true; - - xTaskCreate(&ButtonRemapActivity::taskTrampoline, "ButtonRemapTask", 4096, this, 1, &displayTaskHandle); + requestUpdate(); } -void ButtonRemapActivity::onExit() { - Activity::onExit(); - - // Ensure display task is stopped outside of active rendering. - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} +void ButtonRemapActivity::onExit() { Activity::onExit(); } void ButtonRemapActivity::loop() { + // Clear any temporary warning after its timeout. + if (errorUntil > 0 && millis() > errorUntil) { + errorMessage.clear(); + errorUntil = 0; + requestUpdate(); + return; + } + // Side buttons: // - Up: reset mapping to defaults and exit. // - Down: cancel without saving. @@ -72,60 +61,39 @@ void ButtonRemapActivity::loop() { return; } - // Wait for the UI to refresh before accepting another assignment. - // This avoids rapid double-presses that can advance the step without a visible redraw. - if (updateRequired) { - return; - } + { + // Wait for the UI to refresh before accepting another assignment. + // This avoids rapid double-presses that can advance the step without a visible redraw. + requestUpdateAndWait(); - // 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(pressedButton))) { - updateRequired = true; - return; - } - tempMapping[currentStep] = static_cast(pressedButton); - currentStep++; - - if (currentStep >= kRoleCount) { - // All roles assigned; save to settings and exit. - applyTempMapping(); - SETTINGS.saveToFile(); - onBack(); - return; - } - - updateRequired = true; -} - -[[noreturn]] void ButtonRemapActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - // Ensure render calls are serialized with UI thread changes. - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - updateRequired = false; - xSemaphoreGive(renderingMutex); + // Wait for a front button press to assign to the current role. + const int pressedButton = mappedInput.getPressedFrontButton(); + if (pressedButton < 0) { + return; } - // Clear any temporary warning after its timeout. - if (errorUntil > 0 && millis() > errorUntil) { - errorMessage.clear(); - errorUntil = 0; - updateRequired = true; + // 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(pressedButton))) { + requestUpdate(); + return; + } + tempMapping[currentStep] = static_cast(pressedButton); + currentStep++; + + if (currentStep >= kRoleCount) { + // All roles assigned; save to settings and exit. + applyTempMapping(); + SETTINGS.saveToFile(); + onBack(); + return; } - vTaskDelay(50 / portTICK_PERIOD_MS); + requestUpdate(); } } -void ButtonRemapActivity::render() { +void ButtonRemapActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/settings/ButtonRemapActivity.h b/src/activities/settings/ButtonRemapActivity.h index f87a66ea..5d5ba1a9 100644 --- a/src/activities/settings/ButtonRemapActivity.h +++ b/src/activities/settings/ButtonRemapActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -17,12 +14,10 @@ class ButtonRemapActivity final : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: // Rendering task state. - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; // Callback used to exit the remap flow back to the settings list. const std::function onBack; @@ -34,11 +29,6 @@ class ButtonRemapActivity final : public Activity { unsigned long errorUntil = 0; std::string errorMessage; - // FreeRTOS task helpers. - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); - // Commit temporary mapping to settings. void applyTempMapping(); // Returns false if a hardware button is already assigned to a different role. diff --git a/src/activities/settings/CalibreSettingsActivity.cpp b/src/activities/settings/CalibreSettingsActivity.cpp index 7b7a0ed4..b89fa326 100644 --- a/src/activities/settings/CalibreSettingsActivity.cpp +++ b/src/activities/settings/CalibreSettingsActivity.cpp @@ -15,37 +15,14 @@ constexpr int MENU_ITEMS = 3; const char* menuNames[MENU_ITEMS] = {"OPDS Server URL", "Username", "Password"}; } // namespace -void CalibreSettingsActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void CalibreSettingsActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); selectedIndex = 0; - updateRequired = true; - - xTaskCreate(&CalibreSettingsActivity::taskTrampoline, "CalibreSettingsTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void CalibreSettingsActivity::onExit() { - ActivityWithSubactivity::onExit(); - - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} +void CalibreSettingsActivity::onExit() { ActivityWithSubactivity::onExit(); } void CalibreSettingsActivity::loop() { if (subActivity) { @@ -66,18 +43,16 @@ void CalibreSettingsActivity::loop() { // Handle navigation buttonNavigator.onNext([this] { selectedIndex = (selectedIndex + 1) % MENU_ITEMS; - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this] { selectedIndex = (selectedIndex + MENU_ITEMS - 1) % MENU_ITEMS; - updateRequired = true; + requestUpdate(); }); } void CalibreSettingsActivity::handleSelection() { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (selectedIndex == 0) { // OPDS Server URL exitActivity(); @@ -90,11 +65,11 @@ void CalibreSettingsActivity::handleSelection() { SETTINGS.opdsServerUrl[sizeof(SETTINGS.opdsServerUrl) - 1] = '\0'; SETTINGS.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } else if (selectedIndex == 1) { // Username @@ -108,11 +83,11 @@ void CalibreSettingsActivity::handleSelection() { SETTINGS.opdsUsername[sizeof(SETTINGS.opdsUsername) - 1] = '\0'; SETTINGS.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } else if (selectedIndex == 2) { // Password @@ -126,30 +101,16 @@ void CalibreSettingsActivity::handleSelection() { SETTINGS.opdsPassword[sizeof(SETTINGS.opdsPassword) - 1] = '\0'; SETTINGS.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } - - xSemaphoreGive(renderingMutex); } -void CalibreSettingsActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void CalibreSettingsActivity::render() { +void CalibreSettingsActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/settings/CalibreSettingsActivity.h b/src/activities/settings/CalibreSettingsActivity.h index 53de46bc..7d940732 100644 --- a/src/activities/settings/CalibreSettingsActivity.h +++ b/src/activities/settings/CalibreSettingsActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include @@ -21,18 +18,12 @@ class CalibreSettingsActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; - bool updateRequired = false; int selectedIndex = 0; const std::function onBack; - - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); void handleSelection(); }; diff --git a/src/activities/settings/ClearCacheActivity.cpp b/src/activities/settings/ClearCacheActivity.cpp index 9da9444b..c3dbfbb6 100644 --- a/src/activities/settings/ClearCacheActivity.cpp +++ b/src/activities/settings/ClearCacheActivity.cpp @@ -8,52 +8,16 @@ #include "components/UITheme.h" #include "fontIds.h" -void ClearCacheActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void ClearCacheActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); state = WARNING; - updateRequired = true; - - xTaskCreate(&ClearCacheActivity::taskTrampoline, "ClearCacheActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void ClearCacheActivity::onExit() { - ActivityWithSubactivity::onExit(); +void ClearCacheActivity::onExit() { ActivityWithSubactivity::onExit(); } - // 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; -} - -void ClearCacheActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void ClearCacheActivity::render() { +void ClearCacheActivity::render(Activity::RenderLock&&) { const auto pageHeight = renderer.getScreenHeight(); renderer.clearScreen(); @@ -112,7 +76,7 @@ void ClearCacheActivity::clearCache() { LOG_DBG("CLEAR_CACHE", "Failed to open cache directory"); if (root) root.close(); state = FAILED; - updateRequired = true; + requestUpdate(); return; } @@ -147,7 +111,7 @@ void ClearCacheActivity::clearCache() { LOG_DBG("CLEAR_CACHE", "Cache cleared: %d removed, %d failed", clearedCount, failedCount); state = SUCCESS; - updateRequired = true; + requestUpdate(); } void ClearCacheActivity::loop() { @@ -157,8 +121,7 @@ void ClearCacheActivity::loop() { xSemaphoreTake(renderingMutex, portMAX_DELAY); state = CLEARING; xSemaphoreGive(renderingMutex); - updateRequired = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + requestUpdateAndWait(); clearCache(); } diff --git a/src/activities/settings/ClearCacheActivity.h b/src/activities/settings/ClearCacheActivity.h index 31795a95..8bfbe1f3 100644 --- a/src/activities/settings/ClearCacheActivity.h +++ b/src/activities/settings/ClearCacheActivity.h @@ -1,9 +1,5 @@ #pragma once -#include -#include -#include - #include #include "activities/ActivityWithSubactivity.h" @@ -17,21 +13,16 @@ class ClearCacheActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: enum State { WARNING, CLEARING, SUCCESS, FAILED }; State state = WARNING; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; + const std::function goBack; int clearedCount = 0; int failedCount = 0; - - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); void clearCache(); }; diff --git a/src/activities/settings/KOReaderAuthActivity.cpp b/src/activities/settings/KOReaderAuthActivity.cpp index 78d6ec84..fcbba5db 100644 --- a/src/activities/settings/KOReaderAuthActivity.cpp +++ b/src/activities/settings/KOReaderAuthActivity.cpp @@ -10,11 +10,6 @@ #include "components/UITheme.h" #include "fontIds.h" -void KOReaderAuthActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void KOReaderAuthActivity::onWifiSelectionComplete(const bool success) { exitActivity(); @@ -23,7 +18,7 @@ void KOReaderAuthActivity::onWifiSelectionComplete(const bool success) { state = FAILED; errorMessage = "WiFi connection failed"; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } @@ -31,7 +26,7 @@ void KOReaderAuthActivity::onWifiSelectionComplete(const bool success) { state = AUTHENTICATING; statusMessage = "Authenticating..."; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); performAuthentication(); } @@ -48,21 +43,12 @@ void KOReaderAuthActivity::performAuthentication() { errorMessage = KOReaderSyncClient::errorString(result); } xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); } void KOReaderAuthActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - - xTaskCreate(&KOReaderAuthActivity::taskTrampoline, "KOAuthTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); - // Turn on WiFi WiFi.mode(WIFI_STA); @@ -70,7 +56,7 @@ void KOReaderAuthActivity::onEnter() { if (WiFi.status() == WL_CONNECTED) { state = AUTHENTICATING; statusMessage = "Authenticating..."; - updateRequired = true; + requestUpdate(); // Perform authentication in a separate task xTaskCreate( @@ -96,33 +82,9 @@ void KOReaderAuthActivity::onExit() { delay(100); WiFi.mode(WIFI_OFF); delay(100); - - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; } -void KOReaderAuthActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void KOReaderAuthActivity::render() { - if (subActivity) { - return; - } - +void KOReaderAuthActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); renderer.drawCenteredText(UI_12_FONT_ID, 15, "KOReader Auth", true, EpdFontFamily::BOLD); diff --git a/src/activities/settings/KOReaderAuthActivity.h b/src/activities/settings/KOReaderAuthActivity.h index a6ed0d3e..4b1bbe6d 100644 --- a/src/activities/settings/KOReaderAuthActivity.h +++ b/src/activities/settings/KOReaderAuthActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include @@ -20,15 +17,12 @@ class KOReaderAuthActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; bool preventAutoSleep() override { return state == CONNECTING || state == AUTHENTICATING; } private: enum State { WIFI_SELECTION, CONNECTING, AUTHENTICATING, SUCCESS, FAILED }; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; - State state = WIFI_SELECTION; std::string statusMessage; std::string errorMessage; @@ -37,8 +31,4 @@ class KOReaderAuthActivity final : public ActivityWithSubactivity { void onWifiSelectionComplete(bool success); void performAuthentication(); - - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); }; diff --git a/src/activities/settings/KOReaderSettingsActivity.cpp b/src/activities/settings/KOReaderSettingsActivity.cpp index a72151d6..4196ef90 100644 --- a/src/activities/settings/KOReaderSettingsActivity.cpp +++ b/src/activities/settings/KOReaderSettingsActivity.cpp @@ -16,37 +16,14 @@ constexpr int MENU_ITEMS = 5; const char* menuNames[MENU_ITEMS] = {"Username", "Password", "Sync Server URL", "Document Matching", "Authenticate"}; } // namespace -void KOReaderSettingsActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void KOReaderSettingsActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); selectedIndex = 0; - updateRequired = true; - - xTaskCreate(&KOReaderSettingsActivity::taskTrampoline, "KOReaderSettingsTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void KOReaderSettingsActivity::onExit() { - ActivityWithSubactivity::onExit(); - - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (displayTaskHandle) { - vTaskDelete(displayTaskHandle); - displayTaskHandle = nullptr; - } - vSemaphoreDelete(renderingMutex); - renderingMutex = nullptr; -} +void KOReaderSettingsActivity::onExit() { ActivityWithSubactivity::onExit(); } void KOReaderSettingsActivity::loop() { if (subActivity) { @@ -67,18 +44,16 @@ void KOReaderSettingsActivity::loop() { // Handle navigation buttonNavigator.onNext([this] { selectedIndex = (selectedIndex + 1) % MENU_ITEMS; - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPrevious([this] { selectedIndex = (selectedIndex + MENU_ITEMS - 1) % MENU_ITEMS; - updateRequired = true; + requestUpdate(); }); } void KOReaderSettingsActivity::handleSelection() { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - if (selectedIndex == 0) { // Username exitActivity(); @@ -90,11 +65,11 @@ void KOReaderSettingsActivity::handleSelection() { KOREADER_STORE.setCredentials(username, KOREADER_STORE.getPassword()); KOREADER_STORE.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } else if (selectedIndex == 1) { // Password @@ -107,11 +82,11 @@ void KOReaderSettingsActivity::handleSelection() { KOREADER_STORE.setCredentials(KOREADER_STORE.getUsername(), password); KOREADER_STORE.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } else if (selectedIndex == 2) { // Sync Server URL - prefill with https:// if empty to save typing @@ -128,11 +103,11 @@ void KOReaderSettingsActivity::handleSelection() { KOREADER_STORE.setServerUrl(urlToSave); KOREADER_STORE.saveToFile(); exitActivity(); - updateRequired = true; + requestUpdate(); }, [this]() { exitActivity(); - updateRequired = true; + requestUpdate(); })); } else if (selectedIndex == 3) { // Document Matching - toggle between Filename and Binary @@ -141,7 +116,7 @@ void KOReaderSettingsActivity::handleSelection() { (current == DocumentMatchMethod::FILENAME) ? DocumentMatchMethod::BINARY : DocumentMatchMethod::FILENAME; KOREADER_STORE.setMatchMethod(newMethod); KOREADER_STORE.saveToFile(); - updateRequired = true; + requestUpdate(); } else if (selectedIndex == 4) { // Authenticate if (!KOREADER_STORE.hasCredentials()) { @@ -152,26 +127,12 @@ void KOReaderSettingsActivity::handleSelection() { exitActivity(); enterNewActivity(new KOReaderAuthActivity(renderer, mappedInput, [this] { exitActivity(); - updateRequired = true; + requestUpdate(); })); } - - xSemaphoreGive(renderingMutex); } -void KOReaderSettingsActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void KOReaderSettingsActivity::render() { +void KOReaderSettingsActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/settings/KOReaderSettingsActivity.h b/src/activities/settings/KOReaderSettingsActivity.h index 24f2f820..0534eabb 100644 --- a/src/activities/settings/KOReaderSettingsActivity.h +++ b/src/activities/settings/KOReaderSettingsActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include @@ -21,18 +18,13 @@ class KOReaderSettingsActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; - bool updateRequired = false; int selectedIndex = 0; const std::function onBack; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); void handleSelection(); }; diff --git a/src/activities/settings/OtaUpdateActivity.cpp b/src/activities/settings/OtaUpdateActivity.cpp index e469efce..7e50fa4c 100644 --- a/src/activities/settings/OtaUpdateActivity.cpp +++ b/src/activities/settings/OtaUpdateActivity.cpp @@ -9,11 +9,6 @@ #include "fontIds.h" #include "network/OtaUpdater.h" -void OtaUpdateActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void OtaUpdateActivity::onWifiSelectionComplete(const bool success) { exitActivity(); @@ -28,15 +23,15 @@ void OtaUpdateActivity::onWifiSelectionComplete(const bool success) { xSemaphoreTake(renderingMutex, portMAX_DELAY); state = CHECKING_FOR_UPDATE; xSemaphoreGive(renderingMutex); - updateRequired = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + requestUpdateAndWait(); + const auto res = updater.checkForUpdate(); if (res != OtaUpdater::OK) { LOG_DBG("OTA", "Update check failed: %d", res); xSemaphoreTake(renderingMutex, portMAX_DELAY); state = FAILED; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } @@ -45,28 +40,19 @@ void OtaUpdateActivity::onWifiSelectionComplete(const bool success) { xSemaphoreTake(renderingMutex, portMAX_DELAY); state = NO_UPDATE; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); return; } xSemaphoreTake(renderingMutex, portMAX_DELAY); state = WAITING_CONFIRMATION; xSemaphoreGive(renderingMutex); - updateRequired = true; + requestUpdate(); } void OtaUpdateActivity::onEnter() { ActivityWithSubactivity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - - xTaskCreate(&OtaUpdateActivity::taskTrampoline, "OtaUpdateActivityTask", - 2048, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); - // Turn on WiFi immediately LOG_DBG("OTA", "Turning on WiFi..."); WiFi.mode(WIFI_STA); @@ -85,30 +71,9 @@ void OtaUpdateActivity::onExit() { delay(100); // Allow disconnect frame to be sent WiFi.mode(WIFI_OFF); delay(100); // Allow WiFi hardware to fully power down - - // 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; } -void OtaUpdateActivity::displayTaskLoop() { - while (true) { - if (updateRequired || updater.getRender()) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void OtaUpdateActivity::render() { +void OtaUpdateActivity::render(Activity::RenderLock&&) { if (subActivity) { // Subactivity handles its own rendering return; @@ -182,6 +147,11 @@ void OtaUpdateActivity::render() { } void OtaUpdateActivity::loop() { + // TODO @ngxson : refactor this logic later + if (updater.getRender()) { + requestUpdate(); + } + if (subActivity) { subActivity->loop(); return; @@ -190,26 +160,29 @@ void OtaUpdateActivity::loop() { if (state == WAITING_CONFIRMATION) { if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { LOG_DBG("OTA", "New update available, starting download..."); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - state = UPDATE_IN_PROGRESS; - xSemaphoreGive(renderingMutex); - updateRequired = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + { + RenderLock lock(*this); + state = UPDATE_IN_PROGRESS; + } + requestUpdate(); + requestUpdateAndWait(); const auto res = updater.installUpdate(); if (res != OtaUpdater::OK) { LOG_DBG("OTA", "Update failed: %d", res); - xSemaphoreTake(renderingMutex, portMAX_DELAY); - state = FAILED; - xSemaphoreGive(renderingMutex); - updateRequired = true; + { + RenderLock lock(*this); + state = FAILED; + } + requestUpdate(); return; } - xSemaphoreTake(renderingMutex, portMAX_DELAY); - state = FINISHED; - xSemaphoreGive(renderingMutex); - updateRequired = true; + { + RenderLock lock(*this); + state = FINISHED; + } + requestUpdate(); } if (mappedInput.wasPressed(MappedInputManager::Button::Back)) { diff --git a/src/activities/settings/OtaUpdateActivity.h b/src/activities/settings/OtaUpdateActivity.h index e049b077..7978acf4 100644 --- a/src/activities/settings/OtaUpdateActivity.h +++ b/src/activities/settings/OtaUpdateActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include "activities/ActivityWithSubactivity.h" #include "network/OtaUpdater.h" @@ -21,18 +18,12 @@ class OtaUpdateActivity : public ActivityWithSubactivity { // Can't initialize this to 0 or the first render doesn't happen static constexpr unsigned int UNINITIALIZED_PERCENTAGE = 111; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; - bool updateRequired = false; const std::function goBack; State state = WIFI_SELECTION; unsigned int lastUpdaterPercentage = UNINITIALIZED_PERCENTAGE; OtaUpdater updater; void onWifiSelectionComplete(bool success); - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render(); public: explicit OtaUpdateActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, @@ -41,5 +32,6 @@ class OtaUpdateActivity : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; bool preventAutoSleep() override { return state == CHECKING_FOR_UPDATE || state == UPDATE_IN_PROGRESS; } }; diff --git a/src/activities/settings/SettingsActivity.cpp b/src/activities/settings/SettingsActivity.cpp index c8d95802..b6907bca 100644 --- a/src/activities/settings/SettingsActivity.cpp +++ b/src/activities/settings/SettingsActivity.cpp @@ -17,14 +17,8 @@ const char* SettingsActivity::categoryNames[categoryCount] = {"Display", "Reader", "Controls", "System"}; -void SettingsActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - void SettingsActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); // Build per-category vectors from the shared settings list displaySettings.clear(); @@ -64,28 +58,12 @@ void SettingsActivity::onEnter() { settingsCount = static_cast(displaySettings.size()); // Trigger first update - updateRequired = true; - - xTaskCreate(&SettingsActivity::taskTrampoline, "SettingsActivityTask", - 4096, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } void SettingsActivity::onExit() { ActivityWithSubactivity::onExit(); - // 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; - UITheme::getInstance().reload(); // Re-apply theme in case it was changed } @@ -101,10 +79,10 @@ void SettingsActivity::loop() { if (selectedSettingIndex == 0) { selectedCategoryIndex = (selectedCategoryIndex < categoryCount - 1) ? (selectedCategoryIndex + 1) : 0; hasChangedCategory = true; - updateRequired = true; + requestUpdate(); } else { toggleCurrentSetting(); - updateRequired = true; + requestUpdate(); return; } } @@ -118,24 +96,24 @@ void SettingsActivity::loop() { // Handle navigation buttonNavigator.onNextRelease([this] { selectedSettingIndex = ButtonNavigator::nextIndex(selectedSettingIndex, settingsCount + 1); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousRelease([this] { selectedSettingIndex = ButtonNavigator::previousIndex(selectedSettingIndex, settingsCount + 1); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onNextContinuous([this, &hasChangedCategory] { hasChangedCategory = true; selectedCategoryIndex = ButtonNavigator::nextIndex(selectedCategoryIndex, categoryCount); - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPreviousContinuous([this, &hasChangedCategory] { hasChangedCategory = true; selectedCategoryIndex = ButtonNavigator::previousIndex(selectedCategoryIndex, categoryCount); - updateRequired = true; + requestUpdate(); }); if (hasChangedCategory) { @@ -185,20 +163,18 @@ void SettingsActivity::toggleCurrentSetting() { } } else if (setting.type == SettingType::ACTION) { auto enterSubActivity = [this](Activity* activity) { - xSemaphoreTake(renderingMutex, portMAX_DELAY); exitActivity(); enterNewActivity(activity); - xSemaphoreGive(renderingMutex); }; auto onComplete = [this] { exitActivity(); - updateRequired = true; + requestUpdate(); }; auto onCompleteBool = [this](bool) { exitActivity(); - updateRequired = true; + requestUpdate(); }; switch (setting.action) { @@ -231,19 +207,7 @@ void SettingsActivity::toggleCurrentSetting() { SETTINGS.saveToFile(); } -void SettingsActivity::displayTaskLoop() { - while (true) { - if (updateRequired && !subActivity) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - -void SettingsActivity::render() const { +void SettingsActivity::render(Activity::RenderLock&&) { renderer.clearScreen(); const auto pageWidth = renderer.getScreenWidth(); diff --git a/src/activities/settings/SettingsActivity.h b/src/activities/settings/SettingsActivity.h index 1417c17d..1fdbcc61 100644 --- a/src/activities/settings/SettingsActivity.h +++ b/src/activities/settings/SettingsActivity.h @@ -1,7 +1,4 @@ #pragma once -#include -#include -#include #include #include @@ -135,10 +132,8 @@ struct SettingInfo { }; class SettingsActivity final : public ActivityWithSubactivity { - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; ButtonNavigator buttonNavigator; - bool updateRequired = false; + int selectedCategoryIndex = 0; // Currently selected category int selectedSettingIndex = 0; int settingsCount = 0; @@ -155,9 +150,6 @@ class SettingsActivity final : public ActivityWithSubactivity { static constexpr int categoryCount = 4; static const char* categoryNames[categoryCount]; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); - void render() const; void enterCategory(int categoryIndex); void toggleCurrentSetting(); @@ -168,4 +160,5 @@ class SettingsActivity final : public ActivityWithSubactivity { void onEnter() override; void onExit() override; void loop() override; -}; \ No newline at end of file + void render(Activity::RenderLock&&) override; +}; diff --git a/src/activities/util/KeyboardEntryActivity.cpp b/src/activities/util/KeyboardEntryActivity.cpp index 40f2eaa6..54b025c5 100644 --- a/src/activities/util/KeyboardEntryActivity.cpp +++ b/src/activities/util/KeyboardEntryActivity.cpp @@ -17,51 +17,14 @@ const char* const KeyboardEntryActivity::keyboardShift[NUM_ROWS] = {"~!@#$%^&*() // Shift state strings const char* const KeyboardEntryActivity::shiftString[3] = {"shift", "SHIFT", "LOCK"}; -void KeyboardEntryActivity::taskTrampoline(void* param) { - auto* self = static_cast(param); - self->displayTaskLoop(); -} - -void KeyboardEntryActivity::displayTaskLoop() { - while (true) { - if (updateRequired) { - updateRequired = false; - xSemaphoreTake(renderingMutex, portMAX_DELAY); - render(); - xSemaphoreGive(renderingMutex); - } - vTaskDelay(10 / portTICK_PERIOD_MS); - } -} - void KeyboardEntryActivity::onEnter() { Activity::onEnter(); - renderingMutex = xSemaphoreCreateMutex(); - // Trigger first update - updateRequired = true; - - xTaskCreate(&KeyboardEntryActivity::taskTrampoline, "KeyboardEntryActivity", - 2048, // Stack size - this, // Parameters - 1, // Priority - &displayTaskHandle // Task handle - ); + requestUpdate(); } -void KeyboardEntryActivity::onExit() { - Activity::onExit(); - - // 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; -} +void KeyboardEntryActivity::onExit() { Activity::onExit(); } int KeyboardEntryActivity::getRowLength(const int row) const { if (row < 0 || row >= NUM_ROWS) return 0; @@ -148,7 +111,7 @@ void KeyboardEntryActivity::loop() { const int maxCol = getRowLength(selectedRow) - 1; if (selectedCol > maxCol) selectedCol = maxCol; - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPressAndContinuous({MappedInputManager::Button::Down}, [this] { @@ -156,7 +119,7 @@ void KeyboardEntryActivity::loop() { const int maxCol = getRowLength(selectedRow) - 1; if (selectedCol > maxCol) selectedCol = maxCol; - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPressAndContinuous({MappedInputManager::Button::Left}, [this] { @@ -182,7 +145,7 @@ void KeyboardEntryActivity::loop() { selectedCol = ButtonNavigator::previousIndex(selectedCol, maxCol + 1); } - updateRequired = true; + requestUpdate(); }); buttonNavigator.onPressAndContinuous({MappedInputManager::Button::Right}, [this] { @@ -207,13 +170,13 @@ void KeyboardEntryActivity::loop() { } else { selectedCol = ButtonNavigator::nextIndex(selectedCol, maxCol + 1); } - updateRequired = true; + requestUpdate(); }); // Selection if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { handleKeyPress(); - updateRequired = true; + requestUpdate(); } // Cancel @@ -221,11 +184,11 @@ void KeyboardEntryActivity::loop() { if (onCancel) { onCancel(); } - updateRequired = true; + requestUpdate(); } } -void KeyboardEntryActivity::render() const { +void KeyboardEntryActivity::render(Activity::RenderLock&&) { const auto pageWidth = renderer.getScreenWidth(); renderer.clearScreen(); diff --git a/src/activities/util/KeyboardEntryActivity.h b/src/activities/util/KeyboardEntryActivity.h index 8e94fd3c..04c9a157 100644 --- a/src/activities/util/KeyboardEntryActivity.h +++ b/src/activities/util/KeyboardEntryActivity.h @@ -1,8 +1,5 @@ #pragma once #include -#include -#include -#include #include #include @@ -57,6 +54,7 @@ class KeyboardEntryActivity : public Activity { void onEnter() override; void onExit() override; void loop() override; + void render(Activity::RenderLock&&) override; private: std::string title; @@ -64,10 +62,8 @@ class KeyboardEntryActivity : public Activity { std::string text; size_t maxLength; bool isPassword; - TaskHandle_t displayTaskHandle = nullptr; - SemaphoreHandle_t renderingMutex = nullptr; + ButtonNavigator buttonNavigator; - bool updateRequired = false; // Keyboard state int selectedRow = 0; @@ -92,11 +88,8 @@ class KeyboardEntryActivity : public Activity { static constexpr int BACKSPACE_COL = 7; static constexpr int DONE_COL = 9; - static void taskTrampoline(void* param); - [[noreturn]] void displayTaskLoop(); char getSelectedChar() const; void handleKeyPress(); int getRowLength(int row) const; - void render() const; void renderItemWithSelector(int x, int y, const char* item, bool isSelected) const; }; diff --git a/src/network/OtaUpdater.cpp b/src/network/OtaUpdater.cpp index 80138e6a..c5d405c0 100644 --- a/src/network/OtaUpdater.cpp +++ b/src/network/OtaUpdater.cpp @@ -243,7 +243,7 @@ OtaUpdater::OtaUpdaterError OtaUpdater::installUpdate() { processedSize = esp_https_ota_get_image_len_read(ota_handle); /* Sent signal to OtaUpdateActivity */ render = true; - vTaskDelay(10 / portTICK_PERIOD_MS); + delay(100); // TODO: should we replace this with something better? } while (esp_err == ESP_ERR_HTTPS_OTA_IN_PROGRESS); /* Return back to default power saving for WiFi in case of failing */