fix: use RAII render lock everywhere (#916)
## Summary Follow-up to https://github.com/crosspoint-reader/crosspoint-reader/pull/774 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? **NO** <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Refactor** * Modernized internal synchronization mechanisms across multiple components to improve code reliability and maintainability. All functionality remains unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -244,9 +244,10 @@ void WifiSelectionActivity::checkConnectionStatus() {
|
||||
|
||||
// Save this as the last connected network - SD card operations need lock as
|
||||
// we use SPI for both
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
WIFI_STORE.setLastConnectedSsid(selectedSSID);
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
|
||||
// If we entered a new password, ask if user wants to save it
|
||||
// Otherwise, immediately complete so parent can start web server
|
||||
|
||||
@@ -221,11 +221,12 @@ void EpubReaderActivity::loop() {
|
||||
|
||||
if (skipChapter) {
|
||||
// We don't want to delete the section mid-render, so grab the semaphore
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
nextPageNumber = 0;
|
||||
currentSpineIndex = nextTriggered ? currentSpineIndex + 1 : currentSpineIndex - 1;
|
||||
section.reset();
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
@@ -241,11 +242,12 @@ void EpubReaderActivity::loop() {
|
||||
section->currentPage--;
|
||||
} else {
|
||||
// We don't want to delete the section mid-render, so grab the semaphore
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
nextPageNumber = UINT16_MAX;
|
||||
currentSpineIndex--;
|
||||
section.reset();
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
}
|
||||
requestUpdate();
|
||||
} else {
|
||||
@@ -253,11 +255,12 @@ void EpubReaderActivity::loop() {
|
||||
section->currentPage++;
|
||||
} else {
|
||||
// We don't want to delete the section mid-render, so grab the semaphore
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
nextPageNumber = 0;
|
||||
currentSpineIndex++;
|
||||
section.reset();
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
@@ -325,12 +328,13 @@ void EpubReaderActivity::jumpToPercent(int percent) {
|
||||
}
|
||||
|
||||
// Reset state so render() reloads and repositions on the target spine.
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
currentSpineIndex = targetSpineIndex;
|
||||
nextPageNumber = 0;
|
||||
pendingPercentJump = true;
|
||||
section.reset();
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
}
|
||||
|
||||
void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction action) {
|
||||
@@ -403,7 +407,8 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
|
||||
break;
|
||||
}
|
||||
case EpubReaderMenuActivity::MenuAction::DELETE_CACHE: {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
if (epub) {
|
||||
// 2. BACKUP: Read current progress
|
||||
// We use the current variables that track our position
|
||||
@@ -420,7 +425,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
|
||||
|
||||
saveProgress(backupSpine, backupPage, backupPageCount);
|
||||
}
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
// Defer go home to avoid race condition with display task
|
||||
pendingGoHome = true;
|
||||
break;
|
||||
@@ -458,7 +463,8 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) {
|
||||
}
|
||||
|
||||
// Preserve current reading position so we can restore after reflow.
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
if (section) {
|
||||
cachedSpineIndex = currentSpineIndex;
|
||||
cachedChapterTotalPageCount = section->pageCount;
|
||||
@@ -474,7 +480,7 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) {
|
||||
|
||||
// Reset section to force re-layout in the new orientation.
|
||||
section.reset();
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Failure handling
|
||||
|
||||
@@ -51,18 +51,20 @@ void KOReaderSyncActivity::onWifiSelectionComplete(const bool success) {
|
||||
|
||||
LOG_DBG("KOSync", "WiFi connected, starting sync");
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SYNCING;
|
||||
statusMessage = "Syncing time...";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
|
||||
// Sync time with NTP before making API requests
|
||||
syncTimeWithNTP();
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
statusMessage = "Calculating document hash...";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
|
||||
performSync();
|
||||
@@ -76,19 +78,21 @@ void KOReaderSyncActivity::performSync() {
|
||||
documentHash = KOReaderDocumentId::calculate(epubPath);
|
||||
}
|
||||
if (documentHash.empty()) {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SYNC_FAILED;
|
||||
statusMessage = "Failed to calculate document hash";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
LOG_DBG("KOSync", "Document hash: %s", documentHash.c_str());
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
statusMessage = "Fetching remote progress...";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdateAndWait();
|
||||
|
||||
// Fetch remote progress
|
||||
@@ -96,19 +100,21 @@ void KOReaderSyncActivity::performSync() {
|
||||
|
||||
if (result == KOReaderSyncClient::NOT_FOUND) {
|
||||
// No remote progress - offer to upload
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = NO_REMOTE_PROGRESS;
|
||||
hasRemoteProgress = false;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
if (result != KOReaderSyncClient::OK) {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SYNC_FAILED;
|
||||
statusMessage = KOReaderSyncClient::errorString(result);
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
@@ -122,18 +128,20 @@ void KOReaderSyncActivity::performSync() {
|
||||
CrossPointPosition localPos = {currentSpineIndex, currentPage, totalPagesInSpine};
|
||||
localProgress = ProgressMapper::toKOReader(epub, localPos);
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SHOWING_RESULT;
|
||||
selectedOption = 0; // Default to "Apply"
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
void KOReaderSyncActivity::performUpload() {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = UPLOADING;
|
||||
statusMessage = "Uploading progress...";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
requestUpdateAndWait();
|
||||
|
||||
@@ -149,17 +157,19 @@ void KOReaderSyncActivity::performUpload() {
|
||||
const auto result = KOReaderSyncClient::updateProgress(progress);
|
||||
|
||||
if (result != KOReaderSyncClient::OK) {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SYNC_FAILED;
|
||||
statusMessage = KOReaderSyncClient::errorString(result);
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = UPLOAD_COMPLETE;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
@@ -190,9 +200,10 @@ void KOReaderSyncActivity::onEnter() {
|
||||
auto* self = static_cast<KOReaderSyncActivity*>(param);
|
||||
// Sync time first
|
||||
syncTimeWithNTP();
|
||||
xSemaphoreTake(self->renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*self);
|
||||
self->statusMessage = "Calculating document hash...";
|
||||
xSemaphoreGive(self->renderingMutex);
|
||||
}
|
||||
self->requestUpdate();
|
||||
self->performSync();
|
||||
vTaskDelete(nullptr);
|
||||
|
||||
@@ -118,9 +118,10 @@ void ClearCacheActivity::loop() {
|
||||
if (state == WARNING) {
|
||||
if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) {
|
||||
LOG_DBG("CLEAR_CACHE", "User confirmed, starting cache clear");
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = CLEARING;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdateAndWait();
|
||||
|
||||
clearCache();
|
||||
|
||||
@@ -14,18 +14,20 @@ void KOReaderAuthActivity::onWifiSelectionComplete(const bool success) {
|
||||
exitActivity();
|
||||
|
||||
if (!success) {
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = FAILED;
|
||||
errorMessage = "WiFi connection failed";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = AUTHENTICATING;
|
||||
statusMessage = "Authenticating...";
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
|
||||
performAuthentication();
|
||||
@@ -34,7 +36,8 @@ void KOReaderAuthActivity::onWifiSelectionComplete(const bool success) {
|
||||
void KOReaderAuthActivity::performAuthentication() {
|
||||
const auto result = KOReaderSyncClient::authenticate();
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
if (result == KOReaderSyncClient::OK) {
|
||||
state = SUCCESS;
|
||||
statusMessage = "Successfully authenticated!";
|
||||
@@ -42,7 +45,7 @@ void KOReaderAuthActivity::performAuthentication() {
|
||||
state = FAILED;
|
||||
errorMessage = KOReaderSyncClient::errorString(result);
|
||||
}
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
|
||||
@@ -121,7 +121,6 @@ void KOReaderSettingsActivity::handleSelection() {
|
||||
// Authenticate
|
||||
if (!KOREADER_STORE.hasCredentials()) {
|
||||
// Can't authenticate without credentials - just show message briefly
|
||||
xSemaphoreGive(renderingMutex);
|
||||
return;
|
||||
}
|
||||
exitActivity();
|
||||
|
||||
@@ -20,33 +20,37 @@ void OtaUpdateActivity::onWifiSelectionComplete(const bool success) {
|
||||
|
||||
LOG_DBG("OTA", "WiFi connected, checking for update");
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = CHECKING_FOR_UPDATE;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdateAndWait();
|
||||
|
||||
const auto res = updater.checkForUpdate();
|
||||
if (res != OtaUpdater::OK) {
|
||||
LOG_DBG("OTA", "Update check failed: %d", res);
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = FAILED;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
if (!updater.isUpdateNewer()) {
|
||||
LOG_DBG("OTA", "No new update available");
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = NO_UPDATE;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
return;
|
||||
}
|
||||
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = WAITING_CONFIRMATION;
|
||||
xSemaphoreGive(renderingMutex);
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user