keira: add task creation guards

lib: make buzzer melody task cleanup thread-safe
and3rson · Mar 11, 2024 · d6ac7cb · d6ac7cb
1 parent 83ec907
commit d6ac7cb
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 10 deletions.
diff --git a/firmware/keira/src/app.cpp b/firmware/keira/src/app.cpp
@@ -3,25 +3,37 @@
 App::App(const char* name) : App(name, 0, 24, lilka::display.width(), lilka::display.height() - 24) {
 }
 
-App::App(const char* name, uint16_t x, uint16_t y, uint16_t w, uint16_t h) : name(name), x(x), y(y), w(w), h(h) {
-    canvas = new lilka::Canvas(x, y, w, h);
+App::App(const char* name, uint16_t x, uint16_t y, uint16_t w, uint16_t h) :
+    name(name),
+    x(x),
+    y(y),
+    w(w),
+    h(h),
+    flags(AppFlags::APP_FLAG_NONE),
+    taskHandle(NULL),
+    canvas(new lilka::Canvas(x, y, w, h)),
+    backCanvas(new lilka::Canvas(x, y, w, h)),
+    isDrawQueued(false),
+    backCanvasMutex(xSemaphoreCreateMutex()) {
     canvas->begin();
     canvas->fillScreen(0);
-    backCanvas = new lilka::Canvas(x, y, w, h);
     backCanvas->begin();
     backCanvas->fillScreen(0);
-    backCanvasMutex = xSemaphoreCreateMutex();
-    isDrawQueued = false;
-    flags = AppFlags::APP_FLAG_NONE;
     Serial.println(
         "Created app " + String(name) + " at " + String(x) + ", " + String(y) + " with size " + String(w) + "x" +
         String(h)
     );
 }
 
 void App::start() {
+    if (taskHandle != NULL) {
+        Serial.println("App " + String(name) + " is already running");
+        return;
+    }
     Serial.println("Starting app " + String(name));
-    xTaskCreate(_run, name, 8192, this, 1, &taskHandle);
+    if (xTaskCreate(_run, name, 8192, this, 1, &taskHandle) != pdPASS) {
+        Serial.println("Failed to create task for app " + String(name) + " (not enough memory?)");
+    }
 }
 
 void App::_run(void* data) {
@@ -34,23 +46,34 @@ void App::_run(void* data) {
 }
 
 void App::suspend() {
-    // TODO: Check if the task is already suspended
+    if (taskHandle == NULL) {
+        Serial.println("App " + String(name) + " is not running, cannot suspend");
+        return;
+    }
     Serial.println("Suspending app " + String(name) + " (state = " + String(getState()) + ")");
     onSuspend();
     vTaskSuspend(taskHandle);
 }
 
 void App::resume() {
-    // TODO: Check if the task is already running
+    if (taskHandle == NULL) {
+        Serial.println("App " + String(name) + " is not running, cannot resume");
+        return;
+    }
     Serial.println("Resuming app " + String(name) + " (state = " + String(getState()) + ")");
     onResume();
     vTaskResume(taskHandle);
 }
 
 void App::stop() {
+    if (taskHandle == NULL) {
+        Serial.println("App " + String(name) + " is not running, cannot stop");
+        return;
+    }
     Serial.println("Stopping app " + String(name) + " (state = " + String(getState()) + ")");
     onStop();
     vTaskDelete(taskHandle);
+    taskHandle = NULL;
 }
 
 App::~App() {

diff --git a/sdk/lib/lilka/src/lilka/buzzer.cpp b/sdk/lib/lilka/src/lilka/buzzer.cpp
@@ -91,8 +91,24 @@ void Buzzer::melodyTask(void* arg) {
         xSemaphoreGive(buzzer->buzzerMutex);
         vTaskDelay(duration / portTICK_PERIOD_MS);
     }
+    // Start another lambda task to delete the melody task & release mutex
+    xSemaphoreTake(buzzer->buzzerMutex, portMAX_DELAY);
     noTone(LILKA_BUZZER);
-    vTaskSuspend(NULL);
+    if (xTaskCreate(buzzer->melodyTaskCleanup, "melodyTaskCleanup", 2048, buzzer, 1, NULL) != pdPASS) {
+        serial_err("Failed to create melody task cleanup task (not enough memory?)");
+        // Failsafe: release the mutex & delete task.
+        // This is NOT thread-safe, but it's better than a deadlock.
+        xSemaphoreGive(buzzer->buzzerMutex);
+        vTaskDelete(NULL);
+        noTone(LILKA_BUZZER);
+    }
+}
+
+void Buzzer::melodyTaskCleanup(void* arg) {
+    Buzzer* buzzer = static_cast<Buzzer*>(arg);
+    vTaskDelete(buzzer->melodyTaskHandle);
+    buzzer->melodyTaskHandle = NULL;
+    xSemaphoreGive(buzzer->buzzerMutex);
 }
 
 void Buzzer::stop() {

diff --git a/sdk/lib/lilka/src/lilka/buzzer.h b/sdk/lib/lilka/src/lilka/buzzer.h
@@ -91,6 +91,8 @@ class Buzzer {
 private:
     // cppcheck-suppress unusedPrivateFunction
     void _stop();
+    // cppcheck-suppress unusedPrivateFunction
+    static void melodyTaskCleanup(void* arg);
 
     SemaphoreHandle_t buzzerMutex;
     TaskHandle_t melodyTaskHandle;