Various services improved (#110)

App/Source/Main.cpp

@@ -4,7 +4,7 @@
 #include "Tactility.h"
 
 namespace tt::service::wifi {
-    extern void wifi_main(void*);
+    extern void wifi_task(void*);
 }
 
 extern const tt::app::AppManifest hello_world_app;
@@ -25,9 +25,7 @@ void app_main() {
         .auto_start_app_id = nullptr
     };
 
-    tt::init(config);
-
-    tt::service::wifi::wifi_main(nullptr);
+    tt::run(config);
 }
 
 } // extern

Boards/LilygoTdeck/Source/Sdcard.cpp

@@ -144,7 +144,7 @@ static bool sdcard_is_mounted(void* context) {
      * Writing and reading to the bus from 2 devices at the same time causes crashes.
      * This work-around ensures that this check is only happening when LVGL isn't rendering.
      */
-    bool locked = tt::lvgl::lock(100); // TODO: Refactor to a more reliable locking mechanism
+    bool locked = tt::lvgl::lock(50); // TODO: Refactor to a more reliable locking mechanism
     if (!locked) {
         TT_LOG_W(TAG, "Failed to get LVGL lock");
     }

Documentation/app-lifecycle.puml

@@ -1,9 +1,9 @@
 @startuml
-[*] --> on_create : app is started
-on_create --> on_show : app becomes visible
-on_show --> on_hide : app is no longer visible
-on_hide --> on_destroy : app is being closed
-on_destroy --> [*]
+[*] --> onStart : app is created
+onStart --> onShow : app becomes visible
+onShow --> onHide : app is no longer visible
+onHide --> onStop : app is preparing to be destroyed
+onStop --> [*] : app is destroyed
 skinparam ranksep 25
 skinparam padding 2
 @enduml

Documentation/ideas.md

@@ -1,32 +1,30 @@
 # TODOs
-- Bug: sdcard file reading fails (due to `A:/` prefix?)
-- Publish firmwares with upload tool
-- Bug: When closing a top level app, there's often an error "can't stop root app"
 - Bug: I2C Scanner is on M5Stack devices is broken
+- WiFi AP Connect app: add "Forget" option.
+- T-Deck Plus: Implement battery status
+- Make firmwares available via release process
+- Make firmwares available via web serial website
+- Bug: When closing a top level app, there's often an error "can't stop root app"
 - Create more unit tests for `tactility-core` and `tactility` (PC-only for now)
-- WiFi on-at-boot should be a setting in its app
-- Create app to edit WiFi settings (e.g. "forget" and "auto-connect" option)
 - Show a warning screen if firmware encryption or secure boot are off when saving WiFi credentials.
 - Show a warning screen when a user plugs in the SD card on a device that only supports mounting at boot.
-- T-Deck has random sdcard SPI crashes due to sharing bus with screen SPI: make it use the LVGL lock for sdcard operations?
 - Check service/app id on registration to see if it is a duplicate id
 - Fix screenshot app on ESP32: it currently blocks when allocating memory
-- Localisation of texts
+- Localisation of texts (load in boot app from sd?)
 - Portrait support for GPIO app
-- App lifecycle docs mention on_create/on_destroy but app lifecycle is on_start/on_stop
 - Explore LVGL9's FreeRTOS functionality
 - Explore LVGL9's ILI93414 driver for 2.4" Yellow Board
 - Bug: in LVGL9 with M5Core2, crash when bottom item is clicked without scrolling first
 - Replace M5Unified and M5GFX with custom drivers (so we can fix the Core2 SD card mounting bug, and so we regain some firmware space)
 - Commit fix to esp_lvgl_port to have `esp_lvgl_port_disp.c` user driver_data instead of user_data
 - Wifi bug: when pressing disconnect while between `WIFI_EVENT_STA_START` and `IP_EVENT_STA_GOT_IP`, then auto-connect becomes activate again.
+- T-Deck Plus: Create separate board config
 
 # Core Ideas
 - Support for displays with different DPI. Consider the layer-based system like on Android.
 - If present, use LED to show boot status
 - 2 wire speaker support
 - tt::app::start() and similar functions as proxies for Loader app start/stop/etc.
-- Wi-Fi using dispatcher to dispatch its main functionality to the dedicated Wi-Fi CPU core (to avoid main loop hack)
 
 # App Ideas
 - System logger

Tactility/Private/service/loader/Loader_i.h

@@ -140,10 +140,10 @@ public:
 
 struct Loader {
     Thread* thread;
-    PubSub* pubsub_internal;
-    PubSub* pubsub_external;
+    std::shared_ptr<PubSub> pubsub_internal = std::make_shared<PubSub>();
+    std::shared_ptr<PubSub> pubsub_external = std::make_shared<PubSub>();
     MessageQueue queue = MessageQueue(2, sizeof(LoaderMessage)); // 2 entries, so you can stop the current app while starting a new one without blocking
-    Mutex* mutex;
+    Mutex mutex = Mutex(MutexTypeRecursive);
     std::stack<app::AppInstance*> app_stack;
 };
 

Tactility/Source/Tactility.cpp

@@ -1,3 +1,4 @@
+#include <Dispatcher.h>
 #include "Tactility.h"
 
 #include "app/ManifestRegistry.h"
@@ -127,7 +128,7 @@ static void register_and_start_user_services(const service::ServiceManifest* con
     }
 }
 
-void init(const Configuration& config) {
+void run(const Configuration& config) {
     TT_LOG_I(TAG, "init started");
 
     tt_assert(config.hardware);
@@ -160,6 +161,11 @@ void init(const Configuration& config) {
     }
 
     TT_LOG_I(TAG, "init complete");
+
+    TT_LOG_I(TAG, "Processing main dispatcher");
+    while (true) {
+        getMainDispatcher().consume(TtWaitForever);
+    }
 }
 
 const Configuration* _Nullable getConfiguration() {

Tactility/Source/Tactility.h

@@ -19,7 +19,7 @@ typedef struct {
  * Attempts to initialize Tactility and all configured hardware.
  * @param config
  */
-void init(const Configuration& config);
+void run(const Configuration& config);
 
 /**
  * While technically nullable, this instance is always set if tt_init() succeeds.

Tactility/Source/app/power/Power.cpp

@@ -13,7 +13,7 @@ namespace tt::app::power {
 #define TAG "power"
 
 extern const AppManifest manifest;
-static void on_timer(TT_UNUSED void* context);
+static void on_timer(TT_UNUSED std::shared_ptr<void> context);
 
 struct Data {
     std::unique_ptr<Timer> update_timer = std::unique_ptr<Timer>(new Timer(Timer::TypePeriodic, &on_timer, nullptr));

Tactility/Source/app/wificonnect/WifiConnect.cpp

@@ -54,7 +54,7 @@ static void onConnect(const service::wifi::settings::WifiApSettings* ap_settings
 }
 
 WifiConnect::WifiConnect() {
-    PubSub* wifi_pubsub = service::wifi::getPubsub();
+    auto wifi_pubsub = service::wifi::getPubsub();
     wifiSubscription = tt_pubsub_subscribe(wifi_pubsub, &eventCallback, this);
     bindings = (Bindings) {
         .onConnectSsid = onConnect,
@@ -63,7 +63,7 @@ WifiConnect::WifiConnect() {
 }
 
 WifiConnect::~WifiConnect() {
-    PubSub* pubsub = service::wifi::getPubsub();
+    auto pubsub = service::wifi::getPubsub();
     tt_pubsub_unsubscribe(pubsub, wifiSubscription);
 }
 

Tactility/Source/app/wifimanage/WifiManage.cpp

@@ -108,7 +108,7 @@ static void wifiManageEventCallback(const void* message, void* context) {
 }
 
 void WifiManage::onShow(AppContext& app, lv_obj_t* parent) {
-    PubSub* wifi_pubsub = service::wifi::getPubsub();
+    auto wifi_pubsub = service::wifi::getPubsub();
     wifiSubscription = tt_pubsub_subscribe(wifi_pubsub, &wifiManageEventCallback, this);
 
     // State update (it has its own locking)
@@ -128,6 +128,7 @@ void WifiManage::onShow(AppContext& app, lv_obj_t* parent) {
     bool can_scan = radio_state == service::wifi::WIFI_RADIO_ON ||
         radio_state == service::wifi::WIFI_RADIO_CONNECTION_PENDING ||
         radio_state == service::wifi::WIFI_RADIO_CONNECTION_ACTIVE;
+    TT_LOG_I(TAG, "%d %d", radio_state, service::wifi::isScanning());
     if (can_scan && !service::wifi::isScanning()) {
         service::wifi::scan();
     }
@@ -135,7 +136,7 @@ void WifiManage::onShow(AppContext& app, lv_obj_t* parent) {
 
 void WifiManage::onHide(TT_UNUSED AppContext& app) {
     lock();
-    PubSub* wifi_pubsub = service::wifi::getPubsub();
+    auto wifi_pubsub = service::wifi::getPubsub();
     tt_pubsub_unsubscribe(wifi_pubsub, wifiSubscription);
     wifiSubscription = nullptr;
     isViewEnabled = false;

Tactility/Source/lvgl/Statusbar.cpp

@@ -22,7 +22,7 @@ typedef struct {
 
 typedef struct {
     Mutex* mutex;
-    PubSub* pubsub;
+    std::shared_ptr<PubSub> pubsub;
     StatusbarIcon icons[STATUSBAR_ICON_LIMIT];
 } StatusbarData;
 
@@ -40,7 +40,7 @@ typedef struct {
 
 static void statusbar_init() {
     statusbar_data.mutex = tt_mutex_alloc(MutexTypeRecursive);
-    statusbar_data.pubsub = tt_pubsub_alloc();
+    statusbar_data.pubsub = std::make_shared<PubSub>();
     for (int i = 0; i < STATUSBAR_ICON_LIMIT; i++) {
         statusbar_data.icons[i].image = nullptr;
         statusbar_data.icons[i].visible = false;

Tactility/Source/service/loader/Loader.cpp

@@ -29,38 +29,30 @@ static Loader* loader_singleton = nullptr;
 static Loader* loader_alloc() {
     assert(loader_singleton == nullptr);
     loader_singleton = new Loader();
-    loader_singleton->pubsub_internal = tt_pubsub_alloc();
-    loader_singleton->pubsub_external = tt_pubsub_alloc();
     loader_singleton->thread = new Thread(
         "loader",
         4096, // Last known minimum was 2400 for starting Hello World app
         &loader_main,
         nullptr
     );
-    loader_singleton->mutex = tt_mutex_alloc(MutexTypeRecursive);
     return loader_singleton;
 }
 
 static void loader_free() {
     tt_assert(loader_singleton != nullptr);
     delete loader_singleton->thread;
-    tt_pubsub_free(loader_singleton->pubsub_internal);
-    tt_pubsub_free(loader_singleton->pubsub_external);
-    tt_mutex_free(loader_singleton->mutex);
     delete loader_singleton;
     loader_singleton = nullptr;
 }
 
 static void loader_lock() {
     tt_assert(loader_singleton);
-    tt_assert(loader_singleton->mutex);
-    tt_check(tt_mutex_acquire(loader_singleton->mutex, TtWaitForever) == TtStatusOk);
+    tt_check(loader_singleton->mutex.acquire(TtWaitForever) == TtStatusOk);
 }
 
 static void loader_unlock() {
     tt_assert(loader_singleton);
-    tt_assert(loader_singleton->mutex);
-    tt_check(tt_mutex_release(loader_singleton->mutex) == TtStatusOk);
+    tt_check(loader_singleton->mutex.release() == TtStatusOk);
 }
 
 LoaderStatus startApp(const std::string& id, bool blocking, std::shared_ptr<const Bundle> parameters) {
@@ -107,7 +99,7 @@ app::AppContext* _Nullable getCurrentApp() {
     return dynamic_cast<app::AppContext*>(app);
 }
 
-PubSub* getPubsub() {
+std::shared_ptr<PubSub> getPubsub() {
     tt_assert(loader_singleton);
     // it's safe to return pubsub without locking
     // because it's never freed and loader is never exited

Tactility/Source/service/loader/Loader.h

@@ -37,6 +37,6 @@ app::AppContext* _Nullable getCurrentApp();
 /**
  * @brief PubSub for LoaderEvent
  */
-PubSub* getPubsub();
+std::shared_ptr<PubSub> getPubsub();
 
 } // namespace

Tactility/Source/service/statusbar/Statusbar.cpp

@@ -1,11 +1,13 @@
 #include "Assets.h"
+#include "Mutex.h"
+#include "Timer.h"
+#include "Tactility.h"
+
 #include "hal/Power.h"
 #include "hal/sdcard/Sdcard.h"
-#include "Mutex.h"
+#include "lvgl/Statusbar.h"
 #include "service/ServiceContext.h"
 #include "service/wifi/Wifi.h"
-#include "Tactility.h"
-#include "lvgl/Statusbar.h"
 #include "service/ServiceRegistry.h"
 
 namespace tt::service::statusbar {
@@ -16,8 +18,7 @@ extern const ServiceManifest manifest;
 
 struct ServiceData {
     Mutex mutex;
-    Thread thread;
-    bool service_interrupted = false;
+    std::unique_ptr<Timer> updateTimer;
     int8_t wifi_icon_id = lvgl::statusbar_icon_add(nullptr);
     const char* wifi_last_icon = nullptr;
     int8_t sdcard_icon_id = lvgl::statusbar_icon_add(nullptr);
@@ -153,51 +154,35 @@ static void service_data_free(ServiceData* data) {
    free(data);
 }
 
-int32_t serviceMain(TT_UNUSED void* parameter) {
-    TT_LOG_I(TAG, "Started main loop");
-    delay_ms(20); // TODO: Make service instance findable earlier on (but expose "starting" state?)
-    auto context = tt::service::findServiceById(manifest.id);
-    if (context == nullptr) {
-        TT_LOG_E(TAG, "Service not found");
-        return -1;
-    }
-
-    auto data = std::static_pointer_cast<ServiceData>(context->getData());
-
-    while (!data->service_interrupted) {
+static void onUpdate(std::shared_ptr<void> parameter) {
+    auto data = std::static_pointer_cast<ServiceData>(parameter);
+    // TODO: Make thread-safe for LVGL
     update_wifi_icon(data);
     update_sdcard_icon(data);
     update_power_icon(data);
-        delay_ms(1000);
-    }
-    return 0;
 }
 
 static void onStart(ServiceContext& service) {
     auto data = std::make_shared<ServiceData>();
     service.setData(data);
 
+    // TODO: Make thread-safe for LVGL
     lvgl::statusbar_icon_set_visibility(data->wifi_icon_id, true);
     update_wifi_icon(data);
     update_sdcard_icon(data); // also updates visibility
     update_power_icon(data);
 
-
-    data->thread.setCallback(serviceMain, nullptr);
-    data->thread.setPriority(Thread::PriorityLow);
-    data->thread.setStackSize(3000);
-    data->thread.setName("statusbar");
-    data->thread.start();
+    data->updateTimer = std::make_unique<Timer>(Timer::TypePeriodic, onUpdate, data);
+    // We want to try and scan more often in case of startup or scan lock failure
+    data->updateTimer->start(1000);
 }
 
 static void onStop(ServiceContext& service) {
     auto data = std::static_pointer_cast<ServiceData>(service.getData());
 
     // Stop thread
-    data->lock();
-    data->service_interrupted = true;
-    data->unlock();
-    data->thread.join();
+    data->updateTimer->stop();
+    data->updateTimer = nullptr;
 }
 
 extern const ServiceManifest manifest = {

TactilityCore/Source/Dispatcher.cpp

@@ -1,39 +1,50 @@
 #include "Dispatcher.h"
+#include "Check.h"
 
 namespace tt {
 
-Dispatcher::Dispatcher(size_t queueLimit) :
-    queue(queueLimit, sizeof(DispatcherMessage)),
-    mutex(MutexTypeNormal),
-    buffer({ .callback = nullptr, .context = nullptr }) { }
+#define TAG "Dispatcher"
+#define BACKPRESSURE_WARNING_COUNT 100
+
+Dispatcher::Dispatcher() :
+    mutex(MutexTypeNormal)
+{}
 
 Dispatcher::~Dispatcher() {
-    queue.reset();
     // Wait for Mutex usage
     mutex.acquire(TtWaitForever);
     mutex.release();
 }
 
-void Dispatcher::dispatch(Callback callback, void* context) {
-    DispatcherMessage message = {
-        .callback = callback,
-        .context = context
-    };
+void Dispatcher::dispatch(Callback callback, std::shared_ptr<void> context) {
+    auto message = std::make_shared<DispatcherMessage>(callback, std::move(context));
+    // Mutate
     mutex.acquire(TtWaitForever);
-    queue.put(&message, TtWaitForever);
+    queue.push(std::move(message));
+    if (queue.size() == BACKPRESSURE_WARNING_COUNT) {
+        TT_LOG_W(TAG, "Backpressure: You're not consuming fast enough (100 queued)");
+    }
     mutex.release();
+    // Signal
+    eventFlag.set(1);
 }
 
-bool Dispatcher::consume(uint32_t timeout_ticks) {
-    mutex.acquire(TtWaitForever);
-    if (queue.get(&buffer, timeout_ticks) == TtStatusOk) {
-        buffer.callback(buffer.context);
-        mutex.release();
-        return true;
-    } else {
-        mutex.release();
-        return false;
+uint32_t Dispatcher::consume(uint32_t timeout_ticks) {
+    // Wait for signal and clear
+    eventFlag.wait(1, TtFlagWaitAny, timeout_ticks);
+    eventFlag.clear(1);
+
+    // Mutate
+    if (mutex.acquire(1 / portTICK_PERIOD_MS) == TtStatusOk) {
+        auto item = queue.front();
+        queue.pop();
+        // Don't keep lock as callback might be slow
+        tt_check(mutex.release() == TtStatusOk);
+
+        item->callback(item->context);
     }
+
+    return true;
 }
 
 } // namespace

TactilityCore/Source/Dispatcher.h

@@ -7,29 +7,39 @@
 
 #include "MessageQueue.h"
 #include "Mutex.h"
+#include "EventFlag.h"
+#include <memory>
+#include <queue>
 
 namespace tt {
 
-typedef void (*Callback)(void* data);
+typedef void (*Callback)(std::shared_ptr<void> data);
 
 class Dispatcher {
 private:
-    typedef struct {
+    struct DispatcherMessage {
         Callback callback;
-        void* context;
-    } DispatcherMessage;
+        std::shared_ptr<void> context; // Can't use unique_ptr with void, so we use shared_ptr
+
+        DispatcherMessage(Callback callback, std::shared_ptr<void> context) :
+            callback(callback),
+            context(std::move(context))
+        {}
+
+        ~DispatcherMessage() = default;
+    };
 
-    MessageQueue queue;
     Mutex mutex;
-    DispatcherMessage buffer; // Buffer for consuming a message
+    std::queue<std::shared_ptr<DispatcherMessage>> queue;
+    EventFlag eventFlag;
 
 public:
 
-    explicit Dispatcher(size_t queueLimit = 8);
+    explicit Dispatcher();
     ~Dispatcher();
 
-    void dispatch(Callback callback, void* context);
-    bool consume(uint32_t timeout_ticks);
+    void dispatch(Callback callback, std::shared_ptr<void> context);
+    uint32_t consume(uint32_t timeout_ticks);
 };
 
 } // namespace

TactilityCore/Source/Mutex.cpp

@@ -111,6 +111,10 @@ ThreadId Mutex::getOwner() const {
 }
 
 
+std::unique_ptr<ScopedMutexUsage> Mutex::scoped() const {
+    return std::move(std::make_unique<ScopedMutexUsage>(*this));
+}
+
 Mutex* tt_mutex_alloc(MutexType type) {
     return new Mutex(type);
 }
@@ -125,7 +129,6 @@ TtStatus tt_mutex_acquire(Mutex* mutex, uint32_t timeout) {
 
 TtStatus tt_mutex_release(Mutex* mutex) {
     return mutex->release();
-
 }
 
 ThreadId tt_mutex_get_owner(Mutex* mutex) {

TactilityCore/Source/Mutex.h

@@ -7,9 +7,13 @@
 #include "CoreTypes.h"
 #include "Thread.h"
 #include "RtosCompatSemaphore.h"
+#include "Check.h"
+#include <memory>
 
 namespace tt {
 
+class ScopedMutexUsage;
+
 typedef enum {
     MutexTypeNormal,
     MutexTypeRecursive,
@@ -30,6 +34,30 @@ public:
     TtStatus acquire(uint32_t timeout) const;
     TtStatus release() const;
     ThreadId getOwner() const;
+
+    std::unique_ptr<ScopedMutexUsage> scoped() const;
+};
+
+class ScopedMutexUsage {
+
+    const Mutex& mutex;
+    bool acquired = false;
+
+public:
+
+    ScopedMutexUsage(const Mutex& mutex) : mutex(mutex) {}
+
+    ~ScopedMutexUsage() {
+        if (acquired) {
+            tt_check(mutex.release() == TtStatusOk);
+        }
+    }
+
+    bool acquire(uint32_t timeout) {
+        TtStatus result = mutex.acquire(timeout);
+        acquired = (result == TtStatusOk);
+        return acquired;
+    }
 };
 
 /** Allocate Mutex

TactilityCore/Source/Pubsub.cpp

@@ -1,42 +1,11 @@
 #include "Pubsub.h"
 #include "Check.h"
-#include "Mutex.h"
 #include <list>
 
 namespace tt {
 
-struct PubSubSubscription {
-    uint64_t id;
-    PubSubCallback callback;
-    void* callback_context;
-};
-
-typedef std::list<PubSubSubscription> Subscriptions;
-
-struct PubSub {
-    uint64_t last_id = 0;
-    Subscriptions items;
-    Mutex* mutex;
-};
-
-PubSub* tt_pubsub_alloc() {
-    auto* pubsub = new PubSub();
-
-    pubsub->mutex = tt_mutex_alloc(MutexTypeNormal);
-    tt_assert(pubsub->mutex);
-
-    return pubsub;
-}
-
-void tt_pubsub_free(PubSub* pubsub) {
-    tt_assert(pubsub);
-    tt_check(pubsub->items.empty());
-    tt_mutex_free(pubsub->mutex);
-    delete pubsub;
-}
-
-PubSubSubscription* tt_pubsub_subscribe(PubSub* pubsub, PubSubCallback callback, void* callback_context) {
-    tt_check(tt_mutex_acquire(pubsub->mutex, TtWaitForever) == TtStatusOk);
+PubSubSubscription* tt_pubsub_subscribe(std::shared_ptr<PubSub> pubsub, PubSubCallback callback, void* callback_context) {
+    tt_check(pubsub->mutex.acquire(TtWaitForever) == TtStatusOk);
     PubSubSubscription subscription = {
         .id = (++pubsub->last_id),
         .callback = callback,
@@ -46,16 +15,16 @@ PubSubSubscription* tt_pubsub_subscribe(PubSub* pubsub, PubSubCallback callback,
         subscription
     );
 
-    tt_check(tt_mutex_release(pubsub->mutex) == TtStatusOk);
+    tt_check(pubsub->mutex.release() == TtStatusOk);
 
     return (PubSubSubscription*)pubsub->last_id;
 }
 
-void tt_pubsub_unsubscribe(PubSub* pubsub, PubSubSubscription* pubsub_subscription) {
+void tt_pubsub_unsubscribe(std::shared_ptr<PubSub> pubsub, PubSubSubscription* pubsub_subscription) {
     tt_assert(pubsub);
     tt_assert(pubsub_subscription);
 
-    tt_check(tt_mutex_acquire(pubsub->mutex, TtWaitForever) == TtStatusOk);
+    tt_check(pubsub->mutex.acquire(TtWaitForever) == TtStatusOk);
     bool result = false;
     auto id = (uint64_t)pubsub_subscription;
     for (auto it = pubsub->items.begin(); it != pubsub->items.end(); it++) {
@@ -66,19 +35,19 @@ void tt_pubsub_unsubscribe(PubSub* pubsub, PubSubSubscription* pubsub_subscripti
         }
     }
 
-    tt_check(tt_mutex_release(pubsub->mutex) == TtStatusOk);
+    tt_check(pubsub->mutex.release() == TtStatusOk);
     tt_check(result);
 }
 
-void tt_pubsub_publish(PubSub* pubsub, void* message) {
-    tt_check(tt_mutex_acquire(pubsub->mutex, TtWaitForever) == TtStatusOk);
+void tt_pubsub_publish(std::shared_ptr<PubSub> pubsub, void* message) {
+    tt_check(pubsub->mutex.acquire(TtWaitForever) == TtStatusOk);
 
     // Iterate over subscribers
     for (auto& it : pubsub->items) {
         it.callback(message, it.callback_context);
     }
 
-    tt_check(tt_mutex_release(pubsub->mutex) == TtStatusOk);
+    tt_check(pubsub->mutex.release() == TtStatusOk);
 }
 
 } // namespace

TactilityCore/Source/Pubsub.h

@@ -4,30 +4,30 @@
  */
 #pragma once
 
+#include "Mutex.h"
+#include <list>
+
 namespace tt {
 
 /** PubSub Callback type */
 typedef void (*PubSubCallback)(const void* message, void* context);
 
-/** PubSub type */
-typedef struct PubSub PubSub;
+struct PubSubSubscription {
+    uint64_t id;
+    PubSubCallback callback;
+    void* callback_context;
+};
 
-/** PubSubSubscription type */
-typedef struct PubSubSubscription PubSubSubscription;
+struct PubSub {
+    typedef std::list<PubSubSubscription> Subscriptions;
+    uint64_t last_id = 0;
+    Subscriptions items;
+    Mutex mutex;
 
-/** Allocate PubSub
- *
- * Reentrable, Not threadsafe, one owner
- *
- * @return     pointer to PubSub instance
- */
-PubSub* tt_pubsub_alloc();
-
-/** Free PubSub
- * 
- * @param      pubsub  PubSub instance
- */
-void tt_pubsub_free(PubSub* pubsub);
+    ~PubSub() {
+        tt_check(items.empty());
+    }
+};
 
 /** Subscribe to PubSub
  * 
@@ -40,7 +40,7 @@ void tt_pubsub_free(PubSub* pubsub);
  * @return     pointer to PubSubSubscription instance
  */
 PubSubSubscription*
-tt_pubsub_subscribe(PubSub* pubsub, PubSubCallback callback, void* callback_context);
+tt_pubsub_subscribe(std::shared_ptr<PubSub> pubsub, PubSubCallback callback, void* callback_context);
 
 /** Unsubscribe from PubSub
  * 
@@ -50,7 +50,7 @@ tt_pubsub_subscribe(PubSub* pubsub, PubSubCallback callback, void* callback_cont
  * @param      pubsub               pointer to PubSub instance
  * @param      pubsub_subscription  pointer to PubSubSubscription instance
  */
-void tt_pubsub_unsubscribe(PubSub* pubsub, PubSubSubscription* pubsub_subscription);
+void tt_pubsub_unsubscribe(std::shared_ptr<PubSub> pubsub, PubSubSubscription* pubsub_subscription);
 
 /** Publish message to PubSub
  *
@@ -59,6 +59,6 @@ void tt_pubsub_unsubscribe(PubSub* pubsub, PubSubSubscription* pubsub_subscripti
  * @param      pubsub   pointer to PubSub instance
  * @param      message  message pointer to publish
  */
-void tt_pubsub_publish(PubSub* pubsub, void* message);
+void tt_pubsub_publish(std::shared_ptr<PubSub> pubsub, void* message);
 
 } // namespace

TactilityCore/Source/Timer.cpp

@@ -1,4 +1,6 @@
 #include "Timer.h"
+
+#include <utility>
 #include "Check.h"
 #include "Kernel.h"
 #include "RtosCompat.h"
@@ -13,11 +15,11 @@ static void timer_callback(TimerHandle_t hTimer) {
     }
 }
 
-Timer::Timer(Type type, Callback callback, void* callbackContext) {
+Timer::Timer(Type type, Callback callback, std::shared_ptr<void> callbackContext) {
     tt_assert((kernel_is_irq() == 0U) && (callback != nullptr));
 
     this->callback = callback;
-    this->callbackContext = callbackContext;
+    this->callbackContext = std::move(callbackContext);
 
     UBaseType_t reload;
     if (type == TypeOnce) {

TactilityCore/Source/Timer.h

@@ -3,6 +3,7 @@
 #include "CoreTypes.h"
 
 #include "RtosCompatTimers.h"
+#include <memory>
 
 namespace tt {
 
@@ -11,12 +12,12 @@ private:
     TimerHandle_t timerHandle;
 public:
 
-    typedef void (*Callback)(void* context);
+    typedef void (*Callback)(std::shared_ptr<void> context);
     typedef void (*PendingCallback)(void* context, uint32_t arg);
 
 
     Callback callback;
-    void* callbackContext;
+    std::shared_ptr<void> callbackContext;
 
     typedef enum {
         TypeOnce = 0,    ///< One-shot timer.
@@ -28,7 +29,7 @@ public:
      * @param[in] callback The callback function
      * @param callbackContext The callback context
      */
-    Timer(Type type, Callback callback, void* callbackContext);
+    Timer(Type type, Callback callback, std::shared_ptr<void> callbackContext);
 
     ~Timer();
 

TactilityHeadless/Source/TactilityHeadless.cpp

@@ -1,3 +1,4 @@
+#include <Dispatcher.h>
 #include "TactilityHeadless.h"
 #include "hal/Configuration.h"
 #include "hal/Hal_i.h"
@@ -15,6 +16,8 @@ namespace tt {
 namespace service::wifi { extern const ServiceManifest manifest; }
 namespace service::sdcard { extern const ServiceManifest manifest; }
 
+static Dispatcher mainDispatcher;
+
 static const service::ServiceManifest* const system_services[] = {
     &service::sdcard::manifest,
     &service::wifi::manifest
@@ -40,6 +43,11 @@ void initHeadless(const hal::Configuration& config) {
     register_and_start_system_services();
 }
 
+
+Dispatcher& getMainDispatcher() {
+    return mainDispatcher;
+}
+
 namespace hal {
 
 const Configuration& getConfiguration() {

TactilityHeadless/Source/TactilityHeadless.h

@@ -2,11 +2,14 @@
 
 #include "hal/Configuration.h"
 #include "TactilityHeadlessConfig.h"
+#include "Dispatcher.h"
 
 namespace tt {
 
 void initHeadless(const hal::Configuration& config);
 
+Dispatcher& getMainDispatcher();
+
 } // namespace
 
 namespace tt::hal {

TactilityHeadless/Source/service/sdcard/Sdcard.cpp

@@ -1,11 +1,13 @@
-#include <cstdlib>
-
 #include "Mutex.h"
+#include "Timer.h"
+
 #include "service/ServiceContext.h"
 #include "TactilityCore.h"
 #include "TactilityHeadless.h"
 #include "service/ServiceRegistry.h"
 
+#include <cstdlib>
+
 #define TAG "sdcard_service"
 
 namespace tt::service::sdcard {
@@ -15,21 +17,11 @@ extern const ServiceManifest manifest;
 
 struct ServiceData {
     Mutex mutex;
-    Thread thread = Thread(
-        "sdcard",
-        3000, // Minimum is ~2800 @ ESP-IDF 5.1.2 when ejecting sdcard
-        &sdcard_task,
-        nullptr
-    );
+    std::unique_ptr<Timer> updateTimer;
     hal::sdcard::State lastState = hal::sdcard::StateUnmounted;
-    bool interrupted = false;
 
-    ServiceData() {
-        thread.setPriority(Thread::PriorityLow);
-    }
-
-    void lock() const {
-        tt_check(mutex.acquire(TtWaitForever) == TtStatusOk);
+    bool lock(TickType_t timeout) const {
+        return mutex.acquire(timeout) == TtStatusOk;
     }
 
     void unlock() const {
@@ -38,23 +30,14 @@ struct ServiceData {
 };
 
 
-static int32_t sdcard_task(TT_UNUSED void* context) {
-    delay_ms(20); // TODO: Make service instance findable earlier on (but expose "starting" state?)
-    auto service = findServiceById(manifest.id);
-    if (service == nullptr) {
-        TT_LOG_E(TAG, "Service not found");
-        return -1;
+static void onUpdate(std::shared_ptr<void> context) {
+    auto data = std::static_pointer_cast<ServiceData>(context);
+
+    if (!data->lock(50)) {
+        TT_LOG_W(TAG, "Failed to acquire lock");
+        return;
     }
 
-    auto data = std::static_pointer_cast<ServiceData>(service->getData());
-
-    bool interrupted = false;
-
-    do {
-        data->lock();
-
-        interrupted = data->interrupted;
-
     hal::sdcard::State new_state = hal::sdcard::getState();
 
     if (new_state == hal::sdcard::StateError) {
@@ -66,18 +49,17 @@ static int32_t sdcard_task(TT_UNUSED void* context) {
         data->lastState = new_state;
     }
 
-        data->lock();
-        delay_ms(2000);
-    } while (!interrupted);
-
-    return 0;
+    data->unlock();
 }
 
 static void onStart(ServiceContext& service) {
     if (hal::getConfiguration().sdcard != nullptr) {
         auto data = std::make_shared<ServiceData>();
         service.setData(data);
-        data->thread.start();
+
+        data->updateTimer = std::make_unique<Timer>(Timer::TypePeriodic, onUpdate, data);
+        // We want to try and scan more often in case of startup or scan lock failure
+        data->updateTimer->start(1000);
     } else {
         TT_LOG_I(TAG, "task not started due to config");
     }
@@ -85,12 +67,10 @@ static void onStart(ServiceContext& service) {
 
 static void onStop(ServiceContext& service) {
     auto data = std::static_pointer_cast<ServiceData>(service.getData());
-    if (data != nullptr) {
-        data->lock();
-        data->interrupted = true;
-        data->unlock();
-
-        data->thread.join();
+    if (data->updateTimer != nullptr) {
+        // Stop thread
+        data->updateTimer->stop();
+        data->updateTimer = nullptr;
     }
 }
 

TactilityHeadless/Source/service/wifi/Wifi.h

@@ -59,7 +59,7 @@ enum WifiRadioState {
     WIFI_RADIO_CONNECTION_PENDING,
     WIFI_RADIO_CONNECTION_ACTIVE,
     WIFI_RADIO_OFF_PENDING,
-    WIFI_RADIO_OFF
+    WIFI_RADIO_OFF,
 };
 
 struct WifiEvent {
@@ -74,9 +74,9 @@ struct WifiApRecord {
 
 /**
  * @brief Get wifi pubsub
- * @return PubSub*
+ * @return PubSub
  */
-PubSub* getPubsub();
+std::shared_ptr<PubSub> getPubsub();
 
 WifiRadioState getRadioState();
 /**

TactilityHeadless/Source/service/wifi/WifiMock.cpp

@@ -21,7 +21,7 @@ typedef struct {
     /** @brief Locking mechanism for modifying the Wifi instance */
     Mutex* mutex;
     /** @brief The public event bus */
-    PubSub* pubsub;
+    std::shared_ptr<PubSub> pubsub;
     /** @brief The internal message queue */
     MessageQueue queue;
     bool scan_active;
@@ -50,7 +50,7 @@ static void publish_event_simple(Wifi* wifi, WifiEventType type) {
 static Wifi* wifi_alloc() {
     auto* instance = static_cast<Wifi*>(malloc(sizeof(Wifi)));
     instance->mutex = tt_mutex_alloc(MutexTypeRecursive);
-    instance->pubsub = tt_pubsub_alloc();
+    instance->pubsub = std::make_shared<PubSub>();
     instance->scan_active = false;
     instance->radio_state = WIFI_RADIO_CONNECTION_ACTIVE;
     instance->secure_connection = false;
@@ -59,7 +59,6 @@ static Wifi* wifi_alloc() {
 
 static void wifi_free(Wifi* instance) {
     tt_mutex_free(instance->mutex);
-    tt_pubsub_free(instance->pubsub);
     free(instance);
 }
 
@@ -67,7 +66,7 @@ static void wifi_free(Wifi* instance) {
 
 // region Public functions
 
-PubSub* getPubsub() {
+std::shared_ptr<PubSub> getPubsub() {
     tt_assert(wifi);
     return wifi->pubsub;
 }

Tests/TactilityCore/DispatcherTest.cpp

@@ -4,16 +4,26 @@
 
 using namespace tt;
 
-void increment_callback(void* context) {
-    auto* counter = (uint32_t*)context;
-    (*counter)++;
+static uint32_t counter = 0;
+static const uint32_t value_chacker_expected = 123;
+
+void increment_callback(TT_UNUSED std::shared_ptr<void> context) {
+    counter++;
+}
+
+void value_checker(std::shared_ptr<void> context) {
+    auto value = std::static_pointer_cast<uint32_t>(context);
+    if (*value != value_chacker_expected) {
+        tt_crash_implementation();
+    }
 }
 
 TEST_CASE("dispatcher should not call callback if consume isn't called") {
+    counter = 0;
     Dispatcher dispatcher;
 
-    uint32_t counter = 0;
-    dispatcher.dispatch(&increment_callback, &counter);
+    auto context = std::make_shared<uint32_t>();
+    dispatcher.dispatch(&increment_callback, std::move(context));
     delay_ticks(10);
 
     CHECK_EQ(counter, 0);
@@ -21,16 +31,25 @@ TEST_CASE("dispatcher should not call callback if consume isn't called") {
 
 TEST_CASE("dispatcher should be able to dealloc when message is not consumed") {
     auto* dispatcher = new Dispatcher();
-    uint32_t counter = 0;
-    dispatcher->dispatch(increment_callback, &counter);
+    auto context = std::make_shared<uint32_t>();
+    dispatcher->dispatch(increment_callback, std::move(context));
     delete dispatcher;
 }
 
 TEST_CASE("dispatcher should call callback when consume is called") {
+    counter = 0;
     Dispatcher dispatcher;
 
-    uint32_t counter = 0;
-    dispatcher.dispatch(increment_callback, &counter);
+    auto context = std::make_shared<uint32_t>();
+    dispatcher.dispatch(increment_callback, std::move(context));
     dispatcher.consume(100);
     CHECK_EQ(counter, 1);
 }
+
+TEST_CASE("message should be passed on correctly") {
+    Dispatcher dispatcher;
+
+    auto context = std::make_shared<uint32_t>(value_chacker_expected);
+    dispatcher.dispatch(value_checker, std::move(context));
+    dispatcher.consume(100);
+}

Tests/TactilityCore/TimerTest.cpp

@@ -2,32 +2,34 @@
 #include "TactilityCore.h"
 #include "Timer.h"
 
+#include <utility>
+
 using namespace tt;
 
-void* timer_callback_context = NULL;
-static void timer_callback_with_context(void* context) {
-    timer_callback_context = context;
+std::shared_ptr<void> timer_callback_context = NULL;
+static void timer_callback_with_context(std::shared_ptr<void> context) {
+    timer_callback_context = std::move(context);
 }
 
-static void timer_callback_with_counter(void* context) {
-    int* int_ptr = (int*)context;
+static void timer_callback_with_counter(std::shared_ptr<void> context) {
+    auto int_ptr = std::static_pointer_cast<int>(context);
     (*int_ptr)++;
 }
 
 TEST_CASE("a timer passes the context correctly") {
-    int foo = 1;
-    auto* timer = new Timer(Timer::TypeOnce, &timer_callback_with_context, &foo);
+    auto foo = std::make_shared<int>(1);
+    auto* timer = new Timer(Timer::TypeOnce, &timer_callback_with_context, foo);
     timer->start(1);
     delay_ticks(10);
     timer->stop();
     delete timer;
 
-    CHECK_EQ(timer_callback_context, &foo);
+    CHECK_EQ(*std::static_pointer_cast<int>(timer_callback_context), *foo);
 }
 
 TEST_CASE("TimerTypePeriodic timers can be stopped and restarted") {
-    int counter = 0;
-    auto* timer = new Timer(Timer::TypePeriodic, &timer_callback_with_counter, &counter);
+    auto counter = std::make_shared<int>(0);
+    auto* timer = new Timer(Timer::TypePeriodic, &timer_callback_with_counter, counter);
     timer->start(1);
     delay_ticks(10);
     timer->stop();
@@ -36,24 +38,24 @@ TEST_CASE("TimerTypePeriodic timers can be stopped and restarted") {
     timer->stop();
     delete timer;
 
-    CHECK_GE(counter, 2);
+    CHECK_GE(*counter, 2);
 }
 
 TEST_CASE("TimerTypePeriodic calls the callback periodically") {
-    int counter = 0;
+    auto counter = std::make_shared<int>(0);
     int ticks_to_run = 10;
-    auto* timer = new Timer(Timer::TypePeriodic, &timer_callback_with_counter, &counter);
+    auto* timer = new Timer(Timer::TypePeriodic, &timer_callback_with_counter, counter);
     timer->start(1);
     delay_ticks(ticks_to_run);
     timer->stop();
     delete timer;
 
-    CHECK_EQ(counter, ticks_to_run);
+    CHECK_EQ(*counter, ticks_to_run);
 }
 
 TEST_CASE("restarting TimerTypeOnce timers calls the callback again") {
-    int counter = 0;
-    auto* timer = new Timer(Timer::TypeOnce, &timer_callback_with_counter, &counter);
+    auto counter = std::make_shared<int>(0);
+    auto* timer = new Timer(Timer::TypeOnce, &timer_callback_with_counter, counter);
     timer->start(1);
     delay_ticks(10);
     timer->stop();
@@ -62,5 +64,5 @@ TEST_CASE("restarting TimerTypeOnce timers calls the callback again") {
     timer->stop();
     delete timer;
 
-    CHECK_EQ(counter, 2);
+    CHECK_EQ(*counter, 2);
 }