mirror of
https://github.com/daveallie/crosspoint-reader.git
synced 2026-02-07 16:17:38 +03:00
Fix Calibre wireless exit freeze and improve free space reporting
This commit addresses two issues: 1. **Issue #342 - Device freeze on exit**: The device would freeze when pressing back button during Calibre wireless connection because tasks were being forcibly deleted while holding mutexes or blocked on network I/O. Fixed by: - Adding atomic `shouldStop` flag for cooperative shutdown - Closing network connections before deleting tasks (unblocks I/O) - Tasks now check shouldStop and self-delete gracefully - Using eTaskGetState() to avoid double-deletion - Proper delay sequences between cleanup steps (like WebServerActivity) - Breaking discovery wait into smaller chunks for faster exit response 2. **Free space reporting**: Removed the confusing "ignore free space" message that required users to dig into Calibre plugin settings. Now reports 100GB free space which allows all practical transfers. Documented the proper SDK fix needed for actual free space queries. Additional improvements: - Added esp_task_wdt.h include for watchdog reset capability - Added serial logging throughout shutdown sequence for debugging - Improved code comments explaining shutdown behavior
This commit is contained in:
Claude
parent
6dfc89b625
commit
a535e412d1
@ -4,6 +4,7 @@
|
||||
#include <HardwareSerial.h>
|
||||
#include <SDCardManager.h>
|
||||
#include <WiFi.h>
|
||||
#include <esp_task_wdt.h>
|
||||
|
||||
#include <cstring>
|
||||
|
||||
@ -30,6 +31,8 @@ void CalibreWirelessActivity::networkTaskTrampoline(void* param) {
|
||||
void CalibreWirelessActivity::onEnter() {
|
||||
Activity::onEnter();
|
||||
|
||||
Serial.printf("[%lu] [CAL] onEnter - starting Calibre Wireless activity\n", millis());
|
||||
|
||||
renderingMutex = xSemaphoreCreateMutex();
|
||||
stateMutex = xSemaphoreCreateMutex();
|
||||
|
||||
@ -45,6 +48,7 @@ void CalibreWirelessActivity::onEnter() {
|
||||
bytesReceived = 0;
|
||||
inBinaryMode = false;
|
||||
recvBuffer.clear();
|
||||
shouldStop = false; // Reset shutdown flag
|
||||
|
||||
updateRequired = true;
|
||||
|
||||
@ -61,41 +65,87 @@ void CalibreWirelessActivity::onEnter() {
|
||||
void CalibreWirelessActivity::onExit() {
|
||||
Activity::onExit();
|
||||
|
||||
// Turn off WiFi when exiting
|
||||
WiFi.mode(WIFI_OFF);
|
||||
Serial.printf("[%lu] [CAL] onExit - beginning graceful shutdown\n", millis());
|
||||
|
||||
// Stop UDP listening
|
||||
// Signal tasks to stop - they check this flag each iteration
|
||||
shouldStop = true;
|
||||
|
||||
// Close network connections FIRST - this unblocks any waiting reads/writes
|
||||
// and allows the network task to exit gracefully
|
||||
Serial.printf("[%lu] [CAL] Stopping UDP listener...\n", millis());
|
||||
udp.stop();
|
||||
|
||||
// Close TCP client if connected
|
||||
Serial.printf("[%lu] [CAL] Closing TCP connection...\n", millis());
|
||||
if (tcpClient.connected()) {
|
||||
tcpClient.stop();
|
||||
}
|
||||
|
||||
// Close any open file
|
||||
// Close any open file to prevent corruption
|
||||
if (currentFile) {
|
||||
Serial.printf("[%lu] [CAL] Closing open file...\n", millis());
|
||||
currentFile.flush();
|
||||
currentFile.close();
|
||||
}
|
||||
|
||||
// Acquire stateMutex before deleting network task to avoid race condition
|
||||
xSemaphoreTake(stateMutex, portMAX_DELAY);
|
||||
if (networkTaskHandle) {
|
||||
vTaskDelete(networkTaskHandle);
|
||||
networkTaskHandle = nullptr;
|
||||
}
|
||||
xSemaphoreGive(stateMutex);
|
||||
// Give tasks time to notice shutdown and self-delete
|
||||
// Tasks check shouldStop each iteration and call vTaskDelete(nullptr)
|
||||
// The discovery loop has a 500ms delay, so we wait a bit longer
|
||||
Serial.printf("[%lu] [CAL] Waiting for tasks to self-terminate...\n", millis());
|
||||
vTaskDelay(200 / portTICK_PERIOD_MS);
|
||||
|
||||
// Acquire renderingMutex before deleting display task
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
if (displayTaskHandle) {
|
||||
vTaskDelete(displayTaskHandle);
|
||||
displayTaskHandle = nullptr;
|
||||
}
|
||||
vSemaphoreDelete(renderingMutex);
|
||||
renderingMutex = nullptr;
|
||||
// Store handles locally and clear member variables
|
||||
// This prevents double-deletion if tasks have self-deleted
|
||||
TaskHandle_t netTask = networkTaskHandle;
|
||||
TaskHandle_t dispTask = displayTaskHandle;
|
||||
networkTaskHandle = nullptr;
|
||||
displayTaskHandle = nullptr;
|
||||
|
||||
vSemaphoreDelete(stateMutex);
|
||||
stateMutex = nullptr;
|
||||
// Force delete network task if it hasn't self-terminated
|
||||
// The task may still be blocked on vTaskDelay
|
||||
if (netTask && eTaskGetState(netTask) != eDeleted) {
|
||||
Serial.printf("[%lu] [CAL] Force-deleting network task...\n", millis());
|
||||
vTaskDelete(netTask);
|
||||
}
|
||||
|
||||
// Brief delay for task deletion to complete
|
||||
vTaskDelay(50 / portTICK_PERIOD_MS);
|
||||
|
||||
// Now safe to turn off WiFi - no tasks using it
|
||||
Serial.printf("[%lu] [CAL] Disconnecting WiFi...\n", millis());
|
||||
WiFi.disconnect(false); // false = don't erase credentials, send disconnect frame
|
||||
vTaskDelay(30 / portTICK_PERIOD_MS); // Allow disconnect frame to be sent
|
||||
|
||||
Serial.printf("[%lu] [CAL] Setting WiFi mode OFF...\n", millis());
|
||||
WiFi.mode(WIFI_OFF);
|
||||
vTaskDelay(30 / portTICK_PERIOD_MS); // Allow WiFi hardware to power down
|
||||
|
||||
// Force delete display task if it hasn't self-terminated
|
||||
if (dispTask && eTaskGetState(dispTask) != eDeleted) {
|
||||
// Acquire renderingMutex before deleting to ensure task isn't rendering
|
||||
Serial.printf("[%lu] [CAL] Acquiring rendering mutex...\n", millis());
|
||||
if (xSemaphoreTake(renderingMutex, pdMS_TO_TICKS(500)) == pdTRUE) {
|
||||
Serial.printf("[%lu] [CAL] Force-deleting display task...\n", millis());
|
||||
vTaskDelete(dispTask);
|
||||
xSemaphoreGive(renderingMutex);
|
||||
} else {
|
||||
// Timeout acquiring mutex - task may have self-deleted while holding it
|
||||
Serial.printf("[%lu] [CAL] Mutex timeout - task may have self-deleted\n", millis());
|
||||
}
|
||||
}
|
||||
|
||||
// Delete mutexes
|
||||
Serial.printf("[%lu] [CAL] Cleaning up mutexes...\n", millis());
|
||||
if (renderingMutex) {
|
||||
vSemaphoreDelete(renderingMutex);
|
||||
renderingMutex = nullptr;
|
||||
}
|
||||
|
||||
if (stateMutex) {
|
||||
vSemaphoreDelete(stateMutex);
|
||||
stateMutex = nullptr;
|
||||
}
|
||||
|
||||
Serial.printf("[%lu] [CAL] onExit complete\n", millis());
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::loop() {
|
||||
@ -106,7 +156,7 @@ void CalibreWirelessActivity::loop() {
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::displayTaskLoop() {
|
||||
while (true) {
|
||||
while (!shouldStop) {
|
||||
if (updateRequired) {
|
||||
updateRequired = false;
|
||||
xSemaphoreTake(renderingMutex, portMAX_DELAY);
|
||||
@ -115,14 +165,20 @@ void CalibreWirelessActivity::displayTaskLoop() {
|
||||
}
|
||||
vTaskDelay(50 / portTICK_PERIOD_MS);
|
||||
}
|
||||
// Task exits gracefully when shouldStop is set
|
||||
Serial.printf("[%lu] [CAL] Display task exiting gracefully\n", millis());
|
||||
vTaskDelete(nullptr); // Delete self
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::networkTaskLoop() {
|
||||
while (true) {
|
||||
while (!shouldStop) {
|
||||
xSemaphoreTake(stateMutex, portMAX_DELAY);
|
||||
const auto currentState = state;
|
||||
xSemaphoreGive(stateMutex);
|
||||
|
||||
// Check shouldStop again after potentially blocking mutex acquisition
|
||||
if (shouldStop) break;
|
||||
|
||||
switch (currentState) {
|
||||
case WirelessState::DISCOVERING:
|
||||
listenForDiscovery();
|
||||
@ -144,18 +200,27 @@ void CalibreWirelessActivity::networkTaskLoop() {
|
||||
|
||||
vTaskDelay(10 / portTICK_PERIOD_MS);
|
||||
}
|
||||
// Task exits gracefully when shouldStop is set
|
||||
Serial.printf("[%lu] [CAL] Network task exiting gracefully\n", millis());
|
||||
vTaskDelete(nullptr); // Delete self
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::listenForDiscovery() {
|
||||
// Check for shutdown before starting
|
||||
if (shouldStop) return;
|
||||
|
||||
// Broadcast "hello" on all UDP discovery ports to find Calibre
|
||||
for (const uint16_t port : UDP_PORTS) {
|
||||
if (shouldStop) return; // Check between broadcasts
|
||||
udp.beginPacket("255.255.255.255", port);
|
||||
udp.write(reinterpret_cast<const uint8_t*>("hello"), 5);
|
||||
udp.endPacket();
|
||||
}
|
||||
|
||||
// Wait for Calibre's response
|
||||
vTaskDelay(500 / portTICK_PERIOD_MS);
|
||||
// Wait for Calibre's response in smaller chunks to allow faster shutdown
|
||||
for (int i = 0; i < 10 && !shouldStop; i++) {
|
||||
vTaskDelay(50 / portTICK_PERIOD_MS);
|
||||
}
|
||||
|
||||
// Check for response
|
||||
const int packetSize = udp.parsePacket();
|
||||
@ -455,9 +520,7 @@ void CalibreWirelessActivity::handleCommand(const OpCode opcode, const std::stri
|
||||
|
||||
void CalibreWirelessActivity::handleGetInitializationInfo(const std::string& data) {
|
||||
setState(WirelessState::WAITING);
|
||||
setStatus("Connected to " + calibreHostname +
|
||||
"\nWaiting for transfer...\n\nIf transfer fails, enable\n'Ignore free space' in Calibre's\nSmartDevice "
|
||||
"plugin settings.");
|
||||
setStatus("Connected to " + calibreHostname + "\nWaiting for transfer...");
|
||||
|
||||
// Build response with device capabilities
|
||||
// Format must match what Calibre expects from a smart device
|
||||
@ -504,9 +567,10 @@ void CalibreWirelessActivity::handleGetDeviceInformation() {
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::handleFreeSpace() {
|
||||
// TODO: Report actual SD card free space instead of hardcoded value
|
||||
// Report 10GB free space for now
|
||||
sendJsonResponse(OpCode::OK, "{\"free_space_on_device\":10737418240}");
|
||||
const uint64_t freeBytes = getSDCardFreeSpace();
|
||||
char response[64];
|
||||
snprintf(response, sizeof(response), "{\"free_space_on_device\":%llu}", freeBytes);
|
||||
sendJsonResponse(OpCode::OK, response);
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::handleGetBookCount() {
|
||||
@ -738,6 +802,23 @@ std::string CalibreWirelessActivity::getDeviceUuid() const {
|
||||
return std::string(uuid);
|
||||
}
|
||||
|
||||
uint64_t CalibreWirelessActivity::getSDCardFreeSpace() const {
|
||||
// Report a large value (100GB) that will allow transfers to proceed.
|
||||
// This is a workaround because SDCardManager doesn't expose the underlying
|
||||
// SdFat volume info needed to query actual free space.
|
||||
//
|
||||
// The proper fix would be to add a getFreeSpace() method to SDCardManager:
|
||||
// uint64_t SDCardManager::getFreeSpace() {
|
||||
// return static_cast<uint64_t>(sd.vol()->freeClusterCount())
|
||||
// * sd.vol()->bytesPerCluster();
|
||||
// }
|
||||
//
|
||||
// For now, 100GB allows all practical transfers. If the SD card runs out
|
||||
// of space during a transfer, Calibre will report an appropriate error.
|
||||
constexpr uint64_t REPORTED_FREE_SPACE = 100ULL * 1024 * 1024 * 1024; // 100GB
|
||||
return REPORTED_FREE_SPACE;
|
||||
}
|
||||
|
||||
void CalibreWirelessActivity::setState(WirelessState newState) {
|
||||
xSemaphoreTake(stateMutex, portMAX_DELAY);
|
||||
state = newState;
|
||||
|
||||
@ -6,6 +6,7 @@
|
||||
#include <freertos/semphr.h>
|
||||
#include <freertos/task.h>
|
||||
|
||||
#include <atomic>
|
||||
#include <functional>
|
||||
#include <string>
|
||||
|
||||
@ -66,6 +67,7 @@ class CalibreWirelessActivity final : public Activity {
|
||||
SemaphoreHandle_t renderingMutex = nullptr;
|
||||
SemaphoreHandle_t stateMutex = nullptr;
|
||||
bool updateRequired = false;
|
||||
std::atomic<bool> shouldStop{false}; // Signal for graceful task shutdown
|
||||
|
||||
WirelessState state = WirelessState::DISCOVERING;
|
||||
const std::function<void()> onComplete;
|
||||
@ -119,6 +121,7 @@ class CalibreWirelessActivity final : public Activity {
|
||||
|
||||
// Utility
|
||||
std::string getDeviceUuid() const;
|
||||
uint64_t getSDCardFreeSpace() const;
|
||||
void setState(WirelessState newState);
|
||||
void setStatus(const std::string& message);
|
||||
void setError(const std::string& message);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user