Documentation/ideas.md

@@ -11,7 +11,6 @@
 
 ## Higher Priority
 
-- stopAppFromToolbar() in Tactility.cpp stops the top-most app. Change it so the toolbar knows for which app id it is created, so it can rely on that.
 - Warn if file operations are done from prohibited tasks (e.g. lvgl task)
 - AppHubApp: Prevent download callbacks from accessing a destroyed view.
 - Move USB host task stacks to SPIRAM when available: esp32_usbhost*.cpp

Modules/app-module/include/app/event.h

@@ -84,8 +84,7 @@ struct AppEventSubscription {
  * task_event_group_wait()/task_event_group_wait_any() on this group (OR sub->bit into the mask,
  * or use _wait_any() to include every subscription sharing it), then drain with app_event_poll().
  * @retval ERROR_NONE on success
- * @retval ERROR_RESOURCE @a event_group has no free bits left to claim; @a sub was not registered
+ * @retval ERROR_RESOURCE @a event_group has no free bits left to claim
- * @retval ERROR_INVALID_STATE @a sub is already registered
  */
 error_t app_event_subscribe(struct AppEventSubscription* sub, struct TaskEventGroup* event_group);
 

Modules/app-module/source/event.cpp

@@ -28,19 +28,12 @@ error_t app_event_subscribe(AppEventSubscription* sub, TaskEventGroup* event_gro
         return claim_result;
     }
 
-    mutex_lock(&subscriptions_mutex.handle);
-
-    // Avoid cyclic subscription list that would loop forever
-    if (subscriptions == sub) {
-        mutex_unlock(&subscriptions_mutex.handle);
-        task_event_group_release_bit(event_group, bit);
-        return ERROR_INVALID_STATE;
-    }
-
     sub->bit = bit;
     sub->internal.event_group = event_group;
     sub->internal.head = 0;
     sub->internal.count = 0;
+
+    mutex_lock(&subscriptions_mutex.handle);
     sub->internal.next = subscriptions;
     subscriptions = sub;
     mutex_unlock(&subscriptions_mutex.handle);

Modules/app-module/source/symbols.cpp

@@ -22,6 +22,14 @@ const ModuleSymbol app_module_symbols[] = {
     DEFINE_MODULE_SYMBOL(app_event_unsubscribe),
     DEFINE_MODULE_SYMBOL(app_event_emit),
     DEFINE_MODULE_SYMBOL(app_event_poll),
+    // tactility/concurrent/task_event_group - needed to construct the group app_event_subscribe() takes
+    DEFINE_MODULE_SYMBOL(task_event_group_construct),
+    DEFINE_MODULE_SYMBOL(task_event_group_destruct),
+    DEFINE_MODULE_SYMBOL(task_event_group_claim_bit),
+    DEFINE_MODULE_SYMBOL(task_event_group_release_bit),
+    DEFINE_MODULE_SYMBOL(task_event_group_signal),
+    DEFINE_MODULE_SYMBOL(task_event_group_wait),
+    DEFINE_MODULE_SYMBOL(task_event_group_wait_any),
     // app/install
     DEFINE_MODULE_SYMBOL(app_get_install_path),
     DEFINE_MODULE_SYMBOL(app_install),

Platforms/platform-esp32/source/drivers/esp32_wifi.cpp

@@ -538,18 +538,10 @@ error_t api_event_subscribe(Device* device, WifiEventSubscription* sub, TaskEven
     if (claim_result != ERROR_NONE) {
         return claim_result;
     }
-
-    mutex_lock(&ctx->subscriptionsMutex);
-
-    // Avoid cyclic subscription list that would loop forever
-    if (ctx->subscriptions == sub) {
-        mutex_unlock(&ctx->subscriptionsMutex);
-        task_event_group_release_bit(event_group, bit);
-        return ERROR_INVALID_STATE;
-    }
-
     sub->internal.event_group = event_group;
     sub->bit = bit;
+
+    mutex_lock(&ctx->subscriptionsMutex);
     sub->internal.next = ctx->subscriptions;
     ctx->subscriptions = sub;
     mutex_unlock(&ctx->subscriptionsMutex);
@@ -637,19 +629,6 @@ error_t stop_device(Device* device) {
         bring_down_wifi(ctx);
     }
 
-    // Release any subscribers that never unsubscribed: device_stop() doesn't wait for apps still
-    // using this device, so a later wifi_event_unsubscribe() would find no ctx and skip releasing
-    // the bit and destructing sub->internal.ring_mutex.
-    mutex_lock(&ctx->subscriptionsMutex);
-    for (WifiEventSubscription* sub = ctx->subscriptions; sub != nullptr;) {
-        WifiEventSubscription* next = sub->internal.next;
-        task_event_group_release_bit(sub->internal.event_group, sub->bit);
-        mutex_destruct(&sub->internal.ring_mutex);
-        sub = next;
-    }
-    ctx->subscriptions = nullptr;
-    mutex_unlock(&ctx->subscriptionsMutex);
-
     device_set_driver_data(device, nullptr);
     mutex_destruct(&ctx->subscriptionsMutex);
     mutex_destruct(&ctx->mutex);

Platforms/platform-posix/source/drivers/mock_wifi.cpp

@@ -55,15 +55,15 @@ struct PosixWifiCtx {
 
 void fireEvent(PosixWifiCtx* ctx, WifiEvent event) {
     mutex_lock(&ctx->subscriptionsMutex);
-    for (WifiEventSubscription* sub = ctx->subscriptions; sub != nullptr; sub = sub->internal.next) {
+    for (WifiEventSubscription* sub = ctx->subscriptions; sub != nullptr; sub = sub->next) {
-        mutex_lock(&sub->internal.ring_mutex);
+        mutex_lock(&sub->ring_mutex);
-        if (sub->internal.count < WIFI_EVENT_QUEUE_CAPACITY) {
+        if (sub->count < WIFI_EVENT_QUEUE_CAPACITY) {
-            uint8_t tail = (sub->internal.head + sub->internal.count) % WIFI_EVENT_QUEUE_CAPACITY;
+            uint8_t tail = (sub->head + sub->count) % WIFI_EVENT_QUEUE_CAPACITY;
-            sub->internal.queue[tail] = event;
+            sub->queue[tail] = event;
-            sub->internal.count++;
+            sub->count++;
         }
-        mutex_unlock(&sub->internal.ring_mutex);
+        mutex_unlock(&sub->ring_mutex);
-        task_event_group_signal(sub->internal.event_group, sub->bit);
+        task_event_group_signal(sub->event_group, sub->bit);
     }
     mutex_unlock(&ctx->subscriptionsMutex);
 }
@@ -266,20 +266,12 @@ error_t apiSetRadioOff(Device* device) {
 
     mutex_lock(&ctx->mutex);
     bool already_off = ctx->radioState == WIFI_RADIO_STATE_OFF;
-    bool was_connected = ctx->stationState != WIFI_STATION_STATE_DISCONNECTED;
     ctx->radioState = WIFI_RADIO_STATE_OFF;
     ctx->stationState = WIFI_STATION_STATE_DISCONNECTED;
     ctx->scanning = false;
     mutex_unlock(&ctx->mutex);
     if (already_off) return ERROR_NONE;
 
-    if (was_connected) {
-        WifiEvent station_event = {};
-        station_event.type = WIFI_EVENT_TYPE_STATION_STATE_CHANGED;
-        station_event.station_state = WIFI_STATION_STATE_DISCONNECTED;
-        fireEvent(ctx, station_event);
-    }
-
     WifiEvent radio_event = {};
     radio_event.type = WIFI_EVENT_TYPE_RADIO_STATE_CHANGED;
     radio_event.radio_state = WIFI_RADIO_STATE_OFF;
@@ -298,19 +290,11 @@ error_t apiEventSubscribe(Device* device, WifiEventSubscription* sub, TaskEventG
     if (claim_result != ERROR_NONE) {
         return claim_result;
     }
+    sub->event_group = event_group;
+    sub->bit = bit;
 
     mutex_lock(&ctx->subscriptionsMutex);
-
+    sub->next = ctx->subscriptions;
-    // Avoid cyclic subscription list that would loop forever
-    if (ctx->subscriptions == sub) {
-        mutex_unlock(&ctx->subscriptionsMutex);
-        task_event_group_release_bit(event_group, bit);
-        return ERROR_INVALID_STATE;
-    }
-
-    sub->internal.event_group = event_group;
-    sub->bit = bit;
-    sub->internal.next = ctx->subscriptions;
     ctx->subscriptions = sub;
     mutex_unlock(&ctx->subscriptionsMutex);
     return ERROR_NONE;
@@ -322,9 +306,9 @@ error_t apiEventUnsubscribe(Device* device, WifiEventSubscription* sub) {
 
     error_t result = ERROR_NOT_FOUND;
     mutex_lock(&ctx->subscriptionsMutex);
-    for (WifiEventSubscription** link = &ctx->subscriptions; *link != nullptr; link = &(*link)->internal.next) {
+    for (WifiEventSubscription** link = &ctx->subscriptions; *link != nullptr; link = &(*link)->next) {
         if (*link == sub) {
-            *link = sub->internal.next;
+            *link = sub->next;
             result = ERROR_NONE;
             break;
         }
@@ -332,7 +316,7 @@ error_t apiEventUnsubscribe(Device* device, WifiEventSubscription* sub) {
     mutex_unlock(&ctx->subscriptionsMutex);
 
     if (result == ERROR_NONE) {
-        task_event_group_release_bit(sub->internal.event_group, sub->bit);
+        task_event_group_release_bit(sub->event_group, sub->bit);
     }
     return result;
 }
@@ -380,19 +364,6 @@ error_t stopDevice(Device* device) {
         apiSetRadioOff(device);
     }
 
-    // Release any subscribers that never unsubscribed: device_stop() doesn't wait for apps still
-    // using this device, so a later wifi_event_unsubscribe() would find no ctx and skip releasing
-    // the bit and destructing sub->internal.ring_mutex.
-    mutex_lock(&ctx->subscriptionsMutex);
-    for (WifiEventSubscription* sub = ctx->subscriptions; sub != nullptr;) {
-        WifiEventSubscription* next = sub->internal.next;
-        task_event_group_release_bit(sub->internal.event_group, sub->bit);
-        mutex_destruct(&sub->internal.ring_mutex);
-        sub = next;
-    }
-    ctx->subscriptions = nullptr;
-    mutex_unlock(&ctx->subscriptionsMutex);
-
     device_set_driver_data(device, nullptr);
     mutex_destruct(&ctx->subscriptionsMutex);
     mutex_destruct(&ctx->mutex);

Tactility/Source/Tactility.cpp

@@ -141,6 +141,7 @@ namespace app {
     namespace boot { extern const ::AppManifest manifest; }
     namespace development { extern const ::AppManifest manifest; }
     namespace display { extern const ::AppManifest manifest; }
+    namespace kerneldisplay { extern const ::AppManifest manifest; }
     namespace files { extern const ::AppManifest manifest; }
     namespace fileselection { extern const ::AppManifest manifest; }
     namespace gpssettings { extern const ::AppManifest manifest; }
@@ -205,7 +206,7 @@ static void registerInternalApps() {
         app_manager_add(&app::audiosettings::manifest);
     }
     if (device_exists_of_type(&DISPLAY_TYPE)) {
-        app_manager_add(&app::display::manifest);
+        app_manager_add(&app::kerneldisplay::manifest);
     }
     app_manager_add(&app::files::manifest);
     app_manager_add(&app::fileselection::manifest);

Tactility/Source/app/kerneldisplay/KernelDisplay.cpp

@@ -12,6 +12,7 @@
 #include <Tactility/settings/DisplaySettings.h>
 
 #include <app/event.h>
+#include <app/manager.h>
 #include <app/manifest.h>
 
 #include <lvgl_window_manager/window_manager.h>
@@ -24,11 +25,11 @@
 #include <sdkconfig.h>
 #endif
 
-namespace tt::app::display {
+namespace tt::app::kerneldisplay {
 
 extern const ::AppManifest manifest;
 
-constexpr auto* TAG = "Display";
+constexpr auto* TAG = "KernelDisplay";
 
 namespace {
 
@@ -350,4 +351,4 @@ extern const ::AppManifest manifest = {
     .location = { APP_LOCATION_MEMORY, reinterpret_cast<void*>(appMain) }
 };
 
-} // namespace tt::app::display
+} // namespace tt::app::kerneldisplay

Tactility/Source/app/wifimanage/WifiManage.cpp

@@ -72,14 +72,11 @@ static void onConnectToHidden() {
 }
 
 void updateView(Context* ctx) {
-    // Same lock order as createWidgets() (called with the LVGL lock already held, per the
-    // window-manager's WindowCreateWidgetsFn contract, then acquiring ctx->mutex) - acquiring
-    // these in the opposite order here would deadlock against a concurrent createWidgets() call.
-    lvgl_lock();
     ctx->lock();
+    lvgl_lock();
     ctx->view.update();
-    ctx->unlock();
     lvgl_unlock();
+    ctx->unlock();
 }
 
 void onWifiEvent(Context* ctx, WifiEvent event) {

Tactility/Source/service/wifi/Wifi.cpp

@@ -229,7 +229,18 @@ void dispatchConnect() {
 
     LOG_I(TAG, "Connecting to %s", target.ssid.c_str());
 
-    wifi_station_connect(state.device, target.ssid.c_str(), target.password.c_str(), target.channel);
+    error_t result = wifi_station_connect(state.device, target.ssid.c_str(), target.password.c_str(), target.channel);
+    if (result != ERROR_NONE) {
+        LOG_E(TAG, "Failed to connect to %s (%s)", target.ssid.c_str(), error_to_string(result));
+        WifiEvent event = {};
+        event.type = WIFI_EVENT_TYPE_STATION_CONNECTION_RESULT;
+        // The driver couldn't even initiate the connection attempt; there's no
+        // more specific WifiStationConnectionError for that.
+        event.connection_error = WIFI_STATION_CONNECTION_ERROR_TIMEOUT;
+        publish(event);
+    }
+    // On success, WIFI_EVENT_TYPE_STATION_STATE_CHANGED / _CONNECTION_RESULT arrive
+    // asynchronously via onWifiDeviceEvent().
 }
 
 void dispatchDisconnect() {
@@ -362,6 +373,10 @@ void onWifiDeviceEvent(Device* device, ::WifiEvent event) {
         default:
             break;
     }
+
+    // Forward the event as-is: subscribers inspect event.type and the
+    // relevant union field directly, same as this function does.
+    publish(event);
 }
 
 void autoScanSetPaused(bool paused) {

TactilityKernel/include/tactility/drivers/wifi.h

@@ -245,8 +245,7 @@ struct WifiApi {
      * mask, or use _wait_any() to include every subscription sharing it), then drain with
      * wifi_event_poll().
      * @retval ERROR_NONE on success
-     * @retval ERROR_RESOURCE @a event_group has no free bits left to claim; @a sub was not registered
+     * @retval ERROR_RESOURCE @a event_group has no free bits left to claim
-     * @retval ERROR_INVALID_STATE @a sub is already registered
      */
     error_t (*event_subscribe)(struct Device* device, struct WifiEventSubscription* sub, struct TaskEventGroup* event_group);
 
@@ -299,8 +298,7 @@ error_t wifi_station_get_rssi(struct Device* device, int32_t* rssi);
  * into the mask, or use _wait_any() to include every subscription sharing it), then drain with
  * wifi_event_poll().
  * @retval ERROR_NONE on success
- * @retval ERROR_RESOURCE @a event_group has no free bits left to claim; @a sub was not registered
+ * @retval ERROR_RESOURCE @a event_group has no free bits left to claim
- * @retval ERROR_INVALID_STATE @a sub is already registered
  */
 error_t wifi_event_subscribe(struct Device* device, struct WifiEventSubscription* sub, struct TaskEventGroup* event_group);
 

TactilityKernel/source/system_event.cpp

@@ -173,11 +173,15 @@ error_t system_event_subscribe(SystemEventSubscription* sub, TaskEventGroup* eve
 
     mutex_lock(&poll_subscriptions_mutex.handle);
 
-    // Avoid cyclic subscription list that would loop forever
+    // Check-and-insert in one critical section: registering the same `sub` twice would link
-    if (poll_subscriptions == sub) {
+    // it into a list that already contains it, creating a cycle that notify_poll_subscribers()
-        mutex_unlock(&poll_subscriptions_mutex.handle);
+    // would then traverse forever while holding this same mutex.
-        task_event_group_release_bit(event_group, bit);
+    for (SystemEventSubscription* existing = poll_subscriptions; existing != nullptr; existing = existing->internal.next) {
-        return ERROR_INVALID_STATE;
+        if (existing == sub) {
+            mutex_unlock(&poll_subscriptions_mutex.handle);
+            task_event_group_release_bit(event_group, bit);
+            return ERROR_INVALID_STATE;
+        }
     }
 
     sub->internal.event_group = event_group;