From 4b48ec874680880c6850cf10b678372fd2412f01 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Sat, 7 Mar 2026 14:46:42 +0100 Subject: [PATCH] Fixes --- .../source/drivers/esp32_gpio.cpp | 17 +++++++++-- .../source/drivers/esp32_sdmmc.cpp | 28 ++++++++++++++++--- .../source/drivers/esp32_sdmmc_fs.cpp | 1 - .../Include/Tactility/service/wifi/Wifi.h | 2 +- Tactility/Source/hal/sdcard/SdCardDevice.cpp | 1 - .../tactility/drivers/gpio_controller.h | 2 +- .../include/tactility/drivers/wifi.h | 10 +++---- .../source/filesystem/file_system.cpp | 20 ++++++------- 8 files changed, 55 insertions(+), 26 deletions(-) diff --git a/Platforms/platform-esp32/source/drivers/esp32_gpio.cpp b/Platforms/platform-esp32/source/drivers/esp32_gpio.cpp index a4b8098f..d6d5137a 100644 --- a/Platforms/platform-esp32/source/drivers/esp32_gpio.cpp +++ b/Platforms/platform-esp32/source/drivers/esp32_gpio.cpp @@ -146,20 +146,31 @@ static error_t disable_interrupt(GpioDescriptor* descriptor) { } static error_t start(Device* device) { - ESP_LOGI(TAG, "start %s", device->name); + LOG_I(TAG, "start %s", device->name); const Esp32GpioConfig* config = GET_CONFIG(device); auto* internal = new Esp32GpioInternal(); return gpio_controller_init_descriptors(device, config->gpioCount, internal); } static error_t stop(Device* device) { - ESP_LOGI(TAG, "stop %s", device->name); + LOG_I(TAG, "stop %s", device->name); + const Esp32GpioConfig* config = GET_CONFIG(device); auto* internal = static_cast(gpio_controller_get_controller_context(device)); + + // First, remove all ISR handlers to prevent callbacks during cleanup + for (uint8_t i = 0; i < config->gpioCount; ++i) { + gpio_isr_handler_remove(static_cast(i)); + } + + // Then uninstall ISR service if (internal->isr_service_ref_count > 0) { gpio_uninstall_isr_service(); } - delete internal; + + // Now safe to deinit descriptors and delete internal check(gpio_controller_deinit_descriptors(device) == ERROR_NONE); + delete internal; + return ERROR_NONE; } diff --git a/Platforms/platform-esp32/source/drivers/esp32_sdmmc.cpp b/Platforms/platform-esp32/source/drivers/esp32_sdmmc.cpp index 5955de1d..d36fe5ae 100644 --- a/Platforms/platform-esp32/source/drivers/esp32_sdmmc.cpp +++ b/Platforms/platform-esp32/source/drivers/esp32_sdmmc.cpp @@ -67,8 +67,6 @@ struct Esp32SdmmcInternal { void lock() { recursive_mutex_lock(&mutex); } - error_t try_lock(TickType_t timeout) { return recursive_mutex_try_lock(&mutex, timeout); } - void unlock() { recursive_mutex_unlock(&mutex); } }; @@ -78,6 +76,7 @@ static error_t start(Device* device) { auto* data = new (std::nothrow) Esp32SdmmcInternal(); if (!data) return ERROR_OUT_OF_MEMORY; + data->lock(); device_set_driver_data(device, data); auto* sdmmc_config = GET_CONFIG(device); @@ -101,12 +100,20 @@ static error_t start(Device* device) { LOG_E(TAG, "Failed to acquire required one or more pins"); data->cleanup_pins(); device_set_driver_data(device, nullptr); + data->unlock(); delete data; return ERROR_RESOURCE; } data->esp32_sdmmc_fs_handle = esp32_sdmmc_fs_alloc(sdmmc_config, "/sdcard"); - check(data->esp32_sdmmc_fs_handle); + if (!data->esp32_sdmmc_fs_handle) { + data->cleanup_pins(); + device_set_driver_data(device, nullptr); + data->unlock(); + delete data; + return ERROR_OUT_OF_MEMORY; + } + data->file_system = file_system_add(&esp32_sdmmc_fs_api, data->esp32_sdmmc_fs_handle); if (file_system_mount(data->file_system) != ERROR_NONE) { // Error is not recoverable at the time, but it might be recoverable later, @@ -115,16 +122,21 @@ static error_t start(Device* device) { } data->initialized = true; + data->unlock(); return ERROR_NONE; } static error_t stop(Device* device) { LOG_I(TAG, "stop %s", device->name); auto* data = GET_DATA(device); + if (!data) return ERROR_NONE; + + data->lock(); if (file_system_is_mounted(data->file_system)) { if (file_system_unmount(data->file_system) != ERROR_NONE) { LOG_E(TAG, "Failed to unmount SD card filesystem"); + data->unlock(); return ERROR_RESOURCE; } } @@ -138,6 +150,8 @@ static error_t stop(Device* device) { data->cleanup_pins(); device_set_driver_data(device, nullptr); + + data->unlock(); delete data; return ERROR_NONE; } @@ -145,7 +159,13 @@ static error_t stop(Device* device) { sdmmc_card_t* esp32_sdmmc_get_card(Device* device) { if (!device_is_ready(device)) return nullptr; auto* data = GET_DATA(device); - return esp32_sdmmc_fs_get_card(data->esp32_sdmmc_fs_handle); + if (!data) return nullptr; + + data->lock(); + auto* card = esp32_sdmmc_fs_get_card(data->esp32_sdmmc_fs_handle); + data->unlock(); + + return card; } extern Module platform_esp32_module; diff --git a/Platforms/platform-esp32/source/drivers/esp32_sdmmc_fs.cpp b/Platforms/platform-esp32/source/drivers/esp32_sdmmc_fs.cpp index cf4b8611..131c5829 100644 --- a/Platforms/platform-esp32/source/drivers/esp32_sdmmc_fs.cpp +++ b/Platforms/platform-esp32/source/drivers/esp32_sdmmc_fs.cpp @@ -163,7 +163,6 @@ static bool is_mounted(void* data) { static error_t get_path(void* data, char* out_path, size_t out_path_size) { const auto* fs_data = GET_DATA(data); - if (fs_data->card == nullptr) return ERROR_INVALID_STATE; if (fs_data->mount_path.size() >= out_path_size) return ERROR_BUFFER_OVERFLOW; strncpy(out_path, fs_data->mount_path.c_str(), out_path_size); return ERROR_NONE; diff --git a/Tactility/Include/Tactility/service/wifi/Wifi.h b/Tactility/Include/Tactility/service/wifi/Wifi.h index 86a9a61e..2fdfbe6b 100644 --- a/Tactility/Include/Tactility/service/wifi/Wifi.h +++ b/Tactility/Include/Tactility/service/wifi/Wifi.h @@ -29,7 +29,7 @@ typedef enum { WIFI_AUTH_WPA3_EXT_PSK, /**< authenticate mode : WPA3_PSK_EXT_KEY */ WIFI_AUTH_WPA3_EXT_PSK_MIXED_MODE, /**< authenticate mode: WPA3_PSK + WPA3_PSK_EXT_KEY */ WIFI_AUTH_MAX -} wifi_auth_mode_t; +}; #endif namespace tt::service::wifi { diff --git a/Tactility/Source/hal/sdcard/SdCardDevice.cpp b/Tactility/Source/hal/sdcard/SdCardDevice.cpp index 1c147f5b..44fbd284 100644 --- a/Tactility/Source/hal/sdcard/SdCardDevice.cpp +++ b/Tactility/Source/hal/sdcard/SdCardDevice.cpp @@ -46,7 +46,6 @@ SdCardDevice::SdCardDevice(MountBehaviour mountBehaviour) : mountBehaviour(mount } SdCardDevice::~SdCardDevice() { - check(!isMounted()); file_system_remove(fileSystem); } diff --git a/TactilityKernel/include/tactility/drivers/gpio_controller.h b/TactilityKernel/include/tactility/drivers/gpio_controller.h index 82a2552e..44365a96 100644 --- a/TactilityKernel/include/tactility/drivers/gpio_controller.h +++ b/TactilityKernel/include/tactility/drivers/gpio_controller.h @@ -203,7 +203,7 @@ error_t gpio_controller_deinit_descriptors(struct Device* device); /** * Unlike other drivers, a GPIO controller's internal data is created and set by gpio_controller_init_descriptors() - * This means that the specific controller implementation cannot set the device's driver data, as it's already set by the GPIO controller base coded. + * This means that the specific controller implementation cannot set the device's driver data, as it's already set by the GPIO controller base coded * When calling init descriptors, the caller can pass a controller_context, which is an optional pointer that holds the implementation's internal data. * @param device the GPIO controller device * @return the context void pointer diff --git a/TactilityKernel/include/tactility/drivers/wifi.h b/TactilityKernel/include/tactility/drivers/wifi.h index 331ee03b..cd615545 100644 --- a/TactilityKernel/include/tactility/drivers/wifi.h +++ b/TactilityKernel/include/tactility/drivers/wifi.h @@ -30,7 +30,7 @@ enum WifiAuthenticationType { } wifi_auth_mode_t; struct WifiApRecord { - char ssid[32]; + char ssid[33]; // 32 bytes + null terminator int8_t rssi; int32_t channel; enum WifiAuthenticationType authentication_type; @@ -148,9 +148,9 @@ struct WifiApi { error_t (*station_get_ipv4_address)(struct Device* device, char* ipv4); /** - * Get the IPv4 address of the device. - * @param[in] device the device - * @param[out] ipv4 the buffer to store the IPv4 address (must be at least 33 bytes, will be null-terminated) + * Get the SSID of the access point the device is currently connected to. + * @param[in] device the wifi device + * @param[out] ssid the buffer to store the SSID (must be at least 33 bytes, will be null-terminated) * @return ERROR_NONE on success */ error_t (*station_get_target_ssid)(struct Device* device, char* ssid); @@ -159,7 +159,7 @@ struct WifiApi { * Connect to an access point. * @param[in] device the wifi device * @param[in] ssid the SSID of the access point - * @param[in] password the password of the access point (33 characters at most, including null-termination) + * @param[in] password the password of the access point * @param[in] channel the Wi-Fi channel to connect to (0 means "any" / no preference) * @return ERROR_NONE on success */ diff --git a/TactilityKernel/source/filesystem/file_system.cpp b/TactilityKernel/source/filesystem/file_system.cpp index e144ab0a..876e113f 100644 --- a/TactilityKernel/source/filesystem/file_system.cpp +++ b/TactilityKernel/source/filesystem/file_system.cpp @@ -1,9 +1,10 @@ // SPDX-License-Identifier: Apache-2.0 -#include -#include -#include #include +#include +#include +#include +#include // Define the internal FileSystem structure struct FileSystem { @@ -14,13 +15,14 @@ struct FileSystem { // Global list of file systems and its mutex struct FileSystemsLedger { std::vector file_systems; - Mutex mutex {}; + // Use recursive mutex so that file_system_for_each() can lock within the callback + RecursiveMutex mutex {}; - FileSystemsLedger() { mutex_construct(&mutex); } - ~FileSystemsLedger() { mutex_destruct(&mutex); } + FileSystemsLedger() { recursive_mutex_construct(&mutex); } + ~FileSystemsLedger() { recursive_mutex_destruct(&mutex); } - void lock() { mutex_lock(&mutex); } - void unlock() { mutex_unlock(&mutex); } + void lock() { recursive_mutex_lock(&mutex); } + void unlock() { recursive_mutex_unlock(&mutex); } }; static FileSystemsLedger& get_ledger() { @@ -61,11 +63,9 @@ void file_system_remove(FileSystem* fs) { void file_system_for_each(void* callback_context, bool (*callback)(FileSystem* fs, void* context)) { auto& ledger = get_ledger(); ledger.lock(); - for (auto* fs : ledger.file_systems) { if (!callback(fs, callback_context)) break; } - ledger.unlock(); }