fix: properly implement requestUpdateAndWait() (#1218)
## Summary Properly implement `requestUpdateAndWait()` using freeRTOS direct task notification. FWIW, I think most of the current use cases of `requestUpdateAndWait()` are redundant, better to be replaced by `requestUpdate(true)`. But just keeping them in case we can find a proper use case for it in the future. --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? **YES**, it's trivial, so I asked an AI to write the code --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
@@ -8,11 +8,7 @@ void Activity::onExit() { LOG_DBG("ACT", "Exiting activity: %s", name.c_str());
|
|||||||
|
|
||||||
void Activity::requestUpdate(bool immediate) { activityManager.requestUpdate(immediate); }
|
void Activity::requestUpdate(bool immediate) { activityManager.requestUpdate(immediate); }
|
||||||
|
|
||||||
void Activity::requestUpdateAndWait() {
|
void Activity::requestUpdateAndWait() { activityManager.requestUpdateAndWait(); }
|
||||||
// FIXME @ngxson : properly implement this using freeRTOS notification
|
|
||||||
activityManager.requestUpdate(true);
|
|
||||||
delay(100);
|
|
||||||
}
|
|
||||||
|
|
||||||
void Activity::onGoHome() { activityManager.goHome(); }
|
void Activity::onGoHome() { activityManager.goHome(); }
|
||||||
|
|
||||||
|
|||||||
@@ -38,6 +38,15 @@ void ActivityManager::renderTaskLoop() {
|
|||||||
HalPowerManager::Lock powerLock; // Ensure we don't go into low-power mode while rendering
|
HalPowerManager::Lock powerLock; // Ensure we don't go into low-power mode while rendering
|
||||||
currentActivity->render(std::move(lock));
|
currentActivity->render(std::move(lock));
|
||||||
}
|
}
|
||||||
|
// Notify any task blocked in requestUpdateAndWait() that the render is done.
|
||||||
|
TaskHandle_t waiter = nullptr;
|
||||||
|
taskENTER_CRITICAL(nullptr);
|
||||||
|
waiter = waitingTaskHandle;
|
||||||
|
waitingTaskHandle = nullptr;
|
||||||
|
taskEXIT_CRITICAL(nullptr);
|
||||||
|
if (waiter) {
|
||||||
|
xTaskNotify(waiter, 1, eIncrement);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -225,6 +234,36 @@ void ActivityManager::requestUpdate(bool immediate) {
|
|||||||
requestedUpdate = true;
|
requestedUpdate = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
void ActivityManager::requestUpdateAndWait() {
|
||||||
|
if (!renderTaskHandle) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Atomic section to perform checks
|
||||||
|
taskENTER_CRITICAL(nullptr);
|
||||||
|
auto currTaskHandler = xTaskGetCurrentTaskHandle();
|
||||||
|
auto mutexHolder = xSemaphoreGetMutexHolder(renderingMutex);
|
||||||
|
bool isRenderTask = (currTaskHandler == renderTaskHandle);
|
||||||
|
bool alreadyWaiting = (waitingTaskHandle != nullptr);
|
||||||
|
bool holdingRenderLock = (mutexHolder == currTaskHandler);
|
||||||
|
if (!alreadyWaiting && !isRenderTask && !holdingRenderLock) {
|
||||||
|
waitingTaskHandle = currTaskHandler;
|
||||||
|
}
|
||||||
|
taskEXIT_CRITICAL(nullptr);
|
||||||
|
|
||||||
|
// Render task cannot call requestUpdateAndWait() or it will cause a deadlock
|
||||||
|
assert(!isRenderTask && "Render task cannot call requestUpdateAndWait()");
|
||||||
|
|
||||||
|
// There should never be the case where 2 tasks are waiting for a render at the same time
|
||||||
|
assert(!alreadyWaiting && "Already waiting for a render to complete");
|
||||||
|
|
||||||
|
// Cannot call while holding RenderLock or it will cause a deadlock
|
||||||
|
assert(!holdingRenderLock && "Cannot call requestUpdateAndWait() while holding RenderLock");
|
||||||
|
|
||||||
|
xTaskNotify(renderTaskHandle, 1, eIncrement);
|
||||||
|
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
|
||||||
|
}
|
||||||
|
|
||||||
// RenderLock
|
// RenderLock
|
||||||
|
|
||||||
RenderLock::RenderLock() {
|
RenderLock::RenderLock() {
|
||||||
|
|||||||
@@ -50,6 +50,10 @@ class ActivityManager {
|
|||||||
static void renderTaskTrampoline(void* param);
|
static void renderTaskTrampoline(void* param);
|
||||||
[[noreturn]] virtual void renderTaskLoop();
|
[[noreturn]] virtual void renderTaskLoop();
|
||||||
|
|
||||||
|
// Set by requestUpdateAndWait(); read and cleared by the render task after render completes.
|
||||||
|
// Note: only one waiting task is supported at a time
|
||||||
|
TaskHandle_t waitingTaskHandle = nullptr;
|
||||||
|
|
||||||
// Mutex to protect rendering operations from race conditions
|
// Mutex to protect rendering operations from race conditions
|
||||||
// Must only be used via RenderLock
|
// Must only be used via RenderLock
|
||||||
SemaphoreHandle_t renderingMutex = nullptr;
|
SemaphoreHandle_t renderingMutex = nullptr;
|
||||||
@@ -98,6 +102,10 @@ class ActivityManager {
|
|||||||
// If immediate is true, the update will be triggered immediately.
|
// If immediate is true, the update will be triggered immediately.
|
||||||
// Otherwise, it will be deferred until the end of the current loop iteration.
|
// Otherwise, it will be deferred until the end of the current loop iteration.
|
||||||
void requestUpdate(bool immediate = false);
|
void requestUpdate(bool immediate = false);
|
||||||
|
|
||||||
|
// Trigger a render and block until it completes.
|
||||||
|
// Must NOT be called from the render task or while holding a RenderLock.
|
||||||
|
void requestUpdateAndWait();
|
||||||
};
|
};
|
||||||
|
|
||||||
extern ActivityManager activityManager; // singleton, to be defined in main.cpp
|
extern ActivityManager activityManager; // singleton, to be defined in main.cpp
|
||||||
|
|||||||
@@ -201,6 +201,7 @@ void WifiSelectionActivity::selectNetwork(const int index) {
|
|||||||
state = WifiSelectionState::NETWORK_LIST;
|
state = WifiSelectionState::NETWORK_LIST;
|
||||||
} else {
|
} else {
|
||||||
enteredPassword = std::get<KeyboardResult>(result.data).text;
|
enteredPassword = std::get<KeyboardResult>(result.data).text;
|
||||||
|
// state will be updated in next loop iteration
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
@@ -465,7 +466,6 @@ void WifiSelectionActivity::render(RenderLock&&) {
|
|||||||
// Don't render if we're in PASSWORD_ENTRY state - we're just transitioning
|
// Don't render if we're in PASSWORD_ENTRY state - we're just transitioning
|
||||||
// from the keyboard subactivity back to the main activity
|
// from the keyboard subactivity back to the main activity
|
||||||
if (state == WifiSelectionState::PASSWORD_ENTRY) {
|
if (state == WifiSelectionState::PASSWORD_ENTRY) {
|
||||||
requestUpdateAndWait();
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user