From a7a3b17ff6f2828d36b8636e5e73774bef836455 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sun, 9 Feb 2025 12:01:01 +0100 Subject: [PATCH] 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` --- .../CYD-2432S024C/Source/hal/YellowSdCard.cpp | 2 +- Boards/Simulator/Source/hal/SimulatorSdCard.h | 21 +++++++++---- Documentation/ideas.md | 1 + Tactility/Include/Tactility/TactilityConfig.h | 1 - .../Private/Tactility/TactilityPrivate.h | 9 ++++++ Tactility/Source/Tactility.cpp | 25 +++++++++------- Tactility/Source/app/ElfApp.cpp | 6 +++- Tactility/Source/app/boot/Boot.cpp | 5 +++- Tactility/Source/app/files/State.cpp | 24 +++++++-------- .../Include/Tactility/hal/Device.h | 1 + .../Include/Tactility/hal/SdCard.h | 30 +++++++++++++++---- .../Include/Tactility/hal/SpiSdCard.h | 20 ++++++++----- TactilityHeadless/Source/hal/Hal.cpp | 2 ++ TactilityHeadless/Source/hal/SdCard.cpp | 17 +++++++++++ TactilityHeadless/Source/hal/SpiSdCard.cpp | 20 ++++++------- 15 files changed, 128 insertions(+), 56 deletions(-) create mode 100644 Tactility/Private/Tactility/TactilityPrivate.h create mode 100644 TactilityHeadless/Source/hal/SdCard.cpp diff --git a/Boards/CYD-2432S024C/Source/hal/YellowSdCard.cpp b/Boards/CYD-2432S024C/Source/hal/YellowSdCard.cpp index e1fc5e5d..c465a7a5 100644 --- a/Boards/CYD-2432S024C/Source/hal/YellowSdCard.cpp +++ b/Boards/CYD-2432S024C/Source/hal/YellowSdCard.cpp @@ -15,7 +15,7 @@ std::shared_ptr createYellowSdCard() { GPIO_NUM_NC, GPIO_NUM_NC, SdCard::MountBehaviour::AtBoot, - nullptr, + std::make_shared(), std::vector(), SDCARD_SPI_HOST ); diff --git a/Boards/Simulator/Source/hal/SimulatorSdCard.h b/Boards/Simulator/Source/hal/SimulatorSdCard.h index f7ab367a..e9940806 100644 --- a/Boards/Simulator/Source/hal/SimulatorSdCard.h +++ b/Boards/Simulator/Source/hal/SimulatorSdCard.h @@ -1,6 +1,8 @@ #pragma once #include +#include +#include using namespace tt::hal; @@ -9,26 +11,35 @@ class SimulatorSdCard final : public SdCard { private: State state; + std::shared_ptr lockable; + std::string mountPath; public: - SimulatorSdCard() : SdCard(MountBehaviour::AtBoot), state(State::Unmounted) {} + SimulatorSdCard() : SdCard(MountBehaviour::AtBoot), + state(State::Unmounted), + lockable(std::make_shared()) + {} 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 getLockable() const final { return lockable; } + + State getState() const override { return state; } }; diff --git a/Documentation/ideas.md b/Documentation/ideas.md index daf3702e..3107f349 100644 --- a/Documentation/ideas.md +++ b/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) diff --git a/Tactility/Include/Tactility/TactilityConfig.h b/Tactility/Include/Tactility/TactilityConfig.h index 0f37cdea..33d00898 100644 --- a/Tactility/Include/Tactility/TactilityConfig.h +++ b/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) diff --git a/Tactility/Private/Tactility/TactilityPrivate.h b/Tactility/Private/Tactility/TactilityPrivate.h new file mode 100644 index 00000000..7d57fd8a --- /dev/null +++ b/Tactility/Private/Tactility/TactilityPrivate.h @@ -0,0 +1,9 @@ +#pragma once + +#include + +namespace tt { + +void initFromBootApp(); + +} diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index 6bd5eab4..d236f6ce 100644 --- a/Tactility/Source/Tactility.cpp +++ b/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::vectorservices); + // 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"); diff --git a/Tactility/Source/app/ElfApp.cpp b/Tactility/Source/app/ElfApp.cpp index 51617729..e6c20a20 100644 --- a/Tactility/Source/app/ElfApp.cpp +++ b/Tactility/Source/app/ElfApp.cpp @@ -6,6 +6,7 @@ #include #include +#include #include "esp_elf.h" @@ -49,7 +50,10 @@ private: assert(elfFileData == nullptr); size_t size = 0; - elfFileData = file::readBinary(filePath, size); + hal::withSdCardLock(filePath, [this, &size](){ + elfFileData = file::readBinary(filePath, size); + }); + if (elfFileData == nullptr) { return false; } diff --git a/Tactility/Source/app/boot/Boot.cpp b/Tactility/Source/app/boot/Boot.cpp index 9a011076..ecbac91d 100644 --- a/Tactility/Source/app/boot/Boot.cpp +++ b/Tactility/Source/app/boot/Boot.cpp @@ -5,7 +5,7 @@ #include "Tactility/service/loader/Loader.h" #include "Tactility/lvgl/Style.h" -#include +#include #include #include #include @@ -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); } diff --git a/Tactility/Source/app/files/State.cpp b/Tactility/Source/app/files/State.cpp index 6c13e716..cd924b2a 100644 --- a/Tactility/Source/app/files/State.cpp +++ b/Tactility/Source/app/files/State.cpp @@ -3,10 +3,10 @@ #include #include -#include #include #include +#include #include #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::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 = ""; diff --git a/TactilityHeadless/Include/Tactility/hal/Device.h b/TactilityHeadless/Include/Tactility/hal/Device.h index fe2cbeed..67a69646 100644 --- a/TactilityHeadless/Include/Tactility/hal/Device.h +++ b/TactilityHeadless/Include/Tactility/hal/Device.h @@ -5,6 +5,7 @@ #include #include #include +#include namespace tt::hal { diff --git a/TactilityHeadless/Include/Tactility/hal/SdCard.h b/TactilityHeadless/Include/Tactility/hal/SdCard.h index cfbb44e3..3a027683 100644 --- a/TactilityHeadless/Include/Tactility/hal/SdCard.h +++ b/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 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 _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 +inline ReturnType withSdCardLock(const std::string& path, std::function fn) { + auto sdcard = hal::findSdCard(path); + std::unique_ptr scoped_lockable; + if (sdcard != nullptr) { + scoped_lockable = sdcard->getLockable()->scoped(); + scoped_lockable->lock(portMAX_DELAY); + } + + return fn(); +} + +} // namespace tt::hal diff --git a/TactilityHeadless/Include/Tactility/hal/SpiSdCard.h b/TactilityHeadless/Include/Tactility/hal/SpiSdCard.h index 780b1ca4..2b8bf624 100644 --- a/TactilityHeadless/Include/Tactility/hal/SpiSdCard.h +++ b/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 = nullptr, + std::shared_ptr lockable = std::make_shared(), std::vector csPinWorkAround = std::vector(), 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; 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 getLockable() const final { return config->lockable; } + State getState() const override; sdmmc_card_t* _Nullable getCard() { return card; } diff --git a/TactilityHeadless/Source/hal/Hal.cpp b/TactilityHeadless/Source/hal/Hal.cpp index 629b8826..12585be2 100644 --- a/TactilityHeadless/Source/hal/Hal.cpp +++ b/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) { diff --git a/TactilityHeadless/Source/hal/SdCard.cpp b/TactilityHeadless/Source/hal/SdCard.cpp new file mode 100644 index 00000000..3542f99e --- /dev/null +++ b/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 _Nullable findSdCard(const std::string& path) { + auto sdcards = findDevices(Device::Type::SdCard); + for (auto& sdcard : sdcards) { + if (sdcard->isMounted() && path.starts_with(sdcard->getMountPath())) { + return sdcard; + } + } + + return nullptr; +} + +} diff --git a/TactilityHeadless/Source/hal/SpiSdCard.cpp b/TactilityHeadless/Source/hal/SpiSdCard.cpp index 4b8d1a7a..5385993c 100644 --- a/TactilityHeadless/Source/hal/SpiSdCard.cpp +++ b/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; } }