diff --git a/Buildscripts/TactilitySDK/CMakeLists.txt b/Buildscripts/TactilitySDK/CMakeLists.txt index 3be1ba3ef..ffa734c57 100644 --- a/Buildscripts/TactilitySDK/CMakeLists.txt +++ b/Buildscripts/TactilitySDK/CMakeLists.txt @@ -2,12 +2,12 @@ idf_component_register( INCLUDE_DIRS "Libraries/TactilityC/include" "Libraries/TactilityKernel/include" - "Libraries/TactilityFreeRtos/include" + "Libraries/TactilityFreeRtos/Include" "Libraries/lvgl/include" "Libraries/minmea/include" "Modules/lvgl-module/include" # DRIVER_INCLUDE_DIRS_PLACEHOLDER - REQUIRES esp_timer + REQUIRES esp_timer app-module crypt-module gps-module lvgl-module lvgl-window-manager-module service-module ) # Regular and core features diff --git a/Buildscripts/TactilitySDK/TactilitySDK.cmake b/Buildscripts/TactilitySDK/TactilitySDK.cmake index 71af31dd7..eb3c64682 100644 --- a/Buildscripts/TactilitySDK/TactilitySDK.cmake +++ b/Buildscripts/TactilitySDK/TactilitySDK.cmake @@ -18,13 +18,19 @@ macro(tactility_project project_name) endif() set(EXTRA_COMPONENT_DIRS - "Libraries/TactilityFreeRtos" - "Modules" - "Drivers" + "${TACTILITY_SDK_PATH}/Libraries/TactilityFreeRtos" + "${TACTILITY_SDK_PATH}/Modules" + "${TACTILITY_SDK_PATH}/Drivers" ) set(COMPONENTS TactilityFreeRtos + app-module + crypt-module + gps-module + lvgl-module + lvgl-window-manager-module + service-module # DRIVER_COMPONENTS_PLACEHOLDER ) diff --git a/Buildscripts/release-sdk.py b/Buildscripts/release-sdk.py index 338546621..2aadf72c6 100644 --- a/Buildscripts/release-sdk.py +++ b/Buildscripts/release-sdk.py @@ -167,7 +167,7 @@ def main(): {'src': 'TactilityC/CMakeLists.txt', 'dst': 'Libraries/TactilityC/'}, {'src': 'TactilityC/LICENSE*.*', 'dst': 'Libraries/TactilityC/'}, # TactilityFreeRtos - {'src': 'TactilityFreeRtos/Include/**', 'dst': 'Libraries/TactilityFreeRtos/include/'}, + {'src': 'TactilityFreeRtos/Include/**', 'dst': 'Libraries/TactilityFreeRtos/Include/'}, {'src': 'TactilityFreeRtos/CMakeLists.txt', 'dst': 'Libraries/TactilityFreeRtos/'}, {'src': 'TactilityFreeRtos/LICENSE*.*', 'dst': 'Libraries/TactilityFreeRtos/'}, # TactilityKernel @@ -197,9 +197,11 @@ def main(): map_copy(mappings, target_path) # Modules - add_module(target_path, "lvgl-module") + add_module(target_path, "app-module") add_module(target_path, "crypt-module") add_module(target_path, "gps-module") + add_module(target_path, "lvgl-module") + add_module(target_path, "lvgl-window-manager-module") add_module(target_path, "service-module") # Drivers - only ones actually built for this target (chip-restricted drivers like diff --git a/CMakeLists.txt b/CMakeLists.txt index 02965b4be..d4604c897 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -99,6 +99,8 @@ if (NOT DEFINED ENV{ESP_IDF_VERSION}) add_subdirectory(Modules/crypt-module) add_subdirectory(Modules/gps-module) add_subdirectory(Modules/service-module) + add_subdirectory(Modules/app-module) + add_subdirectory(Modules/lvgl-window-manager-module) add_subdirectory(Drivers/gps-generic-module) add_subdirectory(Drivers/gps-meshtastic-module) diff --git a/Devices/lilygo-tdeck-plus/device.properties b/Devices/lilygo-tdeck-plus/device.properties index 3aaf6fbba..594b5fc50 100644 --- a/Devices/lilygo-tdeck-plus/device.properties +++ b/Devices/lilygo-tdeck-plus/device.properties @@ -23,3 +23,8 @@ cdn.infoMessage=To put the device into bootloader mode:
1. Press the trackb lvgl.colorDepth=16 sdkconfig.CONFIG_CODEC_DUMMY_SUPPORT=y + +# Fix error "PSRAM space not enough for the Flash instructions" on boot: +sdkconfig.CONFIG_SPIRAM_FETCH_INSTRUCTIONS=n +sdkconfig.CONFIG_SPIRAM_RODATA=n +sdkconfig.CONFIG_SPIRAM_XIP_FROM_PSRAM=n diff --git a/Documentation/ideas.md b/Documentation/ideas.md index b2910b85f..963ffabaa 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -12,13 +12,25 @@ ## Higher Priority +- Move "# Fix error "PSRAM space not enough for the Flash instructions" on boot:" fix from T-Deck and others to device.py +- Make it possible to override stack size for an app via config file (loaded at boot), and make it possible to set preferred memory location (e.g. internal/external) +- Put task stacks in PSRAM when possible. +- Wrap file operations like fopen/fclose with file_mutex +- Add bold fonts for e-ink readability improvement +- Split up Claude instructions: https://code.claude.com/docs/en/memory#import-additional-files + and add https://github.com/multica-ai/andrej-karpathy-skills/blob/main/CLAUDE.md +- Move test projects to their relevant subproject +- tt_alertdialog start() etc is broken as it can't fetch the app instance id. Fetch automatically via thread context? +- Migrate Tactility/Paths.cpp functions to TactilityKernel +- app_manager_find_manifest() should make a copy, not return a pointer. +- Httpd.cpp: warn if running on same CPU core (or task) as UI/LVGL/window manager. - Improve Setup: Show "Step done" screen - Improve Setup: Add keyboard/keypad navigation explanation - display.h API: get_backlight does not change ref counting, but it should - bluetooth: various getters for child devices do not change ref counting, but they should - Improve kernel_init.cpp (and other modules): create driver_ensure_added() and driver_ensure_destructed() - Remove and migrate `Include/Tactility/kernel/Kernel.h` into `tactility/delay.h` -- Drivers/audio-codec-module is not a module. Move it somewhere else. Or make it an actual module. +- Drivers/audio-codec-module is not a module. Move it somewhere else. Or make it an actual modulej. - LilyGO T-Dongle S3: 1 button control, stop auto-launching web server - Core2: support power off via software - Create `#define` for empty module (for modules that fully rely on device.properties and don't define drivers or have start/stop logic) @@ -40,6 +52,7 @@ ## Medium Priority +- Consider using https://github.com/Graphify-Labs/graphify - Implement a LED kernel driver (single colour and RGB, plain GPIO and PWM) - Make USB host driver disabled by default, so it doesn't consume memory - Filtering for apps in App Hub: diff --git a/Modules/app-esp32-module/source/app_esp32_loader_service.cpp b/Modules/app-esp32-module/source/app_esp32_loader_service.cpp index ec3e6e1cd..ee510d818 100644 --- a/Modules/app-esp32-module/source/app_esp32_loader_service.cpp +++ b/Modules/app-esp32-module/source/app_esp32_loader_service.cpp @@ -1,24 +1,27 @@ // SPDX-License-Identifier: Apache-2.0 -#include "../../../TactilityKernel/include/tactility/error.h" -#include "../../../TactilityKernel/include/tactility/filesystem/file_mutex.h" -#include "../../app-module/include/app/loader.h" -#include "../../app-module/include/app/location.h" - +#ifdef ESP_PLATFORM +#include +#endif #include -#include +#include + +#include +#include +#include +#include -#include #include #include #include #include -#include #include #include +constexpr auto* TAG = "app_esp32_loader"; + namespace { /** load()-allocated state, passed back through run()/unload(). */ @@ -34,6 +37,7 @@ error_t read_file(const char* path, uint8_t** out_data, size_t* out_size) { FILE* file = fopen(path, "rb"); if (file == nullptr) { + LOG_E(TAG, "Failed to open %s", path); file_mutex_unlock(&mutex); return ERROR_NOT_FOUND; } @@ -68,19 +72,35 @@ error_t read_file(const char* path, uint8_t** out_data, size_t* out_size) { return ERROR_NONE; } -error_t api_load(AppLocation location, AppRuntime* out_runtime) { - auto* runtime = new (std::nothrow) Esp32AppRuntime(); - if (runtime == nullptr) { - return ERROR_OUT_OF_MEMORY; +// location.location can be either an app's install directory or the .elf file directly; the +// former resolves to the per-target binary at {dir}/elf/{CONFIG_IDF_TARGET}.elf. +std::string resolve_elf_path(const std::string& path) { + if (path.ends_with(".elf")) { + return path; } + return path + "/elf/" + CONFIG_IDF_TARGET + ".elf"; +} +error_t api_load(AppLocation location, AppRuntime* out_runtime) { if (location.type != APP_LOCATION_PATH) { + LOG_E(TAG, "Out of memory"); return ERROR_NOT_SUPPORTED; } + LOG_I(TAG, "Loading %s", static_cast(location.location)); + + auto* runtime = new (std::nothrow) Esp32AppRuntime(); + if (runtime == nullptr) { + LOG_E(TAG, "Out of memory"); + return ERROR_OUT_OF_MEMORY; + } + + auto elf_path = resolve_elf_path(static_cast(location.location)); + size_t size = 0; - error_t read_result = read_file(static_cast(location.location), &runtime->file_data, &size); + error_t read_result = read_file(elf_path.c_str(), &runtime->file_data, &size); if (read_result != ERROR_NONE) { + LOG_E(TAG, "Failed to read file"); delete runtime; return read_result; } @@ -88,13 +108,15 @@ error_t api_load(AppLocation location, AppRuntime* out_runtime) { if (esp_elf_init(&runtime->elf) != ESP_OK) { free(runtime->file_data); delete runtime; + LOG_E(TAG, "Failed to init elf"); return ERROR_RESOURCE; } if (esp_elf_relocate(&runtime->elf, runtime->file_data) != 0) { - esp_elf_deinit(&runtime->elf); + // esp_elf_relocate() already frees elf->pdata/ptext itself on a relocation failure free(runtime->file_data); delete runtime; + LOG_E(TAG, "Failed to map elf"); return ERROR_RESOURCE; } @@ -104,9 +126,6 @@ error_t api_load(AppLocation location, AppRuntime* out_runtime) { int32_t api_run(AppRuntime runtime_ptr, uint32_t /*app_instance_id*/, int argc, char* argv[]) { auto* runtime = static_cast(runtime_ptr); - // A side-loaded ELF's own main() only ever gets a real argc/argv from esp_elf_request()'s - // fixed signature - there's no slot for app_instance_id there, and side-loaded apps don't - // need one yet. return esp_elf_request(&runtime->elf, 0, argc, argv); } diff --git a/Modules/app-module/include/app/instance.h b/Modules/app-module/include/app/instance.h index abd95f346..b8993d82f 100644 --- a/Modules/app-module/include/app/instance.h +++ b/Modules/app-module/include/app/instance.h @@ -1,10 +1,15 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once +#include + #ifdef __cplusplus extern "C" { #endif +/** Identifies a running (or previously running) app instance. 0 is never a valid instance id. */ +typedef uint32_t AppInstanceId; + /** Lifecycle state of a running (or previously running) app instance. Every app instance owns * its own task for its entire lifetime - there is no "saved, task given up" state. */ typedef enum { diff --git a/Modules/app-module/include/app/manager.h b/Modules/app-module/include/app/manager.h index 3a18ad62c..55f6c7943 100644 --- a/Modules/app-module/include/app/manager.h +++ b/Modules/app-module/include/app/manager.h @@ -13,9 +13,6 @@ extern "C" { #endif -/** Identifies a running (or previously running) app instance. 0 is never a valid instance id. */ -typedef uint32_t AppInstanceId; - /** * Register an app manifest. * @retval ERROR_INVALID_ARGUMENT a manifest with the same id is already registered @@ -130,6 +127,26 @@ error_t app_manager_get_topmost_instance_id(AppInstanceId* out_app_instance_id); */ error_t app_manager_get_topmost_app_id(char* buffer, size_t buffer_size); +/** + * Registers @a path as a directory to scan for app manifests - each direct subdirectory of + * @a path is expected to hold a manifest.properties (see app/metadata.h), matching the layout + * app_install() creates ({install dir}/{app_id}/manifest.properties), though this is not + * install/uninstall - it only ever adds/removes manifest registrations, never touches files on + * disk or running instances. No-op if @a path is already registered. Does not scan immediately - + * call app_manager_install_path_scan() to do that. + * @retval ERROR_NONE on success + */ +error_t app_manager_install_path_add(const char* path); + +/** + * Scans every path registered via app_manager_install_path_add(): registers + * (app_manager_add()) any direct subdirectory with a valid manifest.properties that isn't + * already registered, and unregisters (app_manager_remove() only - does not stop it if running, + * does not delete anything) any manifest a previous scan registered whose directory has since + * disappeared. Safe to call repeatedly (e.g. after an SD card is mounted/unmounted). + */ +void app_manager_install_path_scan(void); + #ifdef __cplusplus } #endif diff --git a/Modules/app-module/include/app/metadata.h b/Modules/app-module/include/app/metadata.h index 1f02ac6ea..4b759f4ae 100644 --- a/Modules/app-module/include/app/metadata.h +++ b/Modules/app-module/include/app/metadata.h @@ -39,8 +39,8 @@ struct AppMetadata { */ char app_version_name[APP_METADATA_APP_VERSION_NAME_LENGTH + 1]; - /** The technical version (must be incremented with new releases of the app */ - uint64_t app_version_code = 0; + /** The technical version (must be incremented with new releases of the app) */ + uint64_t app_version_code; }; /** diff --git a/Modules/app-module/include/app/scheduler.h b/Modules/app-module/include/app/scheduler.h new file mode 100644 index 000000000..163c44ec9 --- /dev/null +++ b/Modules/app-module/include/app/scheduler.h @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @return the app_instance_id of whichever app instance's task is calling this (every app + * instance's task stashes it in its own thread-local storage when it starts), or 0 if called + * from a task that isn't a running app instance. An app's own main() typically calls this once, + * near the top, to learn its own instance id - see e.g. app_event_subscribe()/ + * window_manager_create(), both of which need it. + */ +AppInstanceId app_scheduler_current_app_id(void); + +#ifdef __cplusplus +} +#endif diff --git a/Modules/app-module/private/app/private/app_fs.h b/Modules/app-module/private/app/private/app_fs.h new file mode 100644 index 000000000..03b4d77c5 --- /dev/null +++ b/Modules/app-module/private/app/private/app_fs.h @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +// Minimal filesystem helpers shared by app-module internals that need to look at on-disk app +// directories (app_install.cpp, manager.cpp's install-path scan) - app-module may not depend +// upward on Tactility::file, so this is a small local re-implementation (see +// app_metadata_parsing.cpp for the same constraint applied to properties-file loading). + +#include "tactility/filesystem/file_mutex.h" + + +#include +#include +#include +#include +#include + +inline bool app_fs_is_directory(const std::string& path) { + struct stat result {}; + FileMutex file_mutex; + file_mutex_get(&file_mutex, path.c_str()); + file_mutex_lock(&file_mutex); + auto is_dir = stat(path.c_str(), &result) == 0 && S_ISDIR(result.st_mode); + file_mutex_unlock(&file_mutex); + return is_dir; +} + +inline bool app_fs_is_file(const std::string& path) { + FileMutex file_mutex; + file_mutex_get(&file_mutex, path.c_str()); + file_mutex_lock(&file_mutex); + struct stat result {}; + auto retval = stat(path.c_str(), &result) == 0 && S_ISREG(result.st_mode); + file_mutex_unlock(&file_mutex); + return retval; +} + +// Appends the full path of every direct subdirectory of @a path to @a out. No-op (not an error) +// if @a path can't be opened. +inline void app_fs_list_direct_subdirectories(const std::string& path, std::vector& out) { + FileMutex file_mutex; + file_mutex_get(&file_mutex, path.c_str()); + file_mutex_lock(&file_mutex); + DIR* dir = opendir(path.c_str()); + if (dir == nullptr) { + file_mutex_unlock(&file_mutex); + return; + } + + struct dirent* entry; + while ((entry = readdir(dir)) != nullptr) { + if (std::strcmp(entry->d_name, ".") == 0 || std::strcmp(entry->d_name, "..") == 0) { + continue; + } + auto child_path = path + "/" + entry->d_name; + if (app_fs_is_directory(child_path)) { + out.push_back(child_path); + } + } + + closedir(dir); + file_mutex_unlock(&file_mutex); +} diff --git a/Modules/app-module/private/app/private/app_ledger.h b/Modules/app-module/private/app/private/app_ledger.h index 3699706e6..fdac66287 100644 --- a/Modules/app-module/private/app/private/app_ledger.h +++ b/Modules/app-module/private/app/private/app_ledger.h @@ -5,7 +5,8 @@ #include #include -#include +#include +#include #include #include @@ -16,8 +17,9 @@ struct AppInstanceRecord { uint32_t id; const AppManifest* manifest; AppInstanceState state; - /** The kernel thread currently executing AppLoaderApi::run() for this instance; NULL when not running. */ - Thread* thread; + /** The FreeRTOS task currently executing AppLoaderApi::run() for this instance; NULL when + * not running. */ + TaskHandle_t task; /** 0 for a top-level launch (app_manager_start()). Non-zero for a modal child launched via * app_manager_start_for_result() - the instance that receives this child's APP_EVENT_RESULT. */ @@ -40,7 +42,7 @@ inline AppLedger& app_ledger() { } /** Frees a deep-copied argv previously built by app_manager_start_with_parameters()/ - * app_manager_start_for_result() (see app_scheduler.cpp's ThreadContext::argv) - each + * app_manager_start_for_result() (see app_scheduler.cpp's TaskContext::argv) - each * individually heap-allocated string, then the array itself. Safe to call with count == 0 / * values == nullptr (no-op). */ inline void app_ledger_free_arguments(int count, char** values) { diff --git a/Modules/app-module/private/app/private/app_scheduler.h b/Modules/app-module/private/app/private/app_scheduler.h index e7d8cd3b1..6f9069a77 100644 --- a/Modules/app-module/private/app/private/app_scheduler.h +++ b/Modules/app-module/private/app/private/app_scheduler.h @@ -4,16 +4,14 @@ #include #include -#include -#include #include /** * Owns per-app task lifecycle on behalf of app_manager_*(). AppLoaderApi implementations - * stay task-agnostic; all of thread_alloc_full()/thread_start()/thread_join() happen here. - * Every app instance gets its own dedicated task for its entire lifetime - no task is ever - * reused for a different instance. + * stay task-agnostic; all of xTaskCreate()/vTaskDelete() happens here, as a plain FreeRTOS task + * (not TactilityKernel's Thread wrapper). Every app instance gets its own dedicated task for its + * entire lifetime - no task is ever reused for a different instance. */ #ifdef __cplusplus @@ -30,13 +28,15 @@ extern "C" { * taken by the scheduler regardless of outcome (freed once the spawned task's run() returns, or * immediately on a failure to start it) */ -error_t app_scheduler_start(uint32_t app_instance_id, struct AppLocation location, int argc, char* argv[]); +error_t app_scheduler_start(AppInstanceId app_instance_id, struct AppLocation location, int argc, char* argv[]); /** * Permanently stops an app instance (APP_EVENT_CLOSE if it was running), bound-waits for its * task to exit, and removes it from the ledger. */ -error_t app_scheduler_stop(uint32_t app_instance_id, TickType_t join_timeout); +error_t app_scheduler_stop(AppInstanceId app_instance_id, TickType_t join_timeout); + +// app_scheduler_current_app_id() is public - see app/scheduler.h. #ifdef __cplusplus } diff --git a/Modules/app-module/source/app_install.cpp b/Modules/app-module/source/app_install.cpp index f3ebe74c6..3586a4034 100644 --- a/Modules/app-module/source/app_install.cpp +++ b/Modules/app-module/source/app_install.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -37,19 +38,9 @@ std::string last_path_segment(const std::string& path) { return index == std::string::npos ? path : path.substr(index + 1); } -bool is_directory(const std::string& path) { - struct stat result {}; - return stat(path.c_str(), &result) == 0 && S_ISDIR(result.st_mode); -} - -bool is_file(const std::string& path) { - struct stat result {}; - return stat(path.c_str(), &result) == 0 && S_ISREG(result.st_mode); -} - // mkdir -p. bool ensure_directory(const std::string& path) { - if (path.empty() || is_directory(path)) { + if (path.empty() || app_fs_is_directory(path)) { return true; } @@ -62,7 +53,7 @@ bool ensure_directory(const std::string& path) { return false; } - return is_directory(path); + return app_fs_is_directory(path); } bool ensure_directory_recursive(const std::string& path) { @@ -75,19 +66,27 @@ bool ensure_directory_recursive(const std::string& path) { } bool delete_recursively(const std::string& path) { + LOG_D(TAG, "Deleting %s...", path.c_str()); if (path.empty() || path == "/" || path == "." || path == "..") { return true; } - if (is_directory(path)) { + if (app_fs_is_directory(path)) { + LOG_D(TAG, "Deleting dir %s", path.c_str()); + + FileMutex file_mutex; + file_mutex_get(&file_mutex, path.c_str()); + file_mutex_lock(&file_mutex); + DIR* dir = opendir(path.c_str()); if (dir == nullptr) { LOG_E(TAG, "Failed to scan directory %s", path.c_str()); + file_mutex_unlock(&file_mutex); return false; } bool success = true; - struct dirent* entry; + dirent* entry; while (success && (entry = readdir(dir)) != nullptr) { if (std::strcmp(entry->d_name, ".") == 0 || std::strcmp(entry->d_name, "..") == 0) { continue; @@ -97,18 +96,17 @@ bool delete_recursively(const std::string& path) { closedir(dir); if (!success) { + file_mutex_unlock(&file_mutex); return false; } - FileMutex mutex {}; - file_mutex_get(&mutex, path.c_str()); - file_mutex_lock(&mutex); bool result = rmdir(path.c_str()) == 0; - file_mutex_unlock(&mutex); + file_mutex_unlock(&file_mutex); return result; } - if (is_file(path)) { + if (app_fs_is_file(path)) { + LOG_D(TAG, "Deleting file %s", path.c_str()); FileMutex mutex {}; file_mutex_get(&mutex, path.c_str()); file_mutex_lock(&mutex); @@ -117,7 +115,7 @@ bool delete_recursively(const std::string& path) { return result; } - // Doesn't exist - nothing to do. + LOG_D(TAG, "Deleting done"); return true; } @@ -213,6 +211,35 @@ InstallRegistry& install_registry() { return registry; } +// Registers @a app_dir_path (already confirmed to hold a valid manifest.properties, parsed into +// @a metadata) with app_manager_add(), taking ownership of its id/name/path strings. +// @warning Caller must hold install_registry().mutex, and must have already ensured +// @a metadata.app_id isn't already registered (app_manager_add() rejects duplicates, but the +// InstalledAppRecord for the earlier registration would leak since this always inserts fresh). +error_t register_installed_app_locked(const std::string& app_dir_path, const AppMetadata& metadata) { + auto& registry = install_registry(); + + auto record = std::make_unique(); + record->id = metadata.app_id; + record->name = metadata.app_name; + record->path = app_dir_path; + record->manifest = AppManifest { + .id = record->id.c_str(), + .name = record->name.c_str(), + .category = APP_CATEGORY_USER, + .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, + .flags = 0, + }; + + error_t add_result = app_manager_add(&record->manifest); + if (add_result != ERROR_NONE) { + return add_result; + } + + registry.apps[record->id] = std::move(record); + return ERROR_NONE; +} + // Stops every currently-running instance of @a manifest. Collects matching instance ids while // holding the ledger lock, then calls app_manager_stop() on each after releasing it - that call // bound-joins the instance's thread, which must not happen while the ledger mutex (also taken by @@ -312,7 +339,7 @@ error_t app_install(const char* source_path) { } auto manifest_path = staging_path + "/manifest.properties"; - if (!is_file(manifest_path)) { + if (!app_fs_is_file(manifest_path)) { LOG_E(TAG, "Manifest not found at %s", manifest_path.c_str()); delete_recursively(staging_path); return ERROR_INVALID_ARGUMENT; @@ -320,7 +347,7 @@ error_t app_install(const char* source_path) { AppMetadata metadata {}; if (app_metadata_parse(manifest_path.c_str(), &metadata) != ERROR_NONE) { - LOG_W(TAG, "Invalid manifest"); + LOG_E(TAG, "Install failed: invalid manifest"); delete_recursively(staging_path); return ERROR_INVALID_ARGUMENT; } @@ -329,8 +356,18 @@ error_t app_install(const char* source_path) { mutex_lock(®istry.mutex); // Replace any previous install of this app id (mirrors the old install()'s "already - // running/present" handling). + // running/present" handling). uninstall_locked() only clears app_install.cpp's own + // registry - the same app id may instead be registered by app_manager_install_path_scan() + // (manager.cpp's separate registry, scanning this same directory tree), which + // uninstall_locked() doesn't know about. Clear the app-manager registration unconditionally + // too, or app_manager_add() below rejects the re-add as a duplicate. uninstall_locked(metadata.app_id); + if (app_manager_remove(metadata.app_id) != ERROR_NONE) { + LOG_E(TAG, "Install failed: failed to remove existing installation"); + mutex_unlock(®istry.mutex); + delete_recursively(staging_path); + return ERROR_RESOURCE; + } auto final_path = app_parent_path + "/" + metadata.app_id; delete_recursively(final_path); @@ -346,30 +383,12 @@ error_t app_install(const char* source_path) { return ERROR_NOT_FOUND; } - auto record = std::make_unique(); - record->id = metadata.app_id; - record->name = metadata.app_name; - record->path = final_path; - record->manifest = AppManifest { - .id = record->id.c_str(), - .name = record->name.c_str(), - .category = APP_CATEGORY_USER, - .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, - .flags = 0, - }; - - error_t add_result = app_manager_add(&record->manifest); - if (add_result != ERROR_NONE) { - // Only remaining failure mode is a duplicate id - can't happen, uninstall_locked() above - // already removed any previous registration for this exact id. - mutex_unlock(®istry.mutex); - return add_result; - } - - registry.apps[record->id] = std::move(record); + // Only remaining failure mode is a duplicate id - can't happen, uninstall_locked() above + // already removed any previous registration for this exact id. + error_t add_result = register_installed_app_locked(final_path, metadata); mutex_unlock(®istry.mutex); - return ERROR_NONE; + return add_result; } error_t app_uninstall(const char* app_id) { diff --git a/Modules/app-module/source/app_metadata_parsing.cpp b/Modules/app-module/source/app_metadata_parsing.cpp index 79849f44e..208f7ea99 100644 --- a/Modules/app-module/source/app_metadata_parsing.cpp +++ b/Modules/app-module/source/app_metadata_parsing.cpp @@ -57,15 +57,16 @@ bool load_properties(const std::string& path, std::map bool got_first_line = false; while (std::getline(file, line)) { auto trimmed_line = trim(line); - if (!got_first_line) { - out_first_line = trimmed_line; - got_first_line = true; - } if (trimmed_line.empty() || trimmed_line.starts_with("#")) { continue; } + if (!got_first_line) { + out_first_line = trimmed_line; + got_first_line = true; + } + if (trimmed_line.starts_with("[")) { section_prefix = trimmed_line; continue; @@ -123,7 +124,8 @@ bool app_metadata_is_valid_version_name(const std::string& version) { } bool app_metadata_is_valid_version_code(const std::string& version) { - return !version.empty() && validate_string(version, [](char c) { + // 20 digits is the maximum decimal width of uint64_t. + return !version.empty() && version.size() <= 20 && validate_string(version, [](char c) { return std::isdigit(static_cast(c)) != 0; }); } diff --git a/Modules/app-module/source/app_metadata_parsing_v1.cpp b/Modules/app-module/source/app_metadata_parsing_v1.cpp index 1f8d8c5f9..ae759a514 100644 --- a/Modules/app-module/source/app_metadata_parsing_v1.cpp +++ b/Modules/app-module/source/app_metadata_parsing_v1.cpp @@ -1,8 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 #include - #include +#include + #include constexpr auto* TAG = "app_metadata_v1"; @@ -77,9 +78,14 @@ bool app_metadata_parse_v1(const std::map& properties, return false; } - out_metadata.app_version_code = std::stoull(version_code_string); - - // [target] + uint64_t version_code = 0; + const auto* first = version_code_string.data(); + const auto* last = first + version_code_string.size(); + if (std::from_chars(first, last, version_code).ec != std::errc {}) { + LOG_E(TAG, "App version code out of range"); + return false; + } + out_metadata.app_version_code = version_code; // [target] std::string target_sdk; if (!app_metadata_get_value(properties, "[target]sdk", target_sdk)) { diff --git a/Modules/app-module/source/app_metadata_parsing_v2.cpp b/Modules/app-module/source/app_metadata_parsing_v2.cpp index a5facdd2e..fe7a58991 100644 --- a/Modules/app-module/source/app_metadata_parsing_v2.cpp +++ b/Modules/app-module/source/app_metadata_parsing_v2.cpp @@ -1,8 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 #include - #include +#include + #include constexpr auto* TAG = "app_metadata_v2"; @@ -77,7 +78,14 @@ bool app_metadata_parse_v2(const std::map& properties, return false; } - out_metadata.app_version_code = std::stoull(version_code_string); + uint64_t version_code = 0; + const auto* first = version_code_string.data(); + const auto* last = first + version_code_string.size(); + if (std::from_chars(first, last, version_code).ec != std::errc {}) { + LOG_E(TAG, "App version code out of range"); + return false; + } + out_metadata.app_version_code = version_code; // [target] // target diff --git a/Modules/app-module/source/app_scheduler.cpp b/Modules/app-module/source/app_scheduler.cpp index 0c5f711ea..91f3d94b8 100644 --- a/Modules/app-module/source/app_scheduler.cpp +++ b/Modules/app-module/source/app_scheduler.cpp @@ -1,31 +1,45 @@ // SPDX-License-Identifier: Apache-2.0 #include #include - #include +#include #include +#include #include #include -#include +#include +#include #include +#include +#include +#include #include -#define TAG "app_scheduler" +constexpr auto* TAG = "app_scheduler"; + +// Slot 0 is reserved by ESP-IDF's pthread API (see TactilityKernel's Thread wrapper for the +// same convention/comment) - app tasks use slot 1 to stash their own app_instance_id, so any +// code running on an app's own task can retrieve it via app_scheduler_current_app_id() without +// needing it threaded through as a parameter. +constexpr size_t APP_INSTANCE_ID_THREAD_SLOT_INDEX = 1; + +// Matches TactilityKernel's Thread wrapper's THREAD_PRIORITY_NORMAL. +constexpr UBaseType_t APP_TASK_PRIORITY = 4; namespace { -struct ThreadContext { +struct TaskContext { const AppLoaderApi* loader; void* runtime; - uint32_t app_instance_id; + AppInstanceId app_instance_id; int argc; char** argv; }; -void set_state(uint32_t app_instance_id, AppInstanceState state) { +void set_state(AppInstanceId app_instance_id, AppInstanceState state) { auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); auto iterator = ledger.instances.find(app_instance_id); @@ -35,21 +49,21 @@ void set_state(uint32_t app_instance_id, AppInstanceState state) { mutex_unlock(&ledger.mutex); } -Thread* get_thread(uint32_t app_instance_id) { +TaskHandle_t get_task(AppInstanceId app_instance_id) { auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); auto iterator = ledger.instances.find(app_instance_id); - Thread* thread = (iterator != ledger.instances.end()) ? iterator->second.thread : nullptr; + TaskHandle_t task = (iterator != ledger.instances.end()) ? iterator->second.task : nullptr; mutex_unlock(&ledger.mutex); - return thread; + return task; } -void set_thread(uint32_t app_instance_id, Thread* thread) { +void set_task(AppInstanceId app_instance_id, TaskHandle_t task) { auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); auto iterator = ledger.instances.find(app_instance_id); if (iterator != ledger.instances.end()) { - iterator->second.thread = thread; + iterator->second.task = task; } mutex_unlock(&ledger.mutex); } @@ -69,10 +83,10 @@ const AppLoaderApi* find_loader_api(AppLocationType type) { // If this instance was launched via app_manager_start_for_result(), delivers @a result (its // own AppMainFn/AppLoaderApi::run() return value) to its parent. No-op for a top-level instance // (parent_id == 0). -void deliver_result_to_parent_if_any(uint32_t app_instance_id, int32_t result) { +void deliver_result_to_parent_if_any(AppInstanceId app_instance_id, int32_t result) { auto& ledger = app_ledger(); - uint32_t parent_id; + AppInstanceId parent_id; AppEvent event { .type = APP_EVENT_RESULT, .timestamp = 0, .result = {} }; mutex_lock(&ledger.mutex); @@ -91,13 +105,20 @@ void deliver_result_to_parent_if_any(uint32_t app_instance_id, int32_t result) { } } -int32_t thread_main(void* context) { - auto* ctx = static_cast(context); +void app_task_main(void* context) { + auto* ctx = static_cast(context); + + check(pvTaskGetThreadLocalStoragePointer(nullptr, APP_INSTANCE_ID_THREAD_SLOT_INDEX) == nullptr); + vTaskSetThreadLocalStoragePointer(nullptr, APP_INSTANCE_ID_THREAD_SLOT_INDEX, reinterpret_cast(static_cast(ctx->app_instance_id))); + + LOG_I(TAG, "Thread for %d started", ctx->app_instance_id); set_state(ctx->app_instance_id, APP_INSTANCE_STATE_ACTIVE); int32_t result = ctx->loader->run(ctx->runtime, ctx->app_instance_id, ctx->argc, ctx->argv); + vTaskSetThreadLocalStoragePointer(nullptr, APP_INSTANCE_ID_THREAD_SLOT_INDEX, nullptr); + ctx->loader->unload(ctx->runtime); deliver_result_to_parent_if_any(ctx->app_instance_id, result); @@ -108,15 +129,26 @@ int32_t thread_main(void* context) { set_state(ctx->app_instance_id, APP_INSTANCE_STATE_STOPPED); app_ledger_free_arguments(ctx->argc, ctx->argv); + + AppInstanceId app_instance_id = ctx->app_instance_id; delete ctx; - return result; + + LOG_I(TAG, "Thread for %d finished", app_instance_id); + + // Erase the ledger entry before self-deleting + auto& ledger = app_ledger(); + mutex_lock(&ledger.mutex); + ledger.instances.erase(app_instance_id); + mutex_unlock(&ledger.mutex); + + vTaskDelete(nullptr); } } // namespace extern "C" { -error_t app_scheduler_start(uint32_t app_instance_id, AppLocation location, int argc, char* argv[]) { +error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, int argc, char* argv[]) { const AppLoaderApi* loader = find_loader_api(location.type); if (loader == nullptr) { LOG_E(TAG, "No app loader is registered (service '%s' not found)", loader_service_id_for(location.type)); @@ -127,54 +159,54 @@ error_t app_scheduler_start(uint32_t app_instance_id, AppLocation location, int void* runtime = nullptr; error_t load_result = loader->load(location, &runtime); if (load_result != ERROR_NONE) { + LOG_E(TAG, "Failed to load app: %s", error_to_string(load_result)); app_ledger_free_arguments(argc, argv); return load_result; } - auto* context = new (std::nothrow) ThreadContext { loader, runtime, app_instance_id, argc, argv }; + auto* context = new (std::nothrow) TaskContext { loader, runtime, app_instance_id, argc, argv }; if (context == nullptr) { + LOG_E(TAG, "Failed to allocate app"); loader->unload(runtime); app_ledger_free_arguments(argc, argv); return ERROR_OUT_OF_MEMORY; } - // -1 (no affinity) matches the FreeRTOS POSIX/simulator port; ESP-IDF's tskNO_AFFINITY is - // a numerically equivalent SMP-only constant not available in the plain FreeRTOS-Kernel port. - Thread* thread = thread_alloc_full("app", 8192, thread_main, context, -1); - if (thread == nullptr) { + char task_name[16]; + snprintf(task_name, sizeof(task_name), "app_%lu", static_cast(app_instance_id)); + + TaskHandle_t task_handle = nullptr; + // 8192 bytes -> stack depth in words, matching what TactilityKernel's Thread wrapper does + // with the stack size it's given. + BaseType_t create_result = xTaskCreate(app_task_main, task_name, 8192 / sizeof(StackType_t), context, APP_TASK_PRIORITY, &task_handle); + if (create_result != pdPASS) { delete context; loader->unload(runtime); app_ledger_free_arguments(argc, argv); return ERROR_OUT_OF_MEMORY; } - set_thread(app_instance_id, thread); - - error_t start_result = thread_start(thread); - if (start_result != ERROR_NONE) { - set_thread(app_instance_id, nullptr); - thread_free(thread); - delete context; - loader->unload(runtime); - app_ledger_free_arguments(argc, argv); - return start_result; - } + set_task(app_instance_id, task_handle); return ERROR_NONE; } -error_t app_scheduler_stop(uint32_t app_instance_id, TickType_t join_timeout) { - Thread* thread = get_thread(app_instance_id); - if (thread != nullptr) { +error_t app_scheduler_stop(AppInstanceId app_instance_id, TickType_t join_timeout) { + TaskHandle_t task = get_task(app_instance_id); + if (task != nullptr) { AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; app_event_emit(app_instance_id, &event); - if (thread_join(thread, join_timeout, pdMS_TO_TICKS(10)) != ERROR_NONE) { - LOG_W(TAG, "App instance %u did not stop in time", app_instance_id); - return ERROR_TIMEOUT; + // Poll for the task to clear its own ledger entry (see app_task_main()) - plain + // FreeRTOS has no built-in task-join primitive. + TickType_t start_ticks = get_ticks(); + while (get_task(app_instance_id) != nullptr) { + delay_ticks(pdMS_TO_TICKS(10)); + if (get_ticks() - start_ticks > join_timeout) { + LOG_W(TAG, "App instance %u did not stop in time", app_instance_id); + return ERROR_TIMEOUT; + } } - thread_free(thread); - set_thread(app_instance_id, nullptr); } set_state(app_instance_id, APP_INSTANCE_STATE_STOPPED); @@ -187,4 +219,9 @@ error_t app_scheduler_stop(uint32_t app_instance_id, TickType_t join_timeout) { return ERROR_NONE; } +AppInstanceId app_scheduler_current_app_id(void) { + void* value = pvTaskGetThreadLocalStoragePointer(nullptr, APP_INSTANCE_ID_THREAD_SLOT_INDEX); + return reinterpret_cast(value); +} + } // extern "C" diff --git a/Modules/app-module/source/manager.cpp b/Modules/app-module/source/manager.cpp index af26a60e7..3c13821e7 100644 --- a/Modules/app-module/source/manager.cpp +++ b/Modules/app-module/source/manager.cpp @@ -1,12 +1,20 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include + +#include #include #include +#include #include +#include #include +#include +#include +#include #define TAG "app_manager" @@ -197,3 +205,113 @@ error_t app_manager_get_topmost_app_id(char* buffer, size_t buffer_size) { } } // extern "C" + +namespace { + +// Owns the AppManifest (and its id/name/path strings) that app_manager_add() only keeps a +// non-owning pointer to (see app_manager_add()'s contract), for manifests registered by +// app_manager_install_path_scan() specifically - separate from app_install.cpp's own registry, +// since scanning only ever adds/removes manifest registrations and never touches files on disk +// or running instances (unlike app_install()/app_uninstall()). +struct ScannedAppManifest { + std::string id; + std::string name; + std::string path; + AppManifest manifest {}; +}; + +struct InstallPathRegistry { + std::vector paths; + std::unordered_map> scanned; + Mutex mutex {}; + + InstallPathRegistry() { mutex_construct(&mutex); } +}; + +InstallPathRegistry& install_path_registry() { + static InstallPathRegistry registry; + return registry; +} + +} // namespace + +extern "C" { + +error_t app_manager_install_path_add(const char* path) { + auto& registry = install_path_registry(); + mutex_lock(®istry.mutex); + if (std::ranges::find(registry.paths, path) == registry.paths.end()) { + registry.paths.emplace_back(path); + } + mutex_unlock(®istry.mutex); + return ERROR_NONE; +} + +void app_manager_install_path_scan(void) { + auto& registry = install_path_registry(); + + mutex_lock(®istry.mutex); + auto paths_copy = registry.paths; + mutex_unlock(®istry.mutex); + + std::vector found_app_dirs; + for (const auto& root : paths_copy) { + app_fs_list_direct_subdirectories(root, found_app_dirs); + } + + mutex_lock(®istry.mutex); + + for (const auto& app_dir : found_app_dirs) { + auto manifest_path = app_dir + "/manifest.properties"; + if (!app_fs_is_file(manifest_path)) { + continue; + } + + AppMetadata metadata {}; + if (app_metadata_parse(manifest_path.c_str(), &metadata) != ERROR_NONE) { + LOG_W(TAG, "Invalid manifest at %s", manifest_path.c_str()); + continue; + } + + if (registry.scanned.contains(metadata.app_id)) { + continue; // already registered by an earlier scan + } + + auto record = std::make_unique(); + record->id = metadata.app_id; + record->name = metadata.app_name; + record->path = app_dir; + record->manifest = AppManifest { + .id = record->id.c_str(), + .name = record->name.c_str(), + .category = APP_CATEGORY_USER, + .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, + .flags = 0, + }; + + if (app_manager_add(&record->manifest) != ERROR_NONE) { + LOG_E(TAG, "Failed to register app %s (duplicate id?)", record->id.c_str()); + continue; + } + + registry.scanned[record->id] = std::move(record); + } + + // Anything a previous scan registered whose directory has since disappeared (e.g. an SD + // card was removed) just gets unregistered - no file deletion, no touching running + // instances, that's app_install()/app_uninstall()'s job, not scanning's. + std::vector missing_ids; + for (const auto& [id, record] : registry.scanned) { + if (!app_fs_is_directory(record->path)) { + missing_ids.push_back(id); + } + } + for (const auto& id : missing_ids) { + app_manager_remove(id.c_str()); + registry.scanned.erase(id); + } + + mutex_unlock(®istry.mutex); +} + +} // extern "C" diff --git a/Modules/app-module/source/module.cpp b/Modules/app-module/source/module.cpp deleted file mode 100644 index f4811265f..000000000 --- a/Modules/app-module/source/module.cpp +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include - -#include - -#include -#include - -extern "C" { - -extern ServiceManifest app_internal_loader_service_manifest; - -static error_t start() { - return service_manager_add(&app_internal_loader_service_manifest, /*auto_start=*/true); -} - -static error_t stop() { - return service_manager_remove(app_internal_loader_service_manifest.id); -} - -Module app_module = { - .name = "app", - .start = start, - .stop = stop, - .drivers = nullptr, - .symbols = nullptr, - .internal = nullptr -}; - -} diff --git a/Modules/app-module/source/symbols.cpp b/Modules/app-module/source/symbols.cpp new file mode 100644 index 000000000..7a1b38a3a --- /dev/null +++ b/Modules/app-module/source/symbols.cpp @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include + +#include + +#include +#include + +extern "C" { + +extern ServiceManifest app_internal_loader_service_manifest; + +const ModuleSymbol app_module_symbols[] = { + // app/scheduler + DEFINE_MODULE_SYMBOL(app_scheduler_current_app_id), + // app/event + DEFINE_MODULE_SYMBOL(app_event_subscribe), + DEFINE_MODULE_SYMBOL(app_event_unsubscribe), + DEFINE_MODULE_SYMBOL(app_event_emit), + DEFINE_MODULE_SYMBOL(app_event_await), + // app/manager + DEFINE_MODULE_SYMBOL(app_manager_start), + DEFINE_MODULE_SYMBOL(app_manager_start_with_parameters), + DEFINE_MODULE_SYMBOL(app_manager_start_for_result), + DEFINE_MODULE_SYMBOL(app_manager_stop), + DEFINE_MODULE_SYMBOL(app_manager_finish), + DEFINE_MODULE_SYMBOL(app_manager_get_state), + DEFINE_MODULE_SYMBOL(app_manager_find_manifest), + DEFINE_MODULE_SYMBOL(app_manager_for_each_manifest), + // terminator + MODULE_SYMBOL_TERMINATOR +}; + +static error_t start() { + return service_manager_add(&app_internal_loader_service_manifest, /*auto_start=*/true); +} + +static error_t stop() { + return service_manager_remove(app_internal_loader_service_manifest.id); +} + +Module app_module = { + .name = "app", + .start = start, + .stop = stop, + .drivers = nullptr, + .symbols = app_module_symbols, + .internal = nullptr +}; + +} diff --git a/Modules/lvgl-window-manager/CMakeLists.txt b/Modules/lvgl-window-manager-module/CMakeLists.txt similarity index 83% rename from Modules/lvgl-window-manager/CMakeLists.txt rename to Modules/lvgl-window-manager-module/CMakeLists.txt index 7067fcdb4..d8e518d56 100644 --- a/Modules/lvgl-window-manager/CMakeLists.txt +++ b/Modules/lvgl-window-manager-module/CMakeLists.txt @@ -4,7 +4,7 @@ include("${CMAKE_CURRENT_LIST_DIR}/../../Buildscripts/module.cmake") file(GLOB_RECURSE SOURCE_FILES "source/*.c*") -tactility_add_module(lvgl-window-manager +tactility_add_module(lvgl-window-manager-module SRCS ${SOURCE_FILES} INCLUDE_DIRS include/ REQUIRES TactilityKernel lvgl-module diff --git a/Modules/lvgl-window-manager/devicetree.yaml b/Modules/lvgl-window-manager-module/devicetree.yaml similarity index 100% rename from Modules/lvgl-window-manager/devicetree.yaml rename to Modules/lvgl-window-manager-module/devicetree.yaml diff --git a/Modules/lvgl-window-manager/include/lvgl_window_manager/module.h b/Modules/lvgl-window-manager-module/include/lvgl_window_manager/module.h similarity index 100% rename from Modules/lvgl-window-manager/include/lvgl_window_manager/module.h rename to Modules/lvgl-window-manager-module/include/lvgl_window_manager/module.h diff --git a/Modules/lvgl-window-manager/include/lvgl_window_manager/window_manager.h b/Modules/lvgl-window-manager-module/include/lvgl_window_manager/window_manager.h similarity index 100% rename from Modules/lvgl-window-manager/include/lvgl_window_manager/window_manager.h rename to Modules/lvgl-window-manager-module/include/lvgl_window_manager/window_manager.h diff --git a/Modules/lvgl-window-manager/source/module.cpp b/Modules/lvgl-window-manager-module/source/symbols.cpp similarity index 51% rename from Modules/lvgl-window-manager/source/module.cpp rename to Modules/lvgl-window-manager-module/source/symbols.cpp index b93d5010f..b41acce2a 100644 --- a/Modules/lvgl-window-manager/source/module.cpp +++ b/Modules/lvgl-window-manager-module/source/symbols.cpp @@ -7,12 +7,21 @@ extern "C" { +const ModuleSymbol lvgl_window_manager_module_symbols[] = { + DEFINE_MODULE_SYMBOL(window_manager_create), + DEFINE_MODULE_SYMBOL(window_manager_remove), + DEFINE_MODULE_SYMBOL(window_manager_get_state), + DEFINE_MODULE_SYMBOL(window_manager_await_state_change), + // terminator + MODULE_SYMBOL_TERMINATOR +}; + Module lvgl_window_manager_module = { .name = "lvgl-window-manager", .start = window_manager_start, .stop = window_manager_stop, .drivers = nullptr, - .symbols = nullptr, + .symbols = lvgl_window_manager_module_symbols, .internal = nullptr }; diff --git a/Modules/lvgl-window-manager/source/window_manager.cpp b/Modules/lvgl-window-manager-module/source/window_manager.cpp similarity index 74% rename from Modules/lvgl-window-manager/source/window_manager.cpp rename to Modules/lvgl-window-manager-module/source/window_manager.cpp index c2d1e91b3..dda814abe 100644 --- a/Modules/lvgl-window-manager/source/window_manager.cpp +++ b/Modules/lvgl-window-manager-module/source/window_manager.cpp @@ -18,8 +18,17 @@ struct WindowRecord { }; struct WindowManagerState { + /** Mutex for read/write operations. Shortly held. */ Mutex mutex {}; + /** Serializes the full start()/stop() transition (including the LVGL work done with + * `mutex` released) so two concurrent starts can't both pass the `started` check and each + * create their own root widget, and a concurrent stop can't run while a start is still + * mid-flight. Never held across a create_widgets()/screen_init() callback - those only + * reach window_manager_create()/remove(), not start()/stop() - so there's no lock-order + * risk with `mutex` or the LVGL lock. */ + Mutex lifecycle_mutex {}; + bool started = false; WindowManagerScreenInitFn screen_init = nullptr; @@ -38,7 +47,10 @@ struct WindowManagerState { /** Task blocked in window_manager_await_state_change(), if any. */ TaskHandle_t waiting_task = nullptr; - WindowManagerState() { mutex_construct(&mutex); } + WindowManagerState() { + mutex_construct(&mutex); + mutex_construct(&lifecycle_mutex); + } }; WindowManagerState& state() { @@ -86,9 +98,16 @@ void window_manager_configure(WindowManagerScreenInitFn screen_init) { error_t window_manager_start(void) { auto& s = state(); + // Held for the whole transition (including the LVGL work below, done with `mutex` + // released) - blocks a concurrent start() from also passing the `started` check and + // building its own root widget, and blocks a concurrent stop() from running while this + // start is still mid-flight. + mutex_lock(&s.lifecycle_mutex); + mutex_lock(&s.mutex); if (s.started) { mutex_unlock(&s.mutex); + mutex_unlock(&s.lifecycle_mutex); return ERROR_NONE; } WindowManagerScreenInitFn screen_init = s.screen_init; @@ -114,6 +133,7 @@ error_t window_manager_start(void) { lvgl_unlock(); if (real_widget == nullptr) { + mutex_unlock(&s.lifecycle_mutex); return ERROR_RESOURCE; } @@ -123,15 +143,21 @@ error_t window_manager_start(void) { s.started = true; mutex_unlock(&s.mutex); + mutex_unlock(&s.lifecycle_mutex); return ERROR_NONE; } error_t window_manager_stop(void) { auto& s = state(); + // See window_manager_start() - blocks until any in-flight start() has fully completed (or + // failed) before this stop can observe/tear down state. + mutex_lock(&s.lifecycle_mutex); + mutex_lock(&s.mutex); if (!s.started) { mutex_unlock(&s.mutex); + mutex_unlock(&s.lifecycle_mutex); return ERROR_NONE; } lv_obj_t* widget = s.real_root_widget; @@ -151,6 +177,7 @@ error_t window_manager_stop(void) { // Deleting the real widget cascades to everything under it - chrome and top_widget alike. delete_widget(widget); + mutex_unlock(&s.lifecycle_mutex); return ERROR_NONE; } @@ -212,6 +239,7 @@ void window_manager_remove(WindowId id) { void* next_user_data = nullptr; WindowId next_id = 0; bool has_next = false; + TaskHandle_t waiter = nullptr; if (was_topmost) { old_widget = s.top_widget; @@ -222,10 +250,12 @@ void window_manager_remove(WindowId id) { next_id = s.windows.back().id; has_next = true; } + // Only the topmost window's state can actually change here - a waiter blocked in + // window_manager_await_state_change() is always waiting on the current top window (see + // that function), so removing a buried window never affects what it's waiting for. + waiter = s.waiting_task; + s.waiting_task = nullptr; } - - TaskHandle_t waiter = s.waiting_task; - s.waiting_task = nullptr; mutex_unlock(&s.mutex); if (waiter != nullptr) { @@ -273,6 +303,21 @@ WindowState window_manager_await_state_change(WindowId id, TickType_t timeout) { ulTaskNotifyTake(pdTRUE, timeout); + /* Deregister ourselves if a create()/remove() hasn't already claimed us (the ordinary, intended wakeup) + * Otherwise a later create()/remove() could notify a task that's no longer waiting here: + * a use-after-exit on the handle if this task is gone, or a stale wakeup the next time it waits. */ + mutex_lock(&s.mutex); + if (s.waiting_task == xTaskGetCurrentTaskHandle()) { + s.waiting_task = nullptr; + } + mutex_unlock(&s.mutex); + + /* create()/remove() read+clear `waiting_task` under the lock but call xTaskNotifyGive() + * after releasing it, so a notification can still land on us right around the timeout + * boundary regardless of which branch above ran. Drain it now (non-blocking) so it doesn't + * linger and cause a spurious immediate return the next time this task awaits. */ + ulTaskNotifyTake(pdTRUE, 0); + return window_manager_get_state(id); } diff --git a/TactilityKernel/include/tactility/system_event.h b/TactilityKernel/include/tactility/system_event.h index 5b0b82c15..9b4cb967b 100644 --- a/TactilityKernel/include/tactility/system_event.h +++ b/TactilityKernel/include/tactility/system_event.h @@ -6,6 +6,7 @@ #include #include +#include #include #ifdef __cplusplus @@ -143,7 +144,11 @@ struct SystemEventSubscription { /** Event type to subscribe to; set by the caller before system_event_subscribe(). */ enum SystemEventType type; - TaskHandle_t task; + /** Own wakeup signal, not the subscribing task's shared default notification value - a + * task with more than one poll subscription would otherwise have events for one + * subscription wake (and consume the notification meant for) system_event_await() calls + * on another. */ + SemaphoreHandle_t semaphore; uint64_t timestamp; uint8_t data[SYSTEM_EVENT_MAX_DATA_SIZE]; @@ -160,7 +165,10 @@ struct SystemEventSubscription { * @warning Does not work in ISR context. * @param[in,out] sub subscription to register; caller sets @a sub->type beforehand, owns the * storage, and must keep it alive (and stationary) until unsubscribed - * @return ERROR_NONE on success + * @retval ERROR_NONE on success + * @retval ERROR_OUT_OF_MEMORY failed to allocate the subscription's wakeup semaphore; @a sub + * was not registered + * @retval ERROR_INVALID_STATE @a sub is already registered */ error_t system_event_subscribe(struct SystemEventSubscription* sub); diff --git a/TactilityKernel/source/system_event.cpp b/TactilityKernel/source/system_event.cpp index ae649d2f0..679d5e3e1 100644 --- a/TactilityKernel/source/system_event.cpp +++ b/TactilityKernel/source/system_event.cpp @@ -67,9 +67,9 @@ error_t system_event_callback_remove( return result; } -// Copies `data` into every current poll subscriber of `type` and wakes its waiting task. +// Copies `data` into every current poll subscriber of `type` and signals its wakeup semaphore. // Held entirely under the lock: unlike the callback path, this never invokes caller code -// (just a memcpy and an xTaskNotifyGive), so there is nothing that could reenter and deadlock. +// (just a memcpy and a semaphore give), so there is nothing that could reenter and deadlock. static void notify_poll_subscribers( SystemEventType type, uint64_t timestamp, @@ -81,12 +81,13 @@ static void notify_poll_subscribers( for (SystemEventSubscription* sub = poll_subscriptions; sub != nullptr; sub = sub->next) { if (sub->type == type) { sub->timestamp = timestamp; - if (data_len > 0) { - std::memcpy(sub->data, data, std::min(data_len, static_cast(SYSTEM_EVENT_MAX_DATA_SIZE))); + const size_t copied_len = std::min(data_len, SYSTEM_EVENT_MAX_DATA_SIZE); + if (copied_len > 0) { + std::memcpy(sub->data, data, copied_len); } - sub->data_len = data_len; + sub->data_len = copied_len; sub->sequence++; - xTaskNotifyGive(sub->task); + xSemaphoreGive(sub->semaphore); } } @@ -162,14 +163,31 @@ error_t system_event_emit( } error_t system_event_subscribe(SystemEventSubscription* sub) { - sub->task = xTaskGetCurrentTaskHandle(); + SemaphoreHandle_t semaphore = xSemaphoreCreateBinary(); + if (semaphore == nullptr) { + return ERROR_OUT_OF_MEMORY; + } + + mutex_lock(&poll_subscriptions_mutex.handle); + + // Check-and-insert in one critical section: registering the same `sub` twice would link + // it into a list that already contains it, creating a cycle that notify_poll_subscribers() + // would then traverse forever while holding this same mutex. + for (SystemEventSubscription* existing = poll_subscriptions; existing != nullptr; existing = existing->next) { + if (existing == sub) { + mutex_unlock(&poll_subscriptions_mutex.handle); + vSemaphoreDelete(semaphore); + return ERROR_INVALID_STATE; + } + } + + sub->semaphore = semaphore; sub->sequence = 0; sub->consumed_sequence = 0; sub->data_len = 0; - - mutex_lock(&poll_subscriptions_mutex.handle); sub->next = poll_subscriptions; poll_subscriptions = sub; + mutex_unlock(&poll_subscriptions_mutex.handle); return ERROR_NONE; @@ -188,6 +206,13 @@ error_t system_event_unsubscribe(SystemEventSubscription* sub) { } mutex_unlock(&poll_subscriptions_mutex.handle); + if (result == ERROR_NONE) { + // Unlinked first, so notify_poll_subscribers() can no longer reach this semaphore + // before it's deleted. + vSemaphoreDelete(sub->semaphore); + sub->semaphore = nullptr; + } + return result; } @@ -195,7 +220,7 @@ error_t system_event_await(SystemEventSubscription* sub, TickType_t timeout) { uint32_t old_sequence = sub->sequence; while (sub->sequence == old_sequence) { - if (ulTaskNotifyTake(pdTRUE, timeout) == 0) { + if (xSemaphoreTake(sub->semaphore, timeout) == pdFALSE) { return ERROR_TIMEOUT; } } diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index e11ad29d8..56ca74268 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -8,6 +8,7 @@ add_subdirectory(TactilityFreeRtos) add_subdirectory(TactilityKernel) add_subdirectory(Tactility) add_subdirectory(crypt-module) +add_subdirectory(app-module) add_custom_target(build-tests) add_dependencies(build-tests ServiceModuleTests) @@ -15,3 +16,4 @@ add_dependencies(build-tests TactilityFreeRtosTests) add_dependencies(build-tests TactilityTests) add_dependencies(build-tests TactilityKernelTests) add_dependencies(build-tests CryptModuleTests) +add_dependencies(build-tests AppModuleTests) diff --git a/Tests/Tactility/CMakeLists.txt b/Tests/Tactility/CMakeLists.txt index db87316c0..8d1377531 100644 --- a/Tests/Tactility/CMakeLists.txt +++ b/Tests/Tactility/CMakeLists.txt @@ -15,6 +15,8 @@ target_link_libraries(TactilityTests PRIVATE TactilityKernel platform-posix lvgl-module + lvgl-window-manager-module + app-module crypt-module gps-module gps-generic-module diff --git a/Tests/app-module/CMakeLists.txt b/Tests/app-module/CMakeLists.txt new file mode 100644 index 000000000..b58e0e0b7 --- /dev/null +++ b/Tests/app-module/CMakeLists.txt @@ -0,0 +1,19 @@ +project(AppModuleTests) + +enable_language(C CXX ASM) + + +file(GLOB_RECURSE TEST_SOURCES ${PROJECT_SOURCE_DIR}/Source/*.cpp) +add_executable(AppModuleTests EXCLUDE_FROM_ALL ${TEST_SOURCES}) + +target_include_directories(AppModuleTests PRIVATE ${DOCTESTINC}) + +add_test(NAME AppModuleTests COMMAND AppModuleTests) + +target_link_libraries(AppModuleTests PUBLIC + TactilityKernel + app-module + service-module + platform-posix + freertos_kernel +) diff --git a/Tests/app-module/Source/AppEventTest.cpp b/Tests/app-module/Source/AppEventTest.cpp new file mode 100644 index 000000000..e12a51296 --- /dev/null +++ b/Tests/app-module/Source/AppEventTest.cpp @@ -0,0 +1,137 @@ +#include "doctest.h" + +#include + +#include +#include +#include + +TEST_CASE("app_event_subscribe/_await deliver events in FIFO order") { + AppEventSubscription sub {}; + sub.app_instance_id = 1; + CHECK_EQ(app_event_subscribe(&sub), ERROR_NONE); + + for (uint32_t i = 0; i < 3; i++) { + AppEvent event { .type = APP_EVENT_RESULT, .timestamp = 0, .result = { .launch_id = i, .result = 0 } }; + CHECK_EQ(app_event_emit(1, &event), ERROR_NONE); + } + + for (uint32_t i = 0; i < 3; i++) { + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, 0), ERROR_NONE); + CHECK_EQ(out.type, APP_EVENT_RESULT); + CHECK_EQ(out.result.launch_id, i); + } + + app_event_unsubscribe(&sub); +} + +TEST_CASE("app_event_emit only delivers to subscriptions for that app_instance_id") { + AppEventSubscription sub {}; + sub.app_instance_id = 10; + app_event_subscribe(&sub); + + AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; + CHECK_EQ(app_event_emit(11, &event), ERROR_NOT_FOUND); + + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, 0), ERROR_TIMEOUT); + + app_event_unsubscribe(&sub); +} + +TEST_CASE("app_event_emit returns ERROR_RESOURCE and drops the newest event once a subscription's queue is full") { + AppEventSubscription sub {}; + sub.app_instance_id = 20; + app_event_subscribe(&sub); + + for (uint32_t i = 0; i < APP_EVENT_QUEUE_CAPACITY; i++) { + AppEvent event { .type = APP_EVENT_RESULT, .timestamp = 0, .result = { .launch_id = i, .result = 0 } }; + CHECK_EQ(app_event_emit(20, &event), ERROR_NONE); + } + + // Queue is now full; this one should be dropped. + AppEvent overflow_event { .type = APP_EVENT_RESULT, .timestamp = 0, .result = { .launch_id = 999, .result = 0 } }; + CHECK_EQ(app_event_emit(20, &overflow_event), ERROR_RESOURCE); + + // The already-queued events survive, in order, and the dropped one never arrives. + for (uint32_t i = 0; i < APP_EVENT_QUEUE_CAPACITY; i++) { + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, 0), ERROR_NONE); + CHECK_EQ(out.result.launch_id, i); + } + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, 0), ERROR_TIMEOUT); + + app_event_unsubscribe(&sub); +} + +TEST_CASE("app_event_unsubscribe stops further delivery") { + AppEventSubscription sub {}; + sub.app_instance_id = 30; + app_event_subscribe(&sub); + + CHECK_EQ(app_event_unsubscribe(&sub), ERROR_NONE); + CHECK_EQ(app_event_unsubscribe(&sub), ERROR_NOT_FOUND); + + AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; + CHECK_EQ(app_event_emit(30, &event), ERROR_NOT_FOUND); +} + +TEST_CASE("app_event_await times out when no event has arrived") { + AppEventSubscription sub {}; + sub.app_instance_id = 40; + app_event_subscribe(&sub); + + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, 0), ERROR_TIMEOUT); + + app_event_unsubscribe(&sub); +} + +TEST_CASE("app_event_emit stamps the event with the current boot-relative time") { + AppEventSubscription sub {}; + sub.app_instance_id = 50; + app_event_subscribe(&sub); + + auto before = static_cast(get_micros_since_boot()); + AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; + app_event_emit(50, &event); + auto after = static_cast(get_micros_since_boot()); + + AppEvent out {}; + REQUIRE_EQ(app_event_await(&sub, &out, 0), ERROR_NONE); + CHECK_GE(out.timestamp, before); + CHECK_LE(out.timestamp, after); + + app_event_unsubscribe(&sub); +} + +TEST_CASE("app_event_await wakes when the event is emitted from another task") { + AppEventSubscription sub {}; + sub.app_instance_id = 60; + CHECK_EQ(app_event_subscribe(&sub), ERROR_NONE); + + auto* thread = thread_alloc_full( + "app-event-emitter", + 4096, + [](void*) -> int32_t { + delay_millis(20); + AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; + app_event_emit(60, &event); + return 0; + }, + nullptr, + -1 + ); + CHECK_EQ(thread_start(thread), ERROR_NONE); + + AppEvent out {}; + CHECK_EQ(app_event_await(&sub, &out, pdMS_TO_TICKS(2000)), ERROR_NONE); + CHECK_EQ(out.type, APP_EVENT_CLOSE); + + CHECK_EQ(thread_join(thread, pdMS_TO_TICKS(2000), 1), ERROR_NONE); + thread_free(thread); + + app_event_unsubscribe(&sub); +} diff --git a/Tests/app-module/Source/AppManagerTest.cpp b/Tests/app-module/Source/AppManagerTest.cpp new file mode 100644 index 000000000..43f999dd1 --- /dev/null +++ b/Tests/app-module/Source/AppManagerTest.cpp @@ -0,0 +1,431 @@ +#include "doctest.h" + +#include +#include +#include + +#include + +#include +#include + +#include +#include +#include +#include +#include + +extern ServiceManifest app_internal_loader_service_manifest; + +namespace { + +error_t fake_load(AppLocation, void** out_runtime) { + *out_runtime = nullptr; + return ERROR_NONE; +} + +// Stashed by fake_run() on every call, for tests that need to verify exactly what argc/argv it +// received (e.g. that app-module deep-copied the caller's argv) without a getter to query it +// through. +int last_received_argc = -1; +std::vector last_received_argv; + +void stash_received_arguments(int argc, char* argv[]) { + last_received_argc = argc; + last_received_argv.clear(); + for (int i = 0; i < argc; i++) { + last_received_argv.emplace_back(argv[i]); + } +} + +// A minimal stand-in for a real app's main(): subscribes to its own app_event stream and exits +// as soon as it's asked to close - exactly the contract every app instance (with its own +// dedicated task for its whole lifetime) is expected to follow. If launched with a single +// parameter (app_manager_start_for_result()), acts as a modal dialog instead: returns the +// requested result (argv[0], parsed as an int) immediately (the app's own return value IS the +// delivered APP_EVENT_RESULT.result - see app_scheduler.cpp's thread_main()). +int32_t fake_run(void*, uint32_t app_instance_id, int argc, char* argv[]) { + stash_received_arguments(argc, argv); + + if (argc == 1) { + // Single-arg shortcut used by the start_for_result() result-delivery tests: returns + // immediately with argv[0] parsed as the result code, instead of running the normal + // event loop below. Tests that pass other argc (0, or >1 to check deep-copy) fall + // through and run the loop as usual. + return static_cast(strtol(argv[0], nullptr, 10)); + } + + AppEventSubscription sub {}; + sub.app_instance_id = app_instance_id; + app_event_subscribe(&sub); + + while (true) { + AppEvent event {}; + if (app_event_await(&sub, &event, pdMS_TO_TICKS(5000)) != ERROR_NONE) { + break; // safety net so a bug here can't hang the test suite + } + if (event.type == APP_EVENT_CLOSE) { + app_manager_finish(app_instance_id); + break; + } + } + + app_event_unsubscribe(&sub); + return 0; +} + +void fake_unload(void*) { +} + +AppLoaderApi fake_loader_api = { + .load = fake_load, + .run = fake_run, + .unload = fake_unload, +}; + +void* create_loader_service(const ServiceManifest*) { + return &fake_loader_api; +} + +void destroy_loader_service(const ServiceManifest*, void*) { +} + +ServiceManifest fake_loader_manifest = { + .id = APP_LOADER_PATH_SERVICE_ID, + .create_service = create_loader_service, + .destroy_service = destroy_loader_service, + .on_start = nullptr, + .on_stop = nullptr, +}; + +void ensure_fake_loader_registered() { + static bool registered = false; + if (!registered) { + CHECK_EQ(service_manager_add(&fake_loader_manifest, /*auto_start=*/true), ERROR_NONE); + registered = true; + } +} + +// app-module's real APP_LOCATION_MEMORY loader (source/app_internal_loader.cpp) - not a fake, +// since it has no platform dependency and is exactly what a statically-linked app would go +// through. +void ensure_memory_loader_registered() { + static bool registered = false; + if (!registered) { + CHECK_EQ(service_manager_add(&app_internal_loader_service_manifest, /*auto_start=*/true), ERROR_NONE); + registered = true; + } +} + +// Same subscribe-until-close contract as fake_run() above, but called directly as an AppMainFn - +// this is what a real internal app's entry point looks like. +int32_t fake_app_main(uint32_t app_instance_id, int argc, char* argv[]) { + return fake_run(nullptr, app_instance_id, argc, argv); +} + +// Wraps app_manager_get_topmost_instance_id() for terse assertions: 0 if no app is Active. +AppInstanceId topmost_instance_id() { + AppInstanceId id = 0; + return app_manager_get_topmost_instance_id(&id) == ERROR_NONE ? id : 0; +} + +bool wait_for_state(uint32_t instance_id, AppInstanceState target, uint32_t timeout_ms) { + uint32_t waited = 0; + while (waited < timeout_ms) { + if (app_manager_get_state(instance_id) == target) { + return true; + } + delay_millis(10); + waited += 10; + } + return app_manager_get_state(instance_id) == target; +} + +} // namespace + +TEST_CASE("app_manager_start activates an app instance, app_manager_stop terminates it") { + ensure_fake_loader_registered(); + + AppManifest manifest { "test.app.a", "Test App A", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t instance_id = 0; + REQUIRE_EQ(app_manager_start("test.app.a", &instance_id), ERROR_NONE); + CHECK(wait_for_state(instance_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + CHECK_EQ(app_manager_stop(instance_id), ERROR_NONE); + CHECK_EQ(app_manager_get_state(instance_id), APP_INSTANCE_STATE_STOPPED); + + app_manager_remove("test.app.a"); +} + +TEST_CASE("app_manager_start never touches another already-running app - every instance gets its own task") { + ensure_fake_loader_registered(); + + AppManifest manifest_b { "test.app.b", "Test App B", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + AppManifest manifest_c { "test.app.c", "Test App C", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest_b), ERROR_NONE); + REQUIRE_EQ(app_manager_add(&manifest_c), ERROR_NONE); + + uint32_t id_b = 0; + REQUIRE_EQ(app_manager_start("test.app.b", &id_b), ERROR_NONE); + CHECK(wait_for_state(id_b, APP_INSTANCE_STATE_ACTIVE, 1000)); + + uint32_t id_c = 0; + REQUIRE_EQ(app_manager_start("test.app.c", &id_c), ERROR_NONE); + CHECK(wait_for_state(id_c, APP_INSTANCE_STATE_ACTIVE, 1000)); + + // b is untouched by c starting - both stay Active at once, each with its own task. + CHECK_EQ(app_manager_get_state(id_b), APP_INSTANCE_STATE_ACTIVE); + + app_manager_stop(id_b); + app_manager_stop(id_c); + app_manager_remove("test.app.b"); + app_manager_remove("test.app.c"); +} + +TEST_CASE("app_manager_start always creates a fresh instance, even for the same manifest id twice") { + ensure_fake_loader_registered(); + + AppManifest manifest { "test.app.twice", "Test App Twice", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t id_first = 0; + REQUIRE_EQ(app_manager_start("test.app.twice", &id_first), ERROR_NONE); + CHECK(wait_for_state(id_first, APP_INSTANCE_STATE_ACTIVE, 1000)); + + uint32_t id_second = 0; + REQUIRE_EQ(app_manager_start("test.app.twice", &id_second), ERROR_NONE); + CHECK(wait_for_state(id_second, APP_INSTANCE_STATE_ACTIVE, 1000)); + + CHECK_NE(id_first, id_second); + CHECK_EQ(app_manager_get_state(id_first), APP_INSTANCE_STATE_ACTIVE); + + app_manager_stop(id_first); + app_manager_stop(id_second); + app_manager_remove("test.app.twice"); +} + +TEST_CASE("app_manager_get_state returns STOPPED for an unknown instance id") { + CHECK_EQ(app_manager_get_state(999999), APP_INSTANCE_STATE_STOPPED); +} + +TEST_CASE("app_manager_start_with_parameters deep-copies argv before the app instance receives it") { + ensure_fake_loader_registered(); + + AppManifest manifest { "test.app.args", "Test App Args", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t instance_id = 0; + { + // Caller's argv is stack-local and goes out of scope immediately after this block - + // proves app-module made its own copy rather than aliasing the caller's strings. + std::string ssid = "MyNetwork"; + std::string password = "hunter2"; + const char* argv[] = { ssid.c_str(), password.c_str() }; + REQUIRE_EQ(app_manager_start_with_parameters("test.app.args", 2, argv, &instance_id), ERROR_NONE); + } + CHECK(wait_for_state(instance_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + REQUIRE_EQ(last_received_argc, 2); + REQUIRE_EQ(last_received_argv.size(), 2u); + CHECK_EQ(last_received_argv[0], "MyNetwork"); + CHECK_EQ(last_received_argv[1], "hunter2"); + + app_manager_stop(instance_id); + app_manager_remove("test.app.args"); +} + +TEST_CASE("app_manager_add rejects a duplicate id") { + AppManifest manifest { "test.app.dup", "Test App Dup", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + CHECK_EQ(app_manager_add(&manifest), ERROR_INVALID_ARGUMENT); + app_manager_remove("test.app.dup"); +} + +TEST_CASE("app_manager_for_each_manifest visits every registered manifest, including newly added ones") { + AppManifest manifest_x { "test.app.foreach.x", "X", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + AppManifest manifest_y { "test.app.foreach.y", "Y", APP_CATEGORY_SETTINGS, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest_x), ERROR_NONE); + REQUIRE_EQ(app_manager_add(&manifest_y), ERROR_NONE); + + std::vector seen_ids; + app_manager_for_each_manifest([](const AppManifest* manifest, void* context) { + static_cast*>(context)->emplace_back(manifest->id); + }, &seen_ids); + + CHECK(std::ranges::find(seen_ids, "test.app.foreach.x") != seen_ids.end()); + CHECK(std::ranges::find(seen_ids, "test.app.foreach.y") != seen_ids.end()); + + app_manager_remove("test.app.foreach.x"); + app_manager_remove("test.app.foreach.y"); + + seen_ids.clear(); + app_manager_for_each_manifest([](const AppManifest* manifest, void* context) { + static_cast*>(context)->emplace_back(manifest->id); + }, &seen_ids); + CHECK(std::ranges::find(seen_ids, "test.app.foreach.x") == seen_ids.end()); +} + +TEST_CASE("app_manager_start fails for an unregistered manifest id") { + uint32_t instance_id = 0; + CHECK_EQ(app_manager_start("test.app.nonexistent", &instance_id), ERROR_NOT_FOUND); +} + +TEST_CASE("app_manager_start runs an APP_LOCATION_MEMORY app via its function pointer, through the real internal loader") { + ensure_memory_loader_registered(); + + AppManifest manifest { + "test.app.memory", + "Test App Memory", + APP_CATEGORY_USER, + { APP_LOCATION_MEMORY, reinterpret_cast(fake_app_main) } + }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t instance_id = 0; + REQUIRE_EQ(app_manager_start("test.app.memory", &instance_id), ERROR_NONE); + CHECK(wait_for_state(instance_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + CHECK_EQ(app_manager_stop(instance_id), ERROR_NONE); + CHECK_EQ(app_manager_get_state(instance_id), APP_INSTANCE_STATE_STOPPED); + + app_manager_remove("test.app.memory"); +} + +TEST_CASE("app_manager_start_for_result delivers APP_EVENT_RESULT to the parent, which stays Active throughout") { + ensure_fake_loader_registered(); + + AppManifest parent_manifest { "test.app.parent", "Parent", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + AppManifest child_manifest { "test.app.child", "Child", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&parent_manifest), ERROR_NONE); + REQUIRE_EQ(app_manager_add(&child_manifest), ERROR_NONE); + + uint32_t parent_id = 0; + REQUIRE_EQ(app_manager_start("test.app.parent", &parent_id), ERROR_NONE); + CHECK(wait_for_state(parent_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + AppEventSubscription parent_sub {}; + parent_sub.app_instance_id = parent_id; + REQUIRE_EQ(app_event_subscribe(&parent_sub), ERROR_NONE); + + const char* argv[] = { "42" }; + uint32_t child_id = 0; + REQUIRE_EQ(app_manager_start_for_result("test.app.child", parent_id, 1, argv, &child_id), ERROR_NONE); + + // Launching a modal child never touches the parent's own task/state. + CHECK_EQ(app_manager_get_state(parent_id), APP_INSTANCE_STATE_ACTIVE); + + AppEvent event {}; + REQUIRE_EQ(app_event_await(&parent_sub, &event, pdMS_TO_TICKS(2000)), ERROR_NONE); + CHECK_EQ(event.type, APP_EVENT_RESULT); + CHECK_EQ(event.result.launch_id, child_id); + CHECK_EQ(event.result.result, 42); + + app_event_unsubscribe(&parent_sub); + app_manager_stop(child_id); + app_manager_stop(parent_id); + app_manager_remove("test.app.parent"); + app_manager_remove("test.app.child"); +} + +TEST_CASE("app_manager_start_for_result delivers the child's own return value as the result") { + ensure_fake_loader_registered(); + + AppManifest parent_manifest { "test.app.parent2", "Parent2", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + AppManifest child_manifest { "test.app.child2", "Child2", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&parent_manifest), ERROR_NONE); + REQUIRE_EQ(app_manager_add(&child_manifest), ERROR_NONE); + + uint32_t parent_id = 0; + REQUIRE_EQ(app_manager_start("test.app.parent2", &parent_id), ERROR_NONE); + CHECK(wait_for_state(parent_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + AppEventSubscription parent_sub {}; + parent_sub.app_instance_id = parent_id; + REQUIRE_EQ(app_event_subscribe(&parent_sub), ERROR_NONE); + + uint32_t child_id = 0; + // No parameters - fake_run falls through to its normal CLOSE loop instead of acting as a + // dialog. + REQUIRE_EQ(app_manager_start_for_result("test.app.child2", parent_id, 0, nullptr, &child_id), ERROR_NONE); + CHECK(wait_for_state(child_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + app_manager_stop(child_id); // force-close + + AppEvent event {}; + REQUIRE_EQ(app_event_await(&parent_sub, &event, pdMS_TO_TICKS(2000)), ERROR_NONE); + CHECK_EQ(event.type, APP_EVENT_RESULT); + CHECK_EQ(event.result.launch_id, child_id); + CHECK_EQ(event.result.result, 0); // fake_run's CLOSE loop always returns 0 + + app_event_unsubscribe(&parent_sub); + app_manager_stop(parent_id); + app_manager_remove("test.app.parent2"); + app_manager_remove("test.app.child2"); +} + +TEST_CASE("app_manager_get_topmost_instance_id returns NOT_FOUND when nothing is active, then tracks who's on top") { + ensure_fake_loader_registered(); + AppInstanceId id = 999999; + CHECK_EQ(app_manager_get_topmost_instance_id(&id), ERROR_NOT_FOUND); + + AppManifest manifest_a { "test.app.top_a", "A", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + AppManifest manifest_b { "test.app.top_b", "B", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest_a), ERROR_NONE); + REQUIRE_EQ(app_manager_add(&manifest_b), ERROR_NONE); + + uint32_t id_a = 0; + REQUIRE_EQ(app_manager_start("test.app.top_a", &id_a), ERROR_NONE); + CHECK(wait_for_state(id_a, APP_INSTANCE_STATE_ACTIVE, 1000)); + CHECK_EQ(topmost_instance_id(), id_a); + + // a stays Active - b just has a higher (more recently allocated) instance id, so it becomes + // topmost without a superseding/saving. + uint32_t id_b = 0; + REQUIRE_EQ(app_manager_start("test.app.top_b", &id_b), ERROR_NONE); + CHECK(wait_for_state(id_b, APP_INSTANCE_STATE_ACTIVE, 1000)); + CHECK_EQ(topmost_instance_id(), id_b); + + char app_id_buffer[64]; + REQUIRE_EQ(app_manager_get_topmost_app_id(app_id_buffer, sizeof(app_id_buffer)), ERROR_NONE); + CHECK_EQ(std::string(app_id_buffer), "test.app.top_b"); + + // A modal child stays Active alongside its parent while shown - the child (started more + // recently) must be reported as topmost, not the parent. No parameters, so fake_run() takes + // its persistent CLOSE loop branch instead of instantly resolving like a real dialog would - + // needed here so there's a reliable window to observe it as topmost. + uint32_t id_c = 0; + REQUIRE_EQ(app_manager_start_for_result("test.app.top_a", id_b, 0, nullptr, &id_c), ERROR_NONE); + CHECK(wait_for_state(id_c, APP_INSTANCE_STATE_ACTIVE, 1000)); + CHECK_EQ(topmost_instance_id(), id_c); + + app_manager_stop(id_c); + CHECK_EQ(topmost_instance_id(), id_b); + + app_manager_stop(id_a); + app_manager_stop(id_b); + app_manager_remove("test.app.top_a"); + app_manager_remove("test.app.top_b"); +} + +TEST_CASE("app_manager_get_topmost_app_id returns BUFFER_OVERFLOW for a too-small buffer, NOT_FOUND when nothing is active") { + ensure_fake_loader_registered(); + + char buffer[4]; + CHECK_EQ(app_manager_get_topmost_app_id(buffer, sizeof(buffer)), ERROR_NOT_FOUND); + CHECK_EQ(app_manager_get_topmost_app_id(buffer, 0), ERROR_BUFFER_OVERFLOW); + + AppManifest manifest { "test.app.top_overflow", "Overflow", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t id = 0; + REQUIRE_EQ(app_manager_start("test.app.top_overflow", &id), ERROR_NONE); + CHECK(wait_for_state(id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + // "test.app.top_overflow" doesn't fit in a 4-byte buffer. + CHECK_EQ(app_manager_get_topmost_app_id(buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); + + app_manager_stop(id); + app_manager_remove("test.app.top_overflow"); +} diff --git a/Tests/app-module/Source/Main.cpp b/Tests/app-module/Source/Main.cpp new file mode 100644 index 000000000..acd1df905 --- /dev/null +++ b/Tests/app-module/Source/Main.cpp @@ -0,0 +1,51 @@ +#define DOCTEST_CONFIG_IMPLEMENT +#include "doctest.h" +#include + +#include "FreeRTOS.h" +#include "task.h" + +typedef struct { + int argc; + char** argv; + int result; +} TestTaskData; + +void test_task(void* parameter) { + auto* data = (TestTaskData*)parameter; + + doctest::Context context; + + context.applyCommandLine(data->argc, data->argv); + + // overrides + context.setOption("no-breaks", true); // don't break in the debugger when assertions fail + + data->result = context.run(); + + vTaskEndScheduler(); + + vTaskDelete(nullptr); +} + +int main(int argc, char** argv) { + TestTaskData data = { + .argc = argc, + .argv = argv, + .result = 0 + }; + + BaseType_t task_result = xTaskCreate( + test_task, + "test_task", + 8192, + &data, + 1, + nullptr + ); + assert(task_result == pdPASS); + + vTaskStartScheduler(); + + return data.result; +}