SD card improvements (#214)

- Implement SD card locking logic and helper functions
- Fix issue with running ELF apps from SD card: this would crash when launched from the AppList
- Reduce Boot app wait time to 1 second
- Speed up boot by about 0.1 second by moving app&service registration to the Boot app
- Files app now uses proper SD card mount point name (and multiple SD cards)
- Removed `TT_SCREENSHOT_MODE`

Boards/CYD-2432S024C/Source/hal/YellowSdCard.cpp

@@ -15,7 +15,7 @@ std::shared_ptr<SdCard> createYellowSdCard() {
         GPIO_NUM_NC,
         GPIO_NUM_NC,
         SdCard::MountBehaviour::AtBoot,
-        nullptr,
+        std::make_shared<tt::Mutex>(),
         std::vector<gpio_num_t>(),
         SDCARD_SPI_HOST
     );

Boards/Simulator/Source/hal/SimulatorSdCard.h

@@ -1,6 +1,8 @@
 #pragma once
 
 #include <Tactility/hal/SdCard.h>
+#include <Tactility/Mutex.h>
+#include <memory>
 
 using namespace tt::hal;
 
@@ -9,26 +11,35 @@ class SimulatorSdCard final : public SdCard {
 private:
 
     State state;
+    std::shared_ptr<tt::Lockable> lockable;
+    std::string mountPath;
 
 public:
 
-    SimulatorSdCard() : SdCard(MountBehaviour::AtBoot), state(State::Unmounted) {}
+    SimulatorSdCard() : SdCard(MountBehaviour::AtBoot),
+        state(State::Unmounted),
+        lockable(std::make_shared<tt::Mutex>())
+    {}
 
     std::string getName() const final { return "Mock SD Card"; }
     std::string getDescription() const final { return ""; }
 
-    bool mount(const char* mountPath) override {
+    bool mount(const std::string& newMountPath) final {
         state = State::Mounted;
+        mountPath = newMountPath;
         return true;
     }
 
     bool unmount() override {
         state = State::Unmounted;
+        mountPath = "";
         return true;
     }
 
-    State getState() const override {
-        return state;
-    }
+    std::string getMountPath() const final { return mountPath; };
+
+    std::shared_ptr<tt::Lockable> getLockable() const final { return lockable; }
+
+    State getState() const override { return state; }
 };
 

Documentation/ideas.md

@@ -1,4 +1,5 @@
 # TODOs
+- Split up boot stages, so the last stage can be done from the splash screen
 - Start using non_null (either via MS GSL, or custom)
 - `hal/Configuration.h` defines C function types: Use C++ std::function instead
 - Fix system time to not be 1980 (use build year as minimum)

Tactility/Include/Tactility/TactilityConfig.h

@@ -5,7 +5,6 @@
 #endif
 
 #define TT_CONFIG_FORCE_ONSCREEN_KEYBOARD false // for development/debug purposes
-#define TT_SCREENSHOT_MODE false // for taking screenshots (e.g. forces SD card presence and Files tree on simulator)
 
 #ifdef ESP_PLATFORM
 #define TT_FEATURE_SCREENSHOT_ENABLED (CONFIG_LV_USE_SNAPSHOT == 1 && CONFIG_SPIRAM_USE_MALLOC == 1)

Tactility/Private/Tactility/TactilityPrivate.h

@@ -0,0 +1,9 @@
+#pragma once
+
+#include <Tactility/Tactility.h>
+
+namespace tt {
+
+void initFromBootApp();
+
+}

Tactility/Source/Tactility.cpp

@@ -69,7 +69,6 @@ namespace app {
 static void registerSystemApps() {
     addApp(app::alertdialog::manifest);
     addApp(app::applist::manifest);
-    addApp(app::boot::manifest);
     addApp(app::display::manifest);
     addApp(app::files::manifest);
     addApp(app::gpio::manifest);
@@ -129,6 +128,17 @@ static void registerAndStartUserServices(const std::vector<const service::Servic
     }
 }
 
+void initFromBootApp() {
+    auto configuration = getConfiguration();
+    // Then we register system apps. They are not used/started yet.
+    registerSystemApps();
+    // Then we register and start user services. They are started after system app
+    // registration just in case they want to figure out which system apps are installed.
+    registerAndStartUserServices(configuration->services);
+    // Now we register the user apps, as they might rely on the user services.
+    registerUserApps(configuration->apps);
+}
+
 void run(const Configuration& config) {
     TT_LOG_D(TAG, "run");
 
@@ -142,18 +152,11 @@ void run(const Configuration& config) {
 
     lvgl::init(hardware);
 
-    // Note: the order of starting apps and services is critical!
-    // System services are registered first so the apps below can find them if needed
     registerAndStartSystemServices();
-    // Then we register system apps. They are not used/started yet.
-    registerSystemApps();
-    // Then we register and start user services. They are started after system app
-    // registration just in case they want to figure out which system apps are installed.
-    registerAndStartUserServices(config.services);
-    // Now we register the user apps, as they might rely on the user services.
-    registerUserApps(config.apps);
 
-    TT_LOG_I(TAG, "init starting desktop app");
+    TT_LOG_I(TAG, "starting boot app");
+    // The boot app takes care of registering system apps, user services and user apps
+    addApp(app::boot::manifest);
     service::loader::startApp(app::boot::manifest.id);
 
     TT_LOG_I(TAG, "init complete");

Tactility/Source/app/ElfApp.cpp

@@ -6,6 +6,7 @@
 
 #include <Tactility/Log.h>
 #include <Tactility/StringUtils.h>
+#include <Tactility/hal/SdCard.h>
 
 #include "esp_elf.h"
 
@@ -49,7 +50,10 @@ private:
         assert(elfFileData == nullptr);
 
         size_t size = 0;
-        elfFileData = file::readBinary(filePath, size);
+        hal::withSdCardLock<void>(filePath, [this, &size](){
+            elfFileData = file::readBinary(filePath, size);
+        });
+
         if (elfFileData == nullptr) {
             return false;
         }

Tactility/Source/app/boot/Boot.cpp

@@ -5,7 +5,7 @@
 #include "Tactility/service/loader/Loader.h"
 #include "Tactility/lvgl/Style.h"
 
-#include <Tactility/Tactility.h>
+#include <Tactility/TactilityPrivate.h>
 #include <Tactility/hal/Display.h>
 #include <Tactility/hal/usb/Usb.h>
 #include <Tactility/kernel/SystemEvents.h>
@@ -51,10 +51,13 @@ private:
             hal::usb::resetUsbBootMode();
             hal::usb::startMassStorageWithSdmmc();
         } else {
+            initFromBootApp();
+
             TickType_t end_time = tt::kernel::getTicks();
             TickType_t ticks_passed = end_time - start_time;
             TickType_t minimum_ticks = (CONFIG_TT_SPLASH_DURATION / portTICK_PERIOD_MS);
             if (minimum_ticks > ticks_passed) {
+                TT_LOG_I(TAG, "Remaining delay: %lu", minimum_ticks - ticks_passed);
                 kernel::delayTicks(minimum_ticks - ticks_passed);
             }
 

Tactility/Source/app/files/State.cpp

@@ -3,10 +3,10 @@
 
 #include <Tactility/Log.h>
 #include <Tactility/Partitions.h>
-#include <Tactility/TactilityHeadless.h>
 #include <Tactility/hal/SdCard.h>
 #include <Tactility/kernel/Kernel.h>
 
+#include <cstring>
 #include <unistd.h>
 
 #define TAG "files_app"
@@ -44,11 +44,7 @@ bool State::setEntriesForPath(const std::string& path) {
      * ESP32 does not have a root directory, so we have to create it manually.
      * We'll add the NVS Flash partitions and the binding for the sdcard.
      */
-#if TT_SCREENSHOT_MODE
-    bool show_custom_root = true;
-#else
     bool show_custom_root = (kernel::getPlatform() == kernel::PlatformEsp) && (path == "/");
-#endif
     if (show_custom_root) {
         TT_LOG_I(TAG, "Setting custom root");
         dir_entries.clear();
@@ -63,21 +59,21 @@ bool State::setEntriesForPath(const std::string& path) {
             .d_name = DATA_PARTITION_NAME
         });
 
-#ifndef TT_SCREENSHOT_MODE
-        auto sdcard = tt::hal::getConfiguration()->sdcard;
-        if (sdcard != nullptr) {
+        auto sdcards = tt::hal::findDevices<hal::SdCard>(hal::Device::Type::SdCard);
+        for (auto& sdcard : sdcards) {
             auto state = sdcard->getState();
             if (state == hal::SdCard::State::Mounted) {
-#endif
-                dir_entries.push_back({
+                auto mount_name = sdcard->getMountPath().substr(1);
+                auto dir_entry = dirent {
                     .d_ino = 2,
                     .d_type = TT_DT_DIR,
-                    .d_name = TT_SDCARD_MOUNT_NAME
-                });
-#ifndef TT_SCREENSHOT_MODE
+                    .d_name = { 0 }
+                };
+                assert(mount_name.length() < sizeof(dirent::d_name));
+                strcpy(dir_entry.d_name, mount_name.c_str());
+                dir_entries.push_back(dir_entry);
             }
         }
-#endif
 
         current_path = path;
         selected_child_entry = "";

TactilityHeadless/Include/Tactility/hal/Device.h

@@ -5,6 +5,7 @@
 #include <ranges>
 #include <string>
 #include <vector>
+#include <cassert>
 
 namespace tt::hal {
 

TactilityHeadless/Include/Tactility/hal/SdCard.h

@@ -6,9 +6,6 @@
 
 namespace tt::hal {
 
-#define TT_SDCARD_MOUNT_NAME "sdcard"
-#define TT_SDCARD_MOUNT_POINT "/sdcard"
-
 class SdCard : public Device {
 
 public:
@@ -36,12 +33,35 @@ public:
 
     Type getType() const final { return Type::SdCard; };
 
-    virtual bool mount(const char* mountPath) = 0;
+    virtual bool mount(const std::string& mountPath) = 0;
     virtual bool unmount() = 0;
     virtual State getState() const = 0;
+    /** Return empty string when not mounted or the mount path if mounted */
+    virtual std::string getMountPath() const = 0;
+
+    virtual std::shared_ptr<Lockable> getLockable() const = 0;
 
     virtual MountBehaviour getMountBehaviour() const { return mountBehaviour; }
     bool isMounted() const { return getState() == State::Mounted; }
 };
 
-} // namespace
+/** Return the SdCard device if the path is within the SdCard mounted path (path std::string::starts_with() check)*/
+std::shared_ptr<SdCard> _Nullable findSdCard(const std::string& path);
+
+/**
+ * Acquires an SD card lock if the path is an SD card path.
+ * Always calls the function, but doesn't lock if the path is not an SD card path.
+ */
+template<typename ReturnType>
+inline ReturnType withSdCardLock(const std::string& path, std::function<ReturnType()> fn) {
+    auto sdcard = hal::findSdCard(path);
+    std::unique_ptr<ScopedLockableUsage> scoped_lockable;
+    if (sdcard != nullptr) {
+        scoped_lockable = sdcard->getLockable()->scoped();
+        scoped_lockable->lock(portMAX_DELAY);
+    }
+
+    return fn();
+}
+
+} // namespace tt::hal

TactilityHeadless/Include/Tactility/hal/SpiSdCard.h

@@ -15,7 +15,7 @@ namespace tt::hal {
 /**
  * SD card interface at the default SPI interface
  */
-class SpiSdCard : public tt::hal::SdCard {
+class SpiSdCard final : public tt::hal::SdCard {
 public:
     struct Config {
         Config(
@@ -24,7 +24,7 @@ public:
             gpio_num_t spiPinWp,
             gpio_num_t spiPinInt,
             MountBehaviour mountBehaviourAtBoot,
-            std::shared_ptr<Lockable> lockable = nullptr,
+            std::shared_ptr<Lockable> lockable = std::make_shared<Mutex>(),
             std::vector<gpio_num_t> csPinWorkAround = std::vector<gpio_num_t>(),
             spi_host_device_t spiHost = SPI2_HOST,
             int spiFrequencyKhz = SDMMC_FREQ_DEFAULT
@@ -37,7 +37,9 @@ public:
             lockable(std::move(lockable)),
             csPinWorkAround(std::move(csPinWorkAround)),
             spiHost(spiHost)
-        {}
+        {
+            assert(this->lockable != nullptr);
+        }
 
         int spiFrequencyKhz;
         gpio_num_t spiPinCs; // Clock
@@ -56,12 +58,12 @@ public:
 
 private:
 
-    std::string mountPoint;
+    std::string mountPath;
     sdmmc_card_t* card = nullptr;
     std::shared_ptr<Config> config;
 
     bool applyGpioWorkAround();
-    bool mountInternal(const char* mount_point);
+    bool mountInternal(const std::string& mountPath);
 
 public:
 
@@ -73,8 +75,12 @@ public:
     std::string getName() const final { return "SD Card"; }
     std::string getDescription() const final { return "SD card via SPI interface"; }
 
-    bool mount(const char* mountPath) override;
-    bool unmount() override;
+    bool mount(const std::string& mountPath) final;
+    bool unmount() final;
+    std::string getMountPath() const final { return mountPath; }
+
+    std::shared_ptr<Lockable> getLockable() const final { return config->lockable; }
+
     State getState() const override;
 
     sdmmc_card_t* _Nullable getCard() { return card; }

TactilityHeadless/Source/hal/Hal.cpp

@@ -9,6 +9,8 @@
 
 #define TAG "hal"
 
+#define TT_SDCARD_MOUNT_POINT "/sdcard"
+
 namespace tt::hal {
 
 void init(const Configuration& configuration) {

TactilityHeadless/Source/hal/SdCard.cpp

@@ -0,0 +1,17 @@
+#include "Tactility/hal/SdCard.h"
+#include "Tactility/hal/Device.h"
+
+namespace tt::hal {
+
+std::shared_ptr<SdCard> _Nullable findSdCard(const std::string& path) {
+    auto sdcards = findDevices<SdCard>(Device::Type::SdCard);
+    for (auto& sdcard : sdcards) {
+        if (sdcard->isMounted() && path.starts_with(sdcard->getMountPath())) {
+            return sdcard;
+        }
+    }
+
+    return nullptr;
+}
+
+}

TactilityHeadless/Source/hal/SpiSdCard.cpp

@@ -50,8 +50,8 @@ bool SpiSdCard::applyGpioWorkAround() {
     return true;
 }
 
-bool SpiSdCard::mountInternal(const char* mountPoint) {
-    TT_LOG_I(TAG, "Mounting %s", mountPoint);
+bool SpiSdCard::mountInternal(const std::string& newMountPath) {
+    TT_LOG_I(TAG, "Mounting %s", newMountPath.c_str());
 
     esp_vfs_fat_sdmmc_mount_config_t mount_config = {
         .format_if_mount_failed = config->formatOnMountFailed,
@@ -76,7 +76,7 @@ bool SpiSdCard::mountInternal(const char* mountPoint) {
     host.max_freq_khz = config->spiFrequencyKhz;
     host.slot = config->spiHost;
 
-    esp_err_t result = esp_vfs_fat_sdspi_mount(mountPoint, &host, &slot_config, &mount_config, &card);
+    esp_err_t result = esp_vfs_fat_sdspi_mount(newMountPath.c_str(), &host, &slot_config, &mount_config, &card);
 
     if (result != ESP_OK) {
         if (result == ESP_FAIL) {
@@ -87,22 +87,22 @@ bool SpiSdCard::mountInternal(const char* mountPoint) {
         return false;
     }
 
-    this->mountPoint = mountPoint;
+    mountPath = newMountPath;
 
     return true;
 }
 
-bool SpiSdCard::mount(const char* mount_point) {
+bool SpiSdCard::mount(const std::string& newMountPath) {
     if (!applyGpioWorkAround()) {
         TT_LOG_E(TAG, "Failed to set SPI CS pins high. This is a pre-requisite for mounting.");
         return false;
     }
 
-    if (mountInternal(mount_point)) {
+    if (mountInternal(newMountPath)) {
         sdmmc_card_print_info(stdout, card);
         return true;
     } else {
-        TT_LOG_E(TAG, "Mount failed for %s", mount_point);
+        TT_LOG_E(TAG, "Mount failed for %s", newMountPath.c_str());
         return false;
     }
 }
@@ -113,12 +113,12 @@ bool SpiSdCard::unmount() {
         return false;
     }
 
-    if (esp_vfs_fat_sdcard_unmount(mountPoint.c_str(), card) == ESP_OK) {
-        mountPoint = "";
+    if (esp_vfs_fat_sdcard_unmount(mountPath.c_str(), card) == ESP_OK) {
+        mountPath = "";
         card = nullptr;
         return true;
     } else {
-        TT_LOG_E(TAG, "Unmount failed for %s", mountPoint.c_str());
+        TT_LOG_E(TAG, "Unmount failed for %s", mountPath.c_str());
         return false;
     }
 }