From c139300a58aaf66c5bcdf9a3d1a950de308c1e92 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 25 Oct 2025 18:08:46 +0200 Subject: [PATCH] Thread+locking improvements and more (#386) --- Documentation/ideas.md | 3 +- .../Tactility/hal/sdcard/SdCardDevice.h | 22 +++++++- Tactility/Source/Tactility.cpp | 13 ++--- .../Source/hal/sdcard/SdCardMounting.cpp | 10 ++-- Tactility/Source/hal/sdcard/SdmmcDevice.cpp | 6 +++ .../Source/hal/sdcard/SpiSdCardDevice.cpp | 6 +++ Tactility/Source/lvgl/Statusbar.cpp | 54 ++++++++----------- .../service/wifi/WifiBootSplashInit.cpp | 17 +++--- 8 files changed, 75 insertions(+), 56 deletions(-) diff --git a/Documentation/ideas.md b/Documentation/ideas.md index 5614d13b..e1ebe395 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -2,14 +2,13 @@ ## Before release -- App Hub - Fix Wi-Fi password(less) decryption crash - Make better esp_lcd driver (and test all devices) ## Higher Priority - Calculator bugs (see GitHub issue) -- Store last synced timestamp in NVS (see how HTTPS client example app) +- Expose http::download() and main dispatcher to TactiltyC. - External app loading: Check the version of Tactility and check ESP target hardware to check for compatibility Check during installation process, but also when starting (SD card might have old app install from before Tactility OS update) - Make a URL handler. Use it for handling local files. Match file types with apps. diff --git a/Tactility/Include/Tactility/hal/sdcard/SdCardDevice.h b/Tactility/Include/Tactility/hal/sdcard/SdCardDevice.h index 648aff23..845f2da3 100644 --- a/Tactility/Include/Tactility/hal/sdcard/SdCardDevice.h +++ b/Tactility/Include/Tactility/hal/sdcard/SdCardDevice.h @@ -6,6 +6,11 @@ namespace tt::hal::sdcard { +/** + * Warning: getLock() does not have to be used when calling any of the functions of this class. + * The lock is only used for file access on the path where the SD card is mounted. + * This is mainly used when accessing the SD card on a shared SPI bus. + */ class SdCardDevice : public Device { public: @@ -33,15 +38,28 @@ public: Type getType() const final { return Type::SdCard; }; + /** + * Mount the device. + * @param mountPath the path to mount at + * @return true on successful mount + */ virtual bool mount(const std::string& mountPath) = 0; + + /** + * Unmount the device. + * @return true on successful unmount + */ virtual bool unmount() = 0; + virtual State getState(TickType_t timeout = portMAX_DELAY) const = 0; - /** Return empty string when not mounted or the mount path if mounted */ + + /** @return empty string when not mounted or the mount path if mounted */ virtual std::string getMountPath() const = 0; - /** Non-null lock */ + /** @return non-null lock, used by code that wants to access files on the mount path of this SD card */ virtual std::shared_ptr getLock() const = 0; + /** @return the MountBehaviour of this device */ virtual MountBehaviour getMountBehaviour() const { return mountBehaviour; } /** @return true if the SD card was mounted, returns false when it was not or when a timeout happened. */ diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index 4734ffd5..f3958b6d 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -101,6 +101,8 @@ namespace app { // List of all apps excluding Boot app (as Boot app calls this function indirectly) static void registerInternalApps() { + TT_LOG_I(TAG, "Registering internal apps"); + addAppManifest(app::alertdialog::manifest); addAppManifest(app::appdetails::manifest); addAppManifest(app::apphub::manifest); @@ -180,6 +182,8 @@ static void registerInstalledApp(std::string path) { } static void registerInstalledApps(const std::string& path) { + TT_LOG_I(TAG, "Registering apps from %s", path.c_str()); + file::listDirectory(path, [&path](const auto& entry) { auto absolute_path = std::format("{}/{}", path, entry.d_name); if (file::isDirectory(absolute_path)) { @@ -200,18 +204,12 @@ static void registerInstalledAppsFromSdCards() { auto sdcard_devices = hal::findDevices(hal::Device::Type::SdCard); for (const auto& sdcard : sdcard_devices) { if (sdcard->isMounted()) { + TT_LOG_I(TAG, "Registering apps from %s", sdcard->getMountPath().c_str()); registerInstalledAppsFromSdCard(sdcard); } } } -static void registerInstalledAppsFromData() { - auto app_path = "/data/app"; - if (file::isDirectory(app_path)) { - registerInstalledApps(app_path); - } -} - static void registerAndStartSecondaryServices() { TT_LOG_I(TAG, "Registering and starting system services"); addService(service::loader::manifest); @@ -242,7 +240,6 @@ void registerApps() { registerInstalledApps(data_apps_path); } registerInstalledAppsFromSdCards(); - registerInstalledAppsFromData(); } void run(const Configuration& config) { diff --git a/Tactility/Source/hal/sdcard/SdCardMounting.cpp b/Tactility/Source/hal/sdcard/SdCardMounting.cpp index 00bea3cb..8bd81195 100644 --- a/Tactility/Source/hal/sdcard/SdCardMounting.cpp +++ b/Tactility/Source/hal/sdcard/SdCardMounting.cpp @@ -9,12 +9,10 @@ constexpr auto* TAG = "SdCardMounting"; constexpr auto* TT_SDCARD_MOUNT_POINT = "/sdcard"; static void mount(const std::shared_ptr& sdcard, const std::string& path) { - sdcard->getLock()->withLock([&sdcard, &path] { - TT_LOG_I(TAG, "Mounting sdcard at %s", path.c_str()); - if (!sdcard->mount(path)) { - TT_LOG_W(TAG, "SD card mount failed for %s (init can continue)", path.c_str()); - } - }); + TT_LOG_I(TAG, "Mounting sdcard at %s", path.c_str()); + if (!sdcard->mount(path)) { + TT_LOG_W(TAG, "SD card mount failed for %s (init can continue)", path.c_str()); + } } static std::string getMountPath(int index, int count) { diff --git a/Tactility/Source/hal/sdcard/SdmmcDevice.cpp b/Tactility/Source/hal/sdcard/SdmmcDevice.cpp index 0ce79396..8bbb198a 100644 --- a/Tactility/Source/hal/sdcard/SdmmcDevice.cpp +++ b/Tactility/Source/hal/sdcard/SdmmcDevice.cpp @@ -58,6 +58,9 @@ bool SdmmcDevice::mountInternal(const std::string& newMountPath) { } bool SdmmcDevice::mount(const std::string& newMountPath) { + auto lock = getLock()->asScopedLock(); + lock.lock(); + if (mountInternal(newMountPath)) { TT_LOG_I(TAG, "Mounted at %s", newMountPath.c_str()); sdmmc_card_print_info(stdout, card); @@ -69,6 +72,9 @@ bool SdmmcDevice::mount(const std::string& newMountPath) { } bool SdmmcDevice::unmount() { + auto lock = getLock()->asScopedLock(); + lock.lock(); + if (card == nullptr) { TT_LOG_E(TAG, "Can't unmount: not mounted"); return false; diff --git a/Tactility/Source/hal/sdcard/SpiSdCardDevice.cpp b/Tactility/Source/hal/sdcard/SpiSdCardDevice.cpp index 09af8847..2fb682ec 100644 --- a/Tactility/Source/hal/sdcard/SpiSdCardDevice.cpp +++ b/Tactility/Source/hal/sdcard/SpiSdCardDevice.cpp @@ -84,6 +84,9 @@ bool SpiSdCardDevice::mountInternal(const std::string& newMountPath) { } bool SpiSdCardDevice::mount(const std::string& newMountPath) { + auto lock = getLock()->asScopedLock(); + lock.lock(); + if (!applyGpioWorkAround()) { TT_LOG_E(TAG, "Failed to apply GPIO work-around"); return false; @@ -100,6 +103,9 @@ bool SpiSdCardDevice::mount(const std::string& newMountPath) { } bool SpiSdCardDevice::unmount() { + auto lock = getLock()->asScopedLock(); + lock.lock(); + if (card == nullptr) { TT_LOG_E(TAG, "Can't unmount: not mounted"); return false; diff --git a/Tactility/Source/lvgl/Statusbar.cpp b/Tactility/Source/lvgl/Statusbar.cpp index 061b85d7..dd755810 100644 --- a/Tactility/Source/lvgl/Statusbar.cpp +++ b/Tactility/Source/lvgl/Statusbar.cpp @@ -48,14 +48,6 @@ typedef struct { PubSub::SubscriptionHandle pubsub_subscription; } Statusbar; -static bool statusbar_lock(TickType_t timeoutTicks = portMAX_DELAY) { - return statusbar_data.mutex.lock(timeoutTicks); -} - -static bool statusbar_unlock() { - return statusbar_data.mutex.unlock(); -} - static void statusbar_constructor(const lv_obj_class_t* class_p, lv_obj_t* obj); static void statusbar_destructor(const lv_obj_class_t* class_p, lv_obj_t* obj); static void statusbar_event(const lv_obj_class_t* class_p, lv_event_t* event); @@ -111,10 +103,12 @@ static const lv_obj_class_t statusbar_class = { static void statusbar_pubsub_event(Statusbar* statusbar) { TT_LOG_D(TAG, "Update event"); - if (lock(portMAX_DELAY)) { + if (lock(defaultLockTime)) { update_main(statusbar); lv_obj_invalidate(&statusbar->obj); unlock(); + } else { + TT_LOG_W(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "Statusbar"); } } @@ -184,7 +178,7 @@ lv_obj_t* statusbar_create(lv_obj_t* parent) { obj_set_style_bg_invisible(left_spacer); lv_obj_set_flex_grow(left_spacer, 1); - statusbar_lock(portMAX_DELAY); + statusbar_data.mutex.lock(portMAX_DELAY); for (int i = 0; i < STATUSBAR_ICON_LIMIT; ++i) { auto* image = lv_image_create(obj); lv_obj_set_size(image, STATUSBAR_ICON_SIZE, STATUSBAR_ICON_SIZE); @@ -194,7 +188,7 @@ lv_obj_t* statusbar_create(lv_obj_t* parent) { update_icon(image, &(statusbar_data.icons[i])); } - statusbar_unlock(); + statusbar_data.mutex.unlock(); return obj; } @@ -212,11 +206,11 @@ static void update_time(Statusbar* statusbar) { static void update_main(Statusbar* statusbar) { update_time(statusbar); - if (statusbar_lock(200 / portTICK_PERIOD_MS)) { + if (statusbar_data.mutex.lock(200 / portTICK_PERIOD_MS)) { for (int i = 0; i < STATUSBAR_ICON_LIMIT; ++i) { update_icon(statusbar->icons[i], &(statusbar_data.icons[i])); } - statusbar_unlock(); + statusbar_data.mutex.unlock(); } } @@ -236,7 +230,7 @@ static void statusbar_event(TT_UNUSED const lv_obj_class_t* class_p, lv_event_t* } int8_t statusbar_icon_add(const std::string& image) { - statusbar_lock(); + statusbar_data.mutex.lock(); int8_t result = -1; for (int8_t i = 0; i < STATUSBAR_ICON_LIMIT; ++i) { if (!statusbar_data.icons[i].claimed) { @@ -248,8 +242,8 @@ int8_t statusbar_icon_add(const std::string& image) { break; } } + statusbar_data.mutex.unlock(); statusbar_data.pubsub->publish(nullptr); - statusbar_unlock(); return result; } @@ -260,37 +254,35 @@ int8_t statusbar_icon_add() { void statusbar_icon_remove(int8_t id) { TT_LOG_D(TAG, "id %d: remove", id); tt_check(id >= 0 && id < STATUSBAR_ICON_LIMIT); - statusbar_lock(); + statusbar_data.mutex.lock(); StatusbarIcon* icon = &statusbar_data.icons[id]; icon->claimed = false; icon->visible = false; icon->image = ""; + statusbar_data.mutex.unlock(); statusbar_data.pubsub->publish(nullptr); - statusbar_unlock(); } void statusbar_icon_set_image(int8_t id, const std::string& image) { TT_LOG_D(TAG, "id %d: set image %s", id, image.empty() ? "(none)" : image.c_str()); tt_check(id >= 0 && id < STATUSBAR_ICON_LIMIT); - if (statusbar_lock()) { - StatusbarIcon* icon = &statusbar_data.icons[id]; - tt_check(icon->claimed); - icon->image = image; - statusbar_data.pubsub->publish(nullptr); - statusbar_unlock(); - } + statusbar_data.mutex.lock(); + StatusbarIcon* icon = &statusbar_data.icons[id]; + tt_check(icon->claimed); + icon->image = image; + statusbar_data.mutex.unlock(); + statusbar_data.pubsub->publish(nullptr); } void statusbar_icon_set_visibility(int8_t id, bool visible) { TT_LOG_D(TAG, "id %d: set visibility %d", id, visible); tt_check(id >= 0 && id < STATUSBAR_ICON_LIMIT); - if (statusbar_lock()) { - StatusbarIcon* icon = &statusbar_data.icons[id]; - tt_check(icon->claimed); - icon->visible = visible; - statusbar_data.pubsub->publish(nullptr); - statusbar_unlock(); - } + statusbar_data.mutex.lock(); + StatusbarIcon* icon = &statusbar_data.icons[id]; + tt_check(icon->claimed); + icon->visible = visible; + statusbar_data.mutex.unlock(); + statusbar_data.pubsub->publish(nullptr); } } // namespace diff --git a/Tactility/Source/service/wifi/WifiBootSplashInit.cpp b/Tactility/Source/service/wifi/WifiBootSplashInit.cpp index 3ab3e44a..d7aba973 100644 --- a/Tactility/Source/service/wifi/WifiBootSplashInit.cpp +++ b/Tactility/Source/service/wifi/WifiBootSplashInit.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace tt::service::wifi { @@ -115,14 +116,16 @@ static void importWifiApSettings(std::shared_ptr sdca } void bootSplashInit() { - const auto sdcards = hal::findDevices(hal::Device::Type::SdCard); - for (auto& sdcard : sdcards) { - if (sdcard->isMounted()) { - importWifiApSettings(sdcard); - } else { - TT_LOG_W(TAG, "Skipping unmounted SD card %s", sdcard->getMountPath().c_str()); + getMainDispatcher().dispatch([] { + const auto sdcards = hal::findDevices(hal::Device::Type::SdCard); + for (auto& sdcard : sdcards) { + if (sdcard->isMounted()) { + importWifiApSettings(sdcard); + } else { + TT_LOG_W(TAG, "Skipping unmounted SD card %s", sdcard->getMountPath().c_str()); + } } - } + }); } }