From 92ca0466813e763fefa0b9184af96b8184b09f3e Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Thu, 27 Aug 2026 21:50:12 +0200 Subject: [PATCH] app-module events & callstack config, crash diagnostics (#630) - Apps can specify task stack depth and preferred memory placement in their manifests. - App identifiers are validated against length and character requirements. - Crash diagnostics now show the crash cause, reason, fault address, call stack, and program-counter details, with logs saved for review. - App closing is more consistent across built-in screens. There's now a dedicated function, and the old _emit() function is made private. - Crash diagnostics no longer display a QR code without a call stack. - Improved memory allocation for unrestricted requests. --- Documentation/ideas.md | 4 +- Modules/app-module/include/app/event.h | 17 +- Modules/app-module/include/app/manifest.h | 23 +++ Modules/app-module/include/app/metadata.h | 6 + .../app-module/private/app/private/event.h | 24 +++ .../private/app/private/{app_fs.h => fs.h} | 0 .../app/private/{app_ledger.h => ledger.h} | 0 ...internal.h => metadata_parsing_internal.h} | 4 +- .../private/{app_scheduler.h => scheduler.h} | 3 +- Modules/app-module/source/app_install.cpp | 5 +- .../source/app_metadata_parsing.cpp | 39 ++-- .../source/app_metadata_parsing_v1.cpp | 9 +- .../source/app_metadata_parsing_v2.cpp | 35 +++- Modules/app-module/source/event.cpp | 5 + Modules/app-module/source/manager.cpp | 18 +- Modules/app-module/source/manifest.cpp | 15 ++ .../{app_scheduler.cpp => scheduler.cpp} | 166 ++++++++++++++++-- Modules/app-module/source/symbols.cpp | 4 +- .../tests/source/app_event_test.cpp | 5 + .../tests/source/app_manager_test.cpp | 36 ++++ .../tests/source/combined_event_test.cpp | 5 + Tactility/Include/Tactility/PanicHandler.h | 15 ++ .../Tactility/app/crashdiagnostics/QrUrl.h | 3 +- Tactility/Source/DeprecatedPaths.cpp | 6 +- Tactility/Source/PanicHandler.cpp | 40 ++++- Tactility/Source/Tactility.cpp | 8 +- Tactility/Source/app/addgps/AddGps.cpp | 7 +- .../Source/app/alertdialog/AlertDialog.cpp | 8 +- .../Source/app/appdetails/AppDetails.cpp | 7 +- Tactility/Source/app/apphub/AppHubApp.cpp | 3 +- .../app/apphubdetails/AppHubDetailsApp.cpp | 3 +- Tactility/Source/app/applist/AppList.cpp | 29 ++- .../Source/app/appsettings/AppSettings.cpp | 29 +-- .../Source/app/apwebserver/ApWebServer.cpp | 7 +- .../app/audiosettings/AudioSettings.cpp | 7 +- Tactility/Source/app/boot/Boot.cpp | 23 ++- Tactility/Source/app/btmanage/BtManage.cpp | 3 +- Tactility/Source/app/btmanage/View.cpp | 7 +- .../app/btpeersettings/BtPeerSettings.cpp | 7 +- Tactility/Source/app/chat/ChatView.cpp | 7 +- .../app/crashdiagnostics/CrashDiagnostics.cpp | 91 +++++++++- .../Source/app/crashdiagnostics/QrUrl.cpp | 4 +- .../Source/app/development/Development.cpp | 7 +- Tactility/Source/app/display/Display.cpp | 7 +- Tactility/Source/app/files/View.cpp | 7 +- .../app/fileselection/FileSelection.cpp | 3 +- Tactility/Source/app/fileselection/View.cpp | 7 +- .../Source/app/gpssettings/GpsSettings.cpp | 7 +- .../app/grovesettings/GroveSettings.cpp | 7 +- .../Source/app/i2cscanner/I2cScanner.cpp | 7 +- .../Source/app/imageviewer/ImageViewer.cpp | 7 +- .../Source/app/inputdialog/InputDialog.cpp | 5 +- .../Source/app/keyboard/KeyboardSettings.cpp | 7 +- Tactility/Source/app/launcher/Launcher.cpp | 9 +- .../app/localesettings/LocaleSettings.cpp | 7 +- Tactility/Source/app/power/Power.cpp | 7 +- Tactility/Source/app/poweroff/PowerOff.cpp | 7 +- .../Source/app/screenshot/Screenshot.cpp | 7 +- .../app/selectiondialog/SelectionDialog.cpp | 11 +- Tactility/Source/app/settings/Settings.cpp | 27 +-- Tactility/Source/app/setup/Setup.cpp | 6 +- .../Source/app/systeminfo/SystemInfo.cpp | 7 +- .../app/timedatesettings/TimeDateSettings.cpp | 7 +- Tactility/Source/app/timezone/TimeZone.cpp | 10 +- .../app/touchcalibration/TouchCalibration.cpp | 7 +- .../app/trackball/TrackballSettings.cpp | 7 +- .../Source/app/usbsettings/UsbSettings.cpp | 7 +- .../webserversettings/WebServerSettings.cpp | 7 +- .../app/wifiapsettings/WifiApSettings.cpp | 6 +- .../Source/app/wificonnect/WifiConnect.cpp | 9 +- Tactility/Source/app/wifimanage/View.cpp | 10 +- .../include/tactility/concurrent/thread.h | 6 +- TactilityKernel/source/concurrent/thread.cpp | 30 ++-- TactilityKernel/source/memory_esp32.cpp | 33 +++- 74 files changed, 654 insertions(+), 381 deletions(-) create mode 100644 Modules/app-module/private/app/private/event.h rename Modules/app-module/private/app/private/{app_fs.h => fs.h} (100%) rename Modules/app-module/private/app/private/{app_ledger.h => ledger.h} (100%) rename Modules/app-module/private/app/private/{app_metadata_parsing_internal.h => metadata_parsing_internal.h} (91%) rename Modules/app-module/private/app/private/{app_scheduler.h => scheduler.h} (91%) create mode 100644 Modules/app-module/source/manifest.cpp rename Modules/app-module/source/{app_scheduler.cpp => scheduler.cpp} (63%) diff --git a/Documentation/ideas.md b/Documentation/ideas.md index 02aa079e..21d35ee4 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -11,8 +11,10 @@ ## Higher Priority +- CrashDiagnostics shouldn't show a QR when there's no callstack - lvgl file lock won't work with display vs sdcard when lvgl is stopped (external app bug risk) -- Apps should be able to specify stack size in their manifest +- Apps should be able to specify stack size in their manifest, per architecture. + Use thread_get_stack_space() to find out the unused bytes - Apps currently have a `Context` object with an `appInstanceId` in it, purely for being able to close the app. Change it so that the app has its own termination signal that it waits for in the loop, it should subscribe to the event group. - stopAppFromToolbar() in Tactility.cpp stops the top-most app. Change it so the toolbar knows for which app id it is created, so it can rely on that. diff --git a/Modules/app-module/include/app/event.h b/Modules/app-module/include/app/event.h index f6b77f90..e8dc3bb1 100644 --- a/Modules/app-module/include/app/event.h +++ b/Modules/app-module/include/app/event.h @@ -114,16 +114,6 @@ error_t app_event_subscribe_with_app_id(struct AppEventSubscription* sub, struct */ error_t app_event_unsubscribe(struct AppEventSubscription* sub); -/** - * Deliver @a event to every subscription registered for @a app_instance_id (normally exactly one). - * @warning Does not work in ISR context. - * @retval ERROR_NONE delivered to at least one subscription - * @retval ERROR_NOT_FOUND no subscription is registered for @a app_instance_id - * @retval ERROR_RESOURCE at least one matching subscription's queue was full; the event was - * dropped for that subscription (still delivered to any other matching subscription) - */ -error_t app_event_emit(AppInstanceId app_instance_id, const struct AppEvent* event); - /** * Non-blocking: pop the next event for @a sub if one is already queued. * @warning Never blocks. To wait for an event, block in task_event_group_wait()/ @@ -134,6 +124,13 @@ error_t app_event_emit(AppInstanceId app_instance_id, const struct AppEvent* eve */ error_t app_event_poll(struct AppEventSubscription* sub, struct AppEvent* out_event); +/** + * Emits a close event to the specified app. + * @param[in] instance_id + * @return ERROR_NONE when event was successfully emitted + */ +error_t app_event_emit_close(AppInstanceId instance_id); + #ifdef __cplusplus } #endif diff --git a/Modules/app-module/include/app/manifest.h b/Modules/app-module/include/app/manifest.h index 0e2e6e2d..5d8ea67e 100644 --- a/Modules/app-module/include/app/manifest.h +++ b/Modules/app-module/include/app/manifest.h @@ -3,12 +3,16 @@ #include "location.h" +#include #include #ifdef __cplusplus extern "C" { #endif +// Character count, excluding null terminator +#define APP_ID_LENGTH 32 + /** Broad classification of an app, used for grouping/launcher presentation. */ enum AppCategory { APP_CATEGORY_SYSTEM, @@ -24,6 +28,21 @@ enum AppManifestFlags { APP_MANIFEST_FLAG_HIDDEN = 1 >> 0, }; +/** Largest stack depth (in words) an app may request. Keeps `depth * sizeof(StackType_t)` safely + * bounded and stops one app from claiming an unreasonable share of available RAM. A depth beyond + * this must be rejected outright, not silently truncated or clamped. */ +#define APP_STACK_SIZE_MAX 16384 + +struct AppStackConfig { + /** Stack depth (in words, matching FreeRTOS's configSTACK_DEPTH_TYPE) for this app's task. + * 0 uses the scheduler's default. Must not exceed APP_STACK_SIZE_MAX. */ + uint16_t depth; + /** Desired memory capability. + * 0 means default. + * Combine one or more of \a MemoryCapability from with a bitwise OR.*/ + uint16_t desired_memory_capability; +}; + /** Describes a registrable app. One manifest exists per app id. */ struct AppManifest { /** Unique app identifier. Should never be NULL. */ @@ -34,8 +53,12 @@ struct AppManifest { struct AppLocation location; /** Bitmask of AppManifestFlags. Most apps should leave this 0. */ uint8_t flags; + /** Stack allocation config for this app's task. */ + struct AppStackConfig stack; }; +bool app_id_is_valid(const char* id); + #ifdef __cplusplus } #endif diff --git a/Modules/app-module/include/app/metadata.h b/Modules/app-module/include/app/metadata.h index ae0a16db..364dd2cf 100644 --- a/Modules/app-module/include/app/metadata.h +++ b/Modules/app-module/include/app/metadata.h @@ -50,6 +50,12 @@ struct AppMetadata { * Must be NULL-terminated. */ char requires_device_id[APP_METADATA_REQUIRES_DEVICE_ID_LENGTH + 1]; + + /** + * Stack depth (in words) for the app's task. Optional; 0 means scheduler default. + * @warning Avoid default values: the default is conservative, which wastes memory. + */ + uint32_t stack_depth; }; /** diff --git a/Modules/app-module/private/app/private/event.h b/Modules/app-module/private/app/private/event.h new file mode 100644 index 00000000..71136bb6 --- /dev/null +++ b/Modules/app-module/private/app/private/event.h @@ -0,0 +1,24 @@ +#pragma once + +#include +#include + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * Deliver @a event to every subscription registered for @a app_instance_id (normally exactly one). + * @warning Does not work in ISR context. + * @retval ERROR_NONE delivered to at least one subscription + * @retval ERROR_NOT_FOUND no subscription is registered for @a app_instance_id + * @retval ERROR_RESOURCE at least one matching subscription's queue was full; the event was + * dropped for that subscription (still delivered to any other matching subscription) + */ +error_t app_event_emit(AppInstanceId app_instance_id, const struct AppEvent* event); + +#ifdef __cplusplus +} +#endif diff --git a/Modules/app-module/private/app/private/app_fs.h b/Modules/app-module/private/app/private/fs.h similarity index 100% rename from Modules/app-module/private/app/private/app_fs.h rename to Modules/app-module/private/app/private/fs.h diff --git a/Modules/app-module/private/app/private/app_ledger.h b/Modules/app-module/private/app/private/ledger.h similarity index 100% rename from Modules/app-module/private/app/private/app_ledger.h rename to Modules/app-module/private/app/private/ledger.h diff --git a/Modules/app-module/private/app/private/app_metadata_parsing_internal.h b/Modules/app-module/private/app/private/metadata_parsing_internal.h similarity index 91% rename from Modules/app-module/private/app/private/app_metadata_parsing_internal.h rename to Modules/app-module/private/app/private/metadata_parsing_internal.h index 9259e569..689abfa4 100644 --- a/Modules/app-module/private/app/private/app_metadata_parsing_internal.h +++ b/Modules/app-module/private/app/private/metadata_parsing_internal.h @@ -13,10 +13,10 @@ bool app_metadata_get_value(const std::map& properties, const std::string& key, std::string& out_value); bool app_metadata_is_valid_format_version(const std::string& version); -bool app_metadata_is_valid_id(const std::string& id); bool app_metadata_is_valid_name(const std::string& name); bool app_metadata_is_valid_version_name(const std::string& version); bool app_metadata_is_valid_version_code(const std::string& version); +bool app_metadata_is_valid_stack_size(const std::string& value); /** Validates a comma-separated list of device ids (alphanumeric + '-' items, matching Devices/ folder names). */ bool app_metadata_is_valid_device_id_list(const std::string& value); @@ -31,3 +31,5 @@ bool app_metadata_parse_v1(const std::map& properties, /** Parses a V2 (flat dot-notation, e.g. "app.version.name=...") manifest map into @a out_metadata. */ bool app_metadata_parse_v2(const std::map& properties, struct AppMetadata& out_metadata); + +bool app_metadata_validate_string(const std::string& value, bool (*is_valid_char)(char)); diff --git a/Modules/app-module/private/app/private/app_scheduler.h b/Modules/app-module/private/app/private/scheduler.h similarity index 91% rename from Modules/app-module/private/app/private/app_scheduler.h rename to Modules/app-module/private/app/private/scheduler.h index d323360c..ac3ed9f0 100644 --- a/Modules/app-module/private/app/private/app_scheduler.h +++ b/Modules/app-module/private/app/private/scheduler.h @@ -21,12 +21,13 @@ extern "C" { * AppLoaderApi::load()/run(), marking the instance ACTIVE for the duration of run(). * @param[in] app_instance_id id already allocated in the ledger for this instance * @param[in] location the location of the app + * @param[in] stack stack allocation config for the app's task * @param[in] argc the amount of arguments to pass to the app's main function * @param[in] argv the array of arguments to pass to the app's main function - ownership is * 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(AppInstanceId app_instance_id, struct AppLocation location, int argc, char* argv[]); +error_t app_scheduler_start(AppInstanceId app_instance_id, struct AppLocation location, struct AppStackConfig stack, int argc, char* argv[]); /** * Permanently stops an app instance (APP_EVENT_CLOSE if it was running), bound-waits for its diff --git a/Modules/app-module/source/app_install.cpp b/Modules/app-module/source/app_install.cpp index d9dfe213..8f9ea37f 100644 --- a/Modules/app-module/source/app_install.cpp +++ b/Modules/app-module/source/app_install.cpp @@ -4,8 +4,8 @@ #include #include -#include -#include +#include +#include #include #include @@ -180,6 +180,7 @@ error_t register_installed_app_locked(const std::string& app_dir_path, const App .category = APP_CATEGORY_USER, .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, .flags = 0, + .stack = { .depth = static_cast(metadata.stack_depth), .desired_memory_capability = 0 }, }; // Belt-and-braces: app_install()'s earlier app_manager_remove() call is meant to have diff --git a/Modules/app-module/source/app_metadata_parsing.cpp b/Modules/app-module/source/app_metadata_parsing.cpp index d1ca9c8d..54947d3e 100644 --- a/Modules/app-module/source/app_metadata_parsing.cpp +++ b/Modules/app-module/source/app_metadata_parsing.cpp @@ -1,10 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 -#include "tactility/filesystem/file_mutex.h" - +#include #include - -#include +#include #include @@ -16,8 +14,19 @@ constexpr auto* TAG = "app_metadata"; +bool app_metadata_validate_string(const std::string& value, bool (*is_valid_char)(char)) { + for (char c: value) { + if (!is_valid_char(c)) { + return false; + } + } + return true; +} + namespace { +#define validate_string app_metadata_validate_string + std::string trim(const std::string& value) { constexpr auto* whitespace = " \t\r\n"; auto start = value.find_first_not_of(whitespace); @@ -28,15 +37,6 @@ std::string trim(const std::string& value) { return value.substr(start, end - start + 1); } -bool validate_string(const std::string& value, bool (*is_valid_char)(char)) { - for (char c: value) { - if (!is_valid_char(c)) { - return false; - } - } - return true; -} - /** Validates a comma-separated list: non-empty, no leading/trailing/double commas (which would * produce an empty item), and every item passing @a is_valid_item. */ bool validate_csv_list(const std::string& value, bool (*is_valid_item)(const std::string&)) { @@ -125,12 +125,6 @@ bool app_metadata_is_valid_format_version(const std::string& version) { }); } -bool app_metadata_is_valid_id(const std::string& id) { - return id.size() >= 5 && id.size() <= APP_METADATA_APP_ID_LENGTH && validate_string(id, [](char c) { - return std::isalnum(static_cast(c)) != 0 || c == '.'; - }); -} - bool app_metadata_is_valid_name(const std::string& name) { return name.size() >= 2 && name.size() <= APP_METADATA_APP_NAME_LENGTH && validate_string(name, [](char c) { return std::isalnum(static_cast(c)) != 0 || c == ' ' || c == '-'; @@ -150,6 +144,13 @@ bool app_metadata_is_valid_version_code(const std::string& version) { }); } +bool app_metadata_is_valid_stack_size(const std::string& value) { + // 10 digits is the maximum decimal width of uint32_t. + return !value.empty() && value.size() <= 10 && validate_string(value, [](char c) { + return std::isdigit(static_cast(c)) != 0; + }); +} + bool app_metadata_is_valid_device_id_list(const std::string& value) { return validate_csv_list(value, [](const std::string& item) { bool has_alnum = false; diff --git a/Modules/app-module/source/app_metadata_parsing_v1.cpp b/Modules/app-module/source/app_metadata_parsing_v1.cpp index ae759a51..96e2b36c 100644 --- a/Modules/app-module/source/app_metadata_parsing_v1.cpp +++ b/Modules/app-module/source/app_metadata_parsing_v1.cpp @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 +#include "app/manifest.h" + + #include -#include +#include #include @@ -28,7 +31,7 @@ bool app_metadata_parse_v1(const std::map& properties, return false; } - if (!app_metadata_is_valid_id(id)) { + if (!app_id_is_valid(id.c_str())) { LOG_E(TAG, "Invalid app id"); return false; } @@ -97,5 +100,7 @@ bool app_metadata_parse_v1(const std::map& properties, return false; } + out_metadata.stack_depth = 0; + return true; } diff --git a/Modules/app-module/source/app_metadata_parsing_v2.cpp b/Modules/app-module/source/app_metadata_parsing_v2.cpp index b6baaa0a..7a897da8 100644 --- a/Modules/app-module/source/app_metadata_parsing_v2.cpp +++ b/Modules/app-module/source/app_metadata_parsing_v2.cpp @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 +#include "app/manifest.h" + + #include -#include +#include #include @@ -28,7 +31,7 @@ bool app_metadata_parse_v2(const std::map& properties, return false; } - if (!app_metadata_is_valid_id(id)) { + if (!app_id_is_valid(id.c_str())) { LOG_E(TAG, "Invalid app id"); return false; } @@ -114,5 +117,33 @@ bool app_metadata_parse_v2(const std::map& properties, } } + // app.stack.depth (optional; if present, must be a valid unsigned decimal fitting uint32_t) + + auto stack_size_iterator = properties.find("app.stack.depth"); + if (stack_size_iterator != properties.end()) { + const std::string& stack_size_string = stack_size_iterator->second; + if (!app_metadata_is_valid_stack_size(stack_size_string)) { + LOG_E(TAG, "Invalid app.stack.depth"); + return false; + } + + uint32_t stack_size = 0; + const auto* stack_size_first = stack_size_string.data(); + const auto* stack_size_last = stack_size_first + stack_size_string.size(); + if (std::from_chars(stack_size_first, stack_size_last, stack_size).ec != std::errc {}) { + LOG_E(TAG, "App stack depth out of range"); + return false; + } + + // Reject outright rather than truncating/clamping into AppStackConfig::depth (uint16_t) - + // a value like 1073741825 would otherwise silently narrow to 1, handing the app a + // catastrophically undersized stack instead of the huge one it declared. + if (stack_size > APP_STACK_SIZE_MAX) { + LOG_E(TAG, "App stack depth %u exceeds APP_STACK_SIZE_MAX(%u)", stack_size, APP_STACK_SIZE_MAX); + return false; + } + out_metadata.stack_depth = stack_size; + } + return true; } diff --git a/Modules/app-module/source/event.cpp b/Modules/app-module/source/event.cpp index 95766875..6c7ea331 100644 --- a/Modules/app-module/source/event.cpp +++ b/Modules/app-module/source/event.cpp @@ -120,4 +120,9 @@ error_t app_event_poll(AppEventSubscription* sub, AppEvent* out_event) { return try_pop(sub, out_event) ? ERROR_NONE : ERROR_TIMEOUT; } +error_t app_event_emit_close(AppInstanceId instance_id) { + AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; + return app_event_emit(instance_id, &event); +} + } // extern "C" diff --git a/Modules/app-module/source/manager.cpp b/Modules/app-module/source/manager.cpp index 68867c8d..ae200ac1 100644 --- a/Modules/app-module/source/manager.cpp +++ b/Modules/app-module/source/manager.cpp @@ -1,9 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 #include #include -#include -#include -#include +#include +#include +#include #include #include @@ -103,17 +103,20 @@ error_t start_internal(const char* id, AppInstanceId parent_instance_id, int arg const AppManifest* manifest = manifest_iterator->second; AppInstanceId target_id = ledger.next_instance_id++; - AppInstanceRecord record { target_id, manifest, APP_INSTANCE_STATE_STARTING, nullptr }; + AppInstanceRecord record { .id = target_id, .manifest = manifest, .state = APP_INSTANCE_STATE_STARTING, .task = nullptr }; record.parent_id = parent_instance_id; ledger.instances[target_id] = record; mutex_unlock(&ledger.mutex); - error_t result = app_scheduler_start(target_id, manifest->location, argc, argv); - if (result != ERROR_NONE) { + LOG_I(TAG, "[instance %d] starting %s with parent %d", target_id, manifest->id, parent_instance_id); + + error_t error = app_scheduler_start(target_id, manifest->location, manifest->stack, argc, argv); + if (error != ERROR_NONE) { mutex_lock(&ledger.mutex); ledger.instances.erase(target_id); mutex_unlock(&ledger.mutex); - return result; + LOG_I(TAG, "[instance %d] Failed to start: %s", target_id, error_to_string(error)); + return error; } *out_app_instance_id = target_id; @@ -289,6 +292,7 @@ void app_manager_install_path_scan(void) { .category = APP_CATEGORY_USER, .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, .flags = 0, + .stack = { .depth = static_cast(metadata.stack_depth), .desired_memory_capability = 0 }, }; new_records.push_back(std::move(record)); } diff --git a/Modules/app-module/source/manifest.cpp b/Modules/app-module/source/manifest.cpp new file mode 100644 index 00000000..81adb02d --- /dev/null +++ b/Modules/app-module/source/manifest.cpp @@ -0,0 +1,15 @@ +#include +#include +#include +#include + +extern "C" { + +bool app_id_is_valid(const char* id) { + auto size = strlen(id); + return size >= 5 && size <= APP_ID_LENGTH && app_metadata_validate_string(id, [](char c) { + return std::isalnum(static_cast(c)) != 0 || c == '.'; + }); +} + +} diff --git a/Modules/app-module/source/app_scheduler.cpp b/Modules/app-module/source/scheduler.cpp similarity index 63% rename from Modules/app-module/source/app_scheduler.cpp rename to Modules/app-module/source/scheduler.cpp index fe72cdca..52e96c25 100644 --- a/Modules/app-module/source/app_scheduler.cpp +++ b/Modules/app-module/source/scheduler.cpp @@ -1,9 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 -#include -#include -#include #include #include +#include +#include +#include #include #include @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -27,6 +28,15 @@ 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; +// Used when an app's manifest doesn't request a specific stack depth (0). 8192 bytes' worth. +constexpr size_t APP_DEFAULT_STACK_DEPTH = 8192 / sizeof(StackType_t); + +// Task control blocks must stay in internal RAM; only the stack itself may live in external memory. +constexpr MemoryPolicy APP_TASK_TCB_POLICY = { MEMORY_CAPABILITY_INTERNAL, 0, 0 }; + +constexpr auto* APP_REAPER_TASK_NAME = "app_reaper"; +constexpr size_t APP_REAPER_STACK_DEPTH = 2048 / sizeof(StackType_t); + namespace { struct TaskContext { @@ -36,8 +46,42 @@ struct TaskContext { int argc; char** argv; AppCompletionSignal* completion; + StackType_t* stackBuffer; + StaticTask_t* taskTcb; }; +struct ReaperContext { + TaskHandle_t target; + StackType_t* stackBuffer; + StaticTask_t* taskTcb; +}; + +void reaper_task_main(void* context) { + auto* ctx = static_cast(context); + + while (eTaskGetState(ctx->target) == eRunning) { + taskYIELD(); + } + vTaskDelete(ctx->target); + memory_free(ctx->stackBuffer); + memory_free(ctx->taskTcb); + delete ctx; + + vTaskDelete(nullptr); +} + +// Hands off this task's own statically-allocated stack/TCB (which it can never safely free +// itself - a task can't free the stack it's still running on, see UsbHidInput.cpp for the same +// constraint) to a short-lived helper task, then suspends forever. Never returns. +void reap_self(StackType_t* stack_buffer, StaticTask_t* task_tcb) { + auto* reaper_ctx = new (std::nothrow) ReaperContext { xTaskGetCurrentTaskHandle(), stack_buffer, task_tcb }; + if (reaper_ctx == nullptr || xTaskCreate(reaper_task_main, APP_REAPER_TASK_NAME, APP_REAPER_STACK_DEPTH, reaper_ctx, tskIDLE_PRIORITY, nullptr) != pdPASS) { + LOG_E(TAG, "Failed to create app reaper task; leaking stack buffer"); + delete reaper_ctx; + } + vTaskSuspend(nullptr); +} + void set_state(AppInstanceId app_instance_id, AppInstanceState state) { auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); @@ -144,7 +188,7 @@ void app_task_main(void* 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); + LOG_I(TAG, "[instance %lu] Task started", ctx->app_instance_id); set_state(ctx->app_instance_id, APP_INSTANCE_STATE_ACTIVE); @@ -165,9 +209,11 @@ void app_task_main(void* context) { AppInstanceId app_instance_id = ctx->app_instance_id; AppCompletionSignal* completion = ctx->completion; + StackType_t* stack_buffer = ctx->stackBuffer; + StaticTask_t* task_tcb = ctx->taskTcb; delete ctx; - LOG_I(TAG, "Thread for %d finished", app_instance_id); + LOG_I(TAG, "[instance %lu] Task finished", app_instance_id); // Erase the ledger entry before self-deleting - see "Reap self-terminated app tasks": // nothing else is guaranteed to ever call app_scheduler_stop() for this instance (the @@ -187,17 +233,23 @@ void app_task_main(void* context) { xSemaphoreGive(completion->semaphore); release_completion_signal(completion); // releases app_task_main()'s own reference + LOG_I(TAG, "[instance %lu] minimum free stack space: %d bytes", app_instance_id, uxTaskGetStackHighWaterMark(nullptr)); +#ifdef ESP_PLATFORM + // Statically-allocated stack/TCB (see app_scheduler_start()) - can't self-delete, see reap_self(). + reap_self(stack_buffer, task_tcb); +#else vTaskDelete(nullptr); +#endif } } // namespace extern "C" { -error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, int argc, char* argv[]) { +error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, AppStackConfig stack, 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)); + LOG_E(TAG, "[instance %lu] No app loader is registered (service '%s' not found)", app_instance_id, loader_service_id_for(location.type)); app_ledger_free_arguments(argc, argv); return ERROR_NOT_FOUND; } @@ -205,30 +257,96 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, 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)); + LOG_E(TAG, "[instance %lu] Failed to load app: %s", app_instance_id, error_to_string(load_result)); app_ledger_free_arguments(argc, argv); return load_result; } auto* completion = new (std::nothrow) AppCompletionSignal(); if (completion == nullptr) { - LOG_E(TAG, "Failed to allocate app"); + LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id); loader->unload(runtime); app_ledger_free_arguments(argc, argv); return ERROR_OUT_OF_MEMORY; } completion->semaphore = xSemaphoreCreateBinary(); if (completion->semaphore == nullptr) { - LOG_E(TAG, "Failed to allocate app"); + LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id); delete completion; loader->unload(runtime); app_ledger_free_arguments(argc, argv); return ERROR_OUT_OF_MEMORY; } - auto* context = new (std::nothrow) TaskContext { loader, runtime, app_instance_id, argc, argv, completion }; + // Same bound app_metadata_parse() enforces on manifest.properties-declared depths - a + // manifest built directly in C++ (not parsed from a file) must be held to it too. + if (stack.depth > APP_STACK_SIZE_MAX) { + LOG_E(TAG, "[instance %lu] stack depth %u exceeds APP_STACK_SIZE_MAX(%u)", app_instance_id, stack.depth, APP_STACK_SIZE_MAX); + vSemaphoreDelete(completion->semaphore); + delete completion; + loader->unload(runtime); + app_ledger_free_arguments(argc, argv); + return ERROR_INVALID_ARGUMENT; + } + + if (stack.depth == 0) { + LOG_W(TAG, "[instance %lu] using default stack depth", app_instance_id); + } + size_t effective_stack_depth = stack.depth != 0 ? stack.depth : APP_DEFAULT_STACK_DEPTH; + +#ifdef ESP_PLATFORM + // ESP-IDF's FreeRTOS port has configSUPPORT_STATIC_ALLOCATION, POSIX doesn't + // Try the desired capability first (if any). If it fails or isn't specified, use the fallback/default alloc behaviour (use internal memory). + StackType_t* stack_buffer = nullptr; + if (stack.desired_memory_capability != 0) { + MemoryPolicy requested_policy = { .required = stack.desired_memory_capability, .desired = 0, .alignment = 0 }; + stack_buffer = static_cast(memory_alloc_with_policy(effective_stack_depth * sizeof(StackType_t), &requested_policy)); + } + if (stack_buffer == nullptr) { + MemoryPolicy internal_policy = { .required = MEMORY_CAPABILITY_INTERNAL, .desired = 0, .alignment = 0 }; + stack_buffer = static_cast(memory_alloc_with_policy(effective_stack_depth * sizeof(StackType_t), &internal_policy)); + } + if (stack_buffer == nullptr) { + LOG_E(TAG, "[instance %lu] Failed to allocate app stack", app_instance_id); + vSemaphoreDelete(completion->semaphore); + delete completion; + loader->unload(runtime); + app_ledger_free_arguments(argc, argv); + return ERROR_OUT_OF_MEMORY; + } + + auto* task_tcb = static_cast(memory_alloc_with_policy(sizeof(StaticTask_t), &APP_TASK_TCB_POLICY)); + if (task_tcb == nullptr) { + LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id); + memory_free(stack_buffer); + vSemaphoreDelete(completion->semaphore); + delete completion; + loader->unload(runtime); + app_ledger_free_arguments(argc, argv); + return ERROR_OUT_OF_MEMORY; + } +#else + StackType_t* stack_buffer = nullptr; + StaticTask_t* task_tcb = nullptr; +#endif + + auto* context = new (std::nothrow) TaskContext { + .loader = loader, + .runtime = runtime, + .app_instance_id = app_instance_id, + .argc = argc, + .argv = argv, + .completion = completion, + .stackBuffer = stack_buffer, + .taskTcb = task_tcb + }; + if (context == nullptr) { - LOG_E(TAG, "Failed to allocate app"); + LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id); +#ifdef ESP_PLATFORM + memory_free(task_tcb); + memory_free(stack_buffer); +#endif vSemaphoreDelete(completion->semaphore); delete completion; loader->unload(runtime); @@ -239,14 +357,23 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, 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. // Created at idle priority so it can't preempt us before vTaskSuspend() below runs, then suspended immediately - // the ledger must record the handle (set_task()) before the task can possibly observe or erase its own entry. // (see app_scheduler_stop()'s liveness check and app_task_main()'s exit path) - BaseType_t create_result = xTaskCreate(app_task_main, task_name, 8192 / sizeof(StackType_t), context, tskIDLE_PRIORITY, &task_handle); - if (create_result != pdPASS) { +#ifdef ESP_PLATFORM + TaskHandle_t task_handle = xTaskCreateStatic(app_task_main, task_name, effective_stack_depth, context, tskIDLE_PRIORITY, stack_buffer, task_tcb); +#else + TaskHandle_t task_handle = nullptr; + if (xTaskCreate(app_task_main, task_name, effective_stack_depth, context, tskIDLE_PRIORITY, &task_handle) != pdPASS) { + task_handle = nullptr; + } +#endif + if (task_handle == nullptr) { delete context; +#ifdef ESP_PLATFORM + memory_free(task_tcb); + memory_free(stack_buffer); +#endif vSemaphoreDelete(completion->semaphore); delete completion; loader->unload(runtime); @@ -260,10 +387,17 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location, vTaskPrioritySet(task_handle, APP_TASK_PRIORITY); vTaskResume(task_handle); + memory_print_stats(); + return ERROR_NONE; } error_t app_scheduler_stop(AppInstanceId app_instance_id, TickType_t join_timeout) { + if (app_scheduler_current_app_id() == app_instance_id) { + LOG_E(TAG, "Can't call app_scheduler_stop() from the owning task"); + return ERROR_NOT_ALLOWED; + } + AppCompletionSignal* completion = acquire_completion_signal(app_instance_id); if (completion != nullptr) { AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; diff --git a/Modules/app-module/source/symbols.cpp b/Modules/app-module/source/symbols.cpp index 28711df6..8b32b26e 100644 --- a/Modules/app-module/source/symbols.cpp +++ b/Modules/app-module/source/symbols.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,6 @@ const ModuleSymbol app_module_symbols[] = { DEFINE_MODULE_SYMBOL(app_event_subscribe), DEFINE_MODULE_SYMBOL(app_event_subscribe_with_app_id), DEFINE_MODULE_SYMBOL(app_event_unsubscribe), - DEFINE_MODULE_SYMBOL(app_event_emit), DEFINE_MODULE_SYMBOL(app_event_poll), // app/install DEFINE_MODULE_SYMBOL(app_get_install_path), @@ -42,6 +42,8 @@ const ModuleSymbol app_module_symbols[] = { DEFINE_MODULE_SYMBOL(app_manager_install_path_add), DEFINE_MODULE_SYMBOL(app_manager_install_path_scan), DEFINE_MODULE_SYMBOL(app_manager_install_path_uninstall), + // app/manifest + DEFINE_MODULE_SYMBOL(app_id_is_valid), // app/metadata DEFINE_MODULE_SYMBOL(app_metadata_parse), // app/paths diff --git a/Modules/app-module/tests/source/app_event_test.cpp b/Modules/app-module/tests/source/app_event_test.cpp index 43795aef..526f13d3 100644 --- a/Modules/app-module/tests/source/app_event_test.cpp +++ b/Modules/app-module/tests/source/app_event_test.cpp @@ -6,6 +6,11 @@ #include #include +// app_event_emit() is declared in app-module's private app/private/event.h (PRIV_INCLUDE_DIRS, +// not exposed to this test target) - declared directly here, same as app_manager_test.cpp does +// for app_internal_loader_service_manifest. +extern "C" error_t app_event_emit(AppInstanceId app_instance_id, const struct AppEvent* event); + TEST_CASE("app_event_subscribe/_poll deliver events in FIFO order") { TaskEventGroup event_group {}; task_event_group_construct(&event_group); diff --git a/Modules/app-module/tests/source/app_manager_test.cpp b/Modules/app-module/tests/source/app_manager_test.cpp index 85a89410..1c2e7770 100644 --- a/Modules/app-module/tests/source/app_manager_test.cpp +++ b/Modules/app-module/tests/source/app_manager_test.cpp @@ -451,6 +451,42 @@ TEST_CASE("app_manager_get_topmost_instance_id returns NOT_FOUND when nothing is app_manager_remove("test.app.top_b"); } +TEST_CASE("app_manager_start honors a custom AppManifest::stack.depth") { + ensure_fake_loader_registered(); + + AppManifest manifest { "test.app.stack.custom", "Stack Custom", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + manifest.stack.depth = 4096; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t instance_id = 0; + REQUIRE_EQ(app_manager_start("test.app.stack.custom", &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.stack.custom"); +} + +TEST_CASE("app_manager_start still works when AppManifest::stack is left at its zero-value default") { + ensure_fake_loader_registered(); + + // stack.depth == 0 - app_scheduler_start() must fall back to its own default stack depth + // rather than fail or create a zero-sized stack. + AppManifest manifest { "test.app.stack.default", "Stack Default", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(manifest.stack.depth, 0); + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + + uint32_t instance_id = 0; + REQUIRE_EQ(app_manager_start("test.app.stack.default", &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.stack.default"); +} + 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(); diff --git a/Modules/app-module/tests/source/combined_event_test.cpp b/Modules/app-module/tests/source/combined_event_test.cpp index 88ead21b..2f8f95fd 100644 --- a/Modules/app-module/tests/source/combined_event_test.cpp +++ b/Modules/app-module/tests/source/combined_event_test.cpp @@ -4,6 +4,11 @@ #include +// app_event_emit() is declared in app-module's private app/private/event.h (PRIV_INCLUDE_DIRS, +// not exposed to this test target) - declared directly here, same as app_manager_test.cpp does +// for app_internal_loader_service_manifest. +extern "C" error_t app_event_emit(AppInstanceId app_instance_id, const struct AppEvent* event); + // Regression coverage for the primary motivation behind TaskEventGroup: a task subscribed to // both an app_event and a system_event must be able to block once and wake for either, without // losing an event or regressing either subsystem's own delivery semantics (FIFO for app_event, diff --git a/Tactility/Include/Tactility/PanicHandler.h b/Tactility/Include/Tactility/PanicHandler.h index 9b143f7e..50c1a4f8 100644 --- a/Tactility/Include/Tactility/PanicHandler.h +++ b/Tactility/Include/Tactility/PanicHandler.h @@ -6,6 +6,7 @@ #define CRASH_DATA_CALLSTACK_LIMIT 64 #define CRASH_DATA_INCLUDES_SP false +#define CRASH_DATA_REASON_LENGTH 128 /** Represents a single frame on the callstack. */ struct CallstackFrame { @@ -15,11 +16,25 @@ struct CallstackFrame { #endif }; +/** Broad category of what caused the panic (mirrors ESP-IDF's panic_exception_t). */ +enum class CrashCause : uint8_t { + Unknown, + Debug, + WatchdogInterrupt, + WatchdogTask, + Abort, + Fault, +}; + /** Callstack-related crash data. */ struct CrashData { bool callstackCorrupted = false; uint8_t callstackLength = 0; CallstackFrame callstack[CRASH_DATA_CALLSTACK_LIMIT]; + + CrashCause cause = CrashCause::Unknown; + uint32_t faultAddress = 0; + char reason[CRASH_DATA_REASON_LENGTH] = { 0 }; }; /** @return the crash data */ diff --git a/Tactility/Private/Tactility/app/crashdiagnostics/QrUrl.h b/Tactility/Private/Tactility/app/crashdiagnostics/QrUrl.h index 1fe6b671..72d22333 100644 --- a/Tactility/Private/Tactility/app/crashdiagnostics/QrUrl.h +++ b/Tactility/Private/Tactility/app/crashdiagnostics/QrUrl.h @@ -1,5 +1,6 @@ #pragma once #include +#include -std::string getUrlFromCrashData(); +std::string getUrlFromCrashData(const CrashData& data); diff --git a/Tactility/Source/DeprecatedPaths.cpp b/Tactility/Source/DeprecatedPaths.cpp index 5aa78fac..1d63a982 100644 --- a/Tactility/Source/DeprecatedPaths.cpp +++ b/Tactility/Source/DeprecatedPaths.cpp @@ -1,6 +1,6 @@ #include -#include "../../Modules/app-module/private/app/private/app_metadata_parsing_internal.h" +#include #include @@ -72,12 +72,12 @@ std::string getUserHomePath() { } std::string getAppInstallPath(const std::string& appId) { - assert(app_metadata_is_valid_id(appId.c_str())); + assert(app_id_is_valid(appId.c_str())); return std::format("{}/{}", getAppInstallPath(), appId); } std::string getAppUserPath(const std::string& appId) { - assert(app_metadata_is_valid_id(appId.c_str())); + assert(app_id_is_valid(appId.c_str())); return std::format("{}/app/{}", getUserHomePath(), appId); } diff --git a/Tactility/Source/PanicHandler.cpp b/Tactility/Source/PanicHandler.cpp index ac20d4b1..375bc998 100644 --- a/Tactility/Source/PanicHandler.cpp +++ b/Tactility/Source/PanicHandler.cpp @@ -1,14 +1,21 @@ +#if defined(ESP_PLATFORM) +#include +#endif + #if defined(ESP_PLATFORM) && defined(CONFIG_IDF_TARGET_ARCH_XTENSA) -#include "Tactility/kernel/PanicHandler.h" +#include -#include #include -#include #include #include +#include +#include +#include #include +#include + extern "C" { /** @@ -28,7 +35,34 @@ void __wrap_esp_panic_handler(void* info) { .exc_frame = nullptr }; + const auto* panic_info = static_cast(info); + + switch (panic_info->exception) { + // Watchdag timer issues are not consider real crashes: they trigger relatively often + // and could cause a previous real crash to be overwritten by a watchdog timer warning during reboot. + case PANIC_EXCEPTION_IWDT: crashData.cause = CrashCause::WatchdogInterrupt; return; + case PANIC_EXCEPTION_TWDT: crashData.cause = CrashCause::WatchdogTask; return; + // We also don't care about debugger errors: + case PANIC_EXCEPTION_DEBUG: crashData.cause = CrashCause::Debug; return; + // We only care about 'real' crashes: + case PANIC_EXCEPTION_ABORT: crashData.cause = CrashCause::Abort; break; + case PANIC_EXCEPTION_FAULT: + default: crashData.cause = CrashCause::Fault; break; + } + crashData.callstackLength = 0; + crashData.faultAddress = reinterpret_cast(panic_info->addr); + + // g_panic_abort_details carries the actual assert()/abort() message when present; panic_info->reason + // is ESP-IDF's generic description otherwise (e.g. "IllegalInstruction"). + const char* reason = (panic_info->exception == PANIC_EXCEPTION_ABORT && g_panic_abort_details != nullptr) + ? g_panic_abort_details + : panic_info->reason; + crashData.reason[0] = '\0'; + if (reason != nullptr) { + strncpy(crashData.reason, reason, sizeof(crashData.reason) - 1); + crashData.reason[sizeof(crashData.reason) - 1] = '\0'; + } esp_backtrace_get_start(&frame.pc, &frame.sp, &frame.next_pc); crashData.callstack[0].pc = frame.pc; diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index 74f15140..6e4acaa7 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -355,12 +355,8 @@ static void stopAppFromToolbar(lv_event_t*) { // every not-yet-converted app's toolbar still relies on). AppInstanceId topmost = 0; check(app_manager_get_topmost_instance_id(&topmost) == ERROR_NONE); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that - // bound-waits (thread_join) for the app's own thread to finish, which needs the LVGL - // lock to clean up - but this callback runs ON the LVGL task, which would deadlock - // against itself. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(topmost, &event); + + app_event_emit_close(topmost); } // The on-screen keyboard widget itself, constructed during windowManagerScreenInit diff --git a/Tactility/Source/app/addgps/AddGps.cpp b/Tactility/Source/app/addgps/AddGps.cpp index 03448dd9..f3c25723 100644 --- a/Tactility/Source/app/addgps/AddGps.cpp +++ b/Tactility/Source/app/addgps/AddGps.cpp @@ -54,12 +54,7 @@ std::vector getModelNames() { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onAddGpsPressed(lv_event_t* event) { diff --git a/Tactility/Source/app/alertdialog/AlertDialog.cpp b/Tactility/Source/app/alertdialog/AlertDialog.cpp index 88c31a4c..0ff79276 100644 --- a/Tactility/Source/app/alertdialog/AlertDialog.cpp +++ b/Tactility/Source/app/alertdialog/AlertDialog.cpp @@ -51,13 +51,7 @@ void onButtonPressed(lv_event_t* e) { auto* btnCtx = static_cast(lv_event_get_user_data(e)); LOG_I(TAG, "Selected item at index %d", (int)btnCtx->index); btnCtx->ctx->result = btnCtx->index; - // Async, non-blocking - just wakes this dialog's own thread. Must NOT call - // app_manager_stop() here: that bound-waits (thread_join) for the dialog's thread to - // finish, which needs the LVGL lock (window_manager_remove()) - but this callback is - // running ON the LVGL task, which would deadlock against itself. The caller reaps this - // instance via app_manager_stop() after it receives the APP_EVENT_RESULT instead. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(btnCtx->ctx->appInstanceId, &event); + app_event_emit_close(btnCtx->ctx->appInstanceId); } void createButton(Context* ctx, lv_obj_t* parent, const std::string& text, int32_t index) { diff --git a/Tactility/Source/app/appdetails/AppDetails.cpp b/Tactility/Source/app/appdetails/AppDetails.cpp index c919d130..d716511b 100644 --- a/Tactility/Source/app/appdetails/AppDetails.cpp +++ b/Tactility/Source/app/appdetails/AppDetails.cpp @@ -50,12 +50,7 @@ void onPressUninstall(lv_event_t* event) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/apphub/AppHubApp.cpp b/Tactility/Source/app/apphub/AppHubApp.cpp index 5f1910f4..e25dd1ef 100644 --- a/Tactility/Source/app/apphub/AppHubApp.cpp +++ b/Tactility/Source/app/apphub/AppHubApp.cpp @@ -51,8 +51,7 @@ void refresh(Context* ctx); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - AppEvent closeEvent {.type = APP_EVENT_CLOSE, .timestamp = 0, .result = {}}; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onAppPressed(lv_event_t* e) { diff --git a/Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp b/Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp index 49528122..b477cc57 100644 --- a/Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp +++ b/Tactility/Source/app/apphubdetails/AppHubDetailsApp.cpp @@ -63,8 +63,7 @@ uint32_t showConfirmDialog(Context* ctx, const char* action) { void onBackPressed(lv_event_t* e) { auto* ctx = static_cast(lv_event_get_user_data(e)); - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onInstallPressed(lv_event_t* e) { diff --git a/Tactility/Source/app/applist/AppList.cpp b/Tactility/Source/app/applist/AppList.cpp index cdab48a4..d2495132 100644 --- a/Tactility/Source/app/applist/AppList.cpp +++ b/Tactility/Source/app/applist/AppList.cpp @@ -20,7 +20,9 @@ namespace tt::app::applist { namespace { -uint32_t appListInstanceId = 0; +struct Context { + uint32_t appInstanceId; +}; void onAppPressed(lv_event_t* e) { // Fire-and-forget top-level navigation, same as Launcher's own app-launch buttons. @@ -29,15 +31,9 @@ void onAppPressed(lv_event_t* e) { app_manager_start(manifest->id, &instanceId); } -void onBackPressed(lv_event_t*) { - // The global toolbar nav callback (ToolbarConfig.nav_action_callback, set once in - // Tactility.cpp) only knows how to stop old-model apps, so this new-model app overrides - // its own toolbar's nav action to close itself instead. Async, non-blocking - must NOT - // call app_manager_stop() directly here: that bound-waits (thread_join) for this app's - // own thread to finish, which needs the LVGL lock (window_manager_remove()) - but this - // callback runs ON the LVGL task, which would deadlock against itself. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(appListInstanceId, &event); +void onBackPressed(lv_event_t* event) { + auto* ctx = static_cast(lv_event_get_user_data(event)); + app_event_emit_close(ctx->appInstanceId); } void createAppWidget(const ::AppManifest* manifest, lv_obj_t* list) { @@ -54,9 +50,11 @@ void collectManifest(const ::AppManifest* manifest, void* context) { manifests->push_back(manifest); } -void createWidgets(lv_obj_t* parent, void*) { +void createWidgets(lv_obj_t* parent, void* userData) { + auto* ctx = static_cast(userData); + auto* toolbar = lvgl_toolbar_create(parent, "Apps"); - lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, nullptr); + lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, ctx); lv_obj_align(toolbar, LV_ALIGN_TOP_MID, 0, 0); lv_obj_t* list = lv_list_create(parent); @@ -83,7 +81,7 @@ void createWidgets(lv_obj_t* parent, void*) { int32_t appMain(int argc, char* argv[]) { uint32_t appInstanceId = app_scheduler_current_app_id(); - appListInstanceId = appInstanceId; + Context ctx { appInstanceId }; TaskEventGroup event_group {}; task_event_group_construct(&event_group); @@ -91,7 +89,7 @@ int32_t appMain(int argc, char* argv[]) { AppEventSubscription sub {}; check(app_event_subscribe(&sub, &event_group) == ERROR_NONE); - WindowId window = window_manager_create(appInstanceId, createWidgets, nullptr); + WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx); while (true) { task_event_group_wait_any(&event_group, nullptr, portMAX_DELAY); @@ -119,8 +117,9 @@ extern const ::AppManifest manifest = { .id = "tactility.applist", .name = "Apps", .category = APP_CATEGORY_SYSTEM, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) }, + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, .flags = APP_MANIFEST_FLAG_HIDDEN, + .stack = { .depth = 2400, .desired_memory_capability = 0 }, }; } // namespace diff --git a/Tactility/Source/app/appsettings/AppSettings.cpp b/Tactility/Source/app/appsettings/AppSettings.cpp index 7bfc2155..e767bbdf 100644 --- a/Tactility/Source/app/appsettings/AppSettings.cpp +++ b/Tactility/Source/app/appsettings/AppSettings.cpp @@ -24,21 +24,18 @@ extern const ::AppManifest manifest; namespace { -// Set by appMain() right before window_manager_create(), read by onBackPressed(). -uint32_t appSettingsInstanceId = 0; +struct Context { + uint32_t appInstanceId; +}; void onAppPressed(lv_event_t* e) { const auto* target_manifest = static_cast(lv_event_get_user_data(e)); appdetails::start(target_manifest->id); } -void onBackPressed(lv_event_t*) { - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(appSettingsInstanceId, &event); +void onBackPressed(lv_event_t* event) { + auto* ctx = static_cast(lv_event_get_user_data(event)); + app_event_emit_close(ctx->appInstanceId); } void createAppWidget(const ::AppManifest* target_manifest, lv_obj_t* list) { @@ -55,10 +52,12 @@ void collectManifest(const ::AppManifest* manifest, void* context) { manifests->push_back(manifest); } -void createWidgets(lv_obj_t* parent, void*) { +void createWidgets(lv_obj_t* parent, void* userData) { + auto* ctx = static_cast(userData); + auto* toolbar = lvgl_toolbar_create(parent, "Installed Apps"); // The global toolbar nav callback only knows how to stop old-model apps. - lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, nullptr); + lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, ctx); lv_obj_align(toolbar, LV_ALIGN_TOP_MID, 0, 0); lv_obj_t* list = lv_list_create(parent); @@ -92,7 +91,7 @@ void createWidgets(lv_obj_t* parent, void*) { int32_t appMain(int argc, char* argv[]) { uint32_t appInstanceId = app_scheduler_current_app_id(); - appSettingsInstanceId = appInstanceId; + Context ctx { appInstanceId }; TaskEventGroup event_group {}; task_event_group_construct(&event_group); @@ -100,7 +99,7 @@ int32_t appMain(int argc, char* argv[]) { AppEventSubscription sub {}; check(app_event_subscribe(&sub, &event_group) == ERROR_NONE); - WindowId window = window_manager_create(appInstanceId, createWidgets, nullptr); + WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx); bool shouldClose = false; while (!shouldClose) { @@ -132,7 +131,9 @@ extern const ::AppManifest manifest = { .id = "tactility.appsettings", .name = "Apps", .category = APP_CATEGORY_SETTINGS, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) } + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, + .flags = 0, + .stack = { .depth = 2400, .desired_memory_capability = 0 }, }; } // namespace diff --git a/Tactility/Source/app/apwebserver/ApWebServer.cpp b/Tactility/Source/app/apwebserver/ApWebServer.cpp index 8ede703d..43452512 100644 --- a/Tactility/Source/app/apwebserver/ApWebServer.cpp +++ b/Tactility/Source/app/apwebserver/ApWebServer.cpp @@ -37,12 +37,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/audiosettings/AudioSettings.cpp b/Tactility/Source/app/audiosettings/AudioSettings.cpp index 0d37cd25..a7a248e9 100644 --- a/Tactility/Source/app/audiosettings/AudioSettings.cpp +++ b/Tactility/Source/app/audiosettings/AudioSettings.cpp @@ -37,12 +37,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onInputEnabledSwitch(lv_event_t* event) { diff --git a/Tactility/Source/app/boot/Boot.cpp b/Tactility/Source/app/boot/Boot.cpp index 33381c3a..de443902 100644 --- a/Tactility/Source/app/boot/Boot.cpp +++ b/Tactility/Source/app/boot/Boot.cpp @@ -1,3 +1,4 @@ +#include "tactility/memory.h" #include "tactility/system_event.h" #include @@ -274,16 +275,27 @@ void runBootSequence(TickType_t startTime) { } #endif - if (!setupUsbBootMode()) { - registerApps(); - waitForMinimalSplashDuration(startTime); - startNextApp(); + if (setupUsbBootMode()) { + // Stay open: the splash's "Return to OS" button is this app's only way to leave mass + // storage mode, so it must not self-close here like the normal boot path does below. + return; + } + + registerApps(); + waitForMinimalSplashDuration(startTime); + startNextApp(); + + if (sdCardMissing) { + // Stay open: the error screen's "Reboot" button is this app's only way to leave here. + return; } // This event will likely block as other systems are initialized // e.g. Wi-Fi reads AP configs from SD card LOG_I(TAG, "Publish event"); system_event_emit(KERNEL_EVENT_BOOT_COMPLETED, nullptr, 0); + + app_event_emit_close(app_scheduler_current_app_id()); } int32_t appMain(int argc, char* argv[]) { @@ -337,8 +349,9 @@ extern const ::AppManifest manifest = { .id = "tactility.boot", .name = "Boot", .category = APP_CATEGORY_SYSTEM, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) }, + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, .flags = APP_MANIFEST_FLAG_HIDDEN, + .stack = { .depth = 4096, .desired_memory_capability = MEMORY_CAPABILITY_INTERNAL } }; } // namespace diff --git a/Tactility/Source/app/btmanage/BtManage.cpp b/Tactility/Source/app/btmanage/BtManage.cpp index 6e5dc5ae..112aac05 100644 --- a/Tactility/Source/app/btmanage/BtManage.cpp +++ b/Tactility/Source/app/btmanage/BtManage.cpp @@ -133,8 +133,7 @@ void onBtEvent(Context* ctx, const BtEvent& event) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/btmanage/View.cpp b/Tactility/Source/app/btmanage/View.cpp index bb16aaf5..f6a76e1c 100644 --- a/Tactility/Source/app/btmanage/View.cpp +++ b/Tactility/Source/app/btmanage/View.cpp @@ -20,12 +20,7 @@ namespace tt::app::btmanage { static void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } static void onEnableSwitchChanged(lv_event_t* event) { diff --git a/Tactility/Source/app/btpeersettings/BtPeerSettings.cpp b/Tactility/Source/app/btpeersettings/BtPeerSettings.cpp index 9fbc3dda..d67fbec2 100644 --- a/Tactility/Source/app/btpeersettings/BtPeerSettings.cpp +++ b/Tactility/Source/app/btpeersettings/BtPeerSettings.cpp @@ -93,12 +93,7 @@ void onPressForget(lv_event_t* event) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onToggleAutoConnect(lv_event_t* event) { diff --git a/Tactility/Source/app/chat/ChatView.cpp b/Tactility/Source/app/chat/ChatView.cpp index 9fcffd6e..2fbca38f 100644 --- a/Tactility/Source/app/chat/ChatView.cpp +++ b/Tactility/Source/app/chat/ChatView.cpp @@ -148,12 +148,7 @@ void ChatView::createChannelPanel(lv_obj_t* parent) { void ChatView::onBackPressed(lv_event_t* e) { auto* self = static_cast(lv_event_get_user_data(e)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(self->app->appInstanceId, &closeEvent); + app_event_emit_close(self->app->appInstanceId); } void ChatView::init(lv_obj_t* parent) { diff --git a/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp b/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp index bd73cfc5..02d43146 100644 --- a/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp +++ b/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp @@ -1,8 +1,12 @@ #ifdef ESP_PLATFORM +#include "Tactility/PanicHandler.h" + + #include #include #include +#include #include #include @@ -17,8 +21,19 @@ #include #include #include +#include +#if CONFIG_IDF_TARGET_ARCH_XTENSA +#include +#else +#include +#endif + +#include + +#include #include +#include namespace tt::app::crashdiagnostics { @@ -41,16 +56,70 @@ struct Context { }; +const char* crashCauseToString(CrashCause cause) { + switch (cause) { + case CrashCause::Debug: return "Debug"; + case CrashCause::WatchdogInterrupt: return "Watchdog (interrupt)"; + case CrashCause::WatchdogTask: return "Watchdog (task)"; + case CrashCause::Abort: return "Abort"; + case CrashCause::Fault: return "Fault"; + case CrashCause::Unknown: + default: return "Unknown"; + } +} + +std::string formatCrashData(const CrashData& crashData) { + std::stringstream stream; + + stream << "Cause: " << crashCauseToString(crashData.cause) << "\n"; + + stream << "Reason: "; + if (crashData.reason[0] != '\0') { + stream << crashData.reason; + } else { + stream << "unknown"; + } + stream << "\n"; + + stream << "Fault address: " << std::hex << std::setw(8) << std::setfill('0') << crashData.faultAddress << std::dec << "\n"; + + stream << "Callstack" << (crashData.callstackCorrupted ? " (corrupted)" : "") << ":"; + if (crashData.callstackLength > 0) { + stream << "\n"; + for (uint8_t i = 0; i < crashData.callstackLength; i++) { +#if CONFIG_IDF_TARGET_ARCH_XTENSA + uint32_t pc = esp_cpu_process_stack_pc(crashData.callstack[i].pc); +#else + uint32_t pc = crashData.callstack[i].pc; // No processing needed on RISC-V +#endif + stream << std::hex << std::setw(8) << std::setfill('0') << pc << std::dec << " "; + } + } else { + stream << " empty" << "\n"; + } + + return stream.str(); +} + +// Best-effort: crash.txt is a convenience for offline inspection, not required for the app to work. +void writeCrashLogFile(const CrashData& crashData) { + char root[128]; + if (paths_get_data_path(root, sizeof(root)) != ERROR_NONE) { + LOG_E(TAG, "Failed to resolve data path for crash.txt"); + return; + } + + std::string path = std::string(root) + "/crash.txt"; + file::FileMutexGuard guard(path); + if (!file::writeString(path, formatCrashData(crashData))) { + LOG_E(TAG, "Failed to write %s", path.c_str()); + } +} + void onContinuePressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); ctx->continuePressed = true; - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. launcher::start() is deferred to appMain(), after this app's - // own thread has finished cleaning up. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { @@ -72,7 +141,9 @@ void createWidgets(lv_obj_t* parent, void* userData) { } lv_obj_align(bottom_label, LV_ALIGN_BOTTOM_MID, 0, -2); - std::string url = getUrlFromCrashData(); + const auto& crash_data = getRtcCrashData(); + + std::string url = getUrlFromCrashData(crash_data); LOG_I(TAG, "%s", url.c_str()); size_t url_length = url.length(); @@ -156,6 +227,8 @@ int32_t appMain(int argc, char* argv[]) { Context ctx {}; ctx.appInstanceId = appInstanceId; + writeCrashLogFile(getRtcCrashData()); + TaskEventGroup event_group {}; task_event_group_construct(&event_group); @@ -207,7 +280,7 @@ extern const ::AppManifest manifest = { .id = "tactility.crashdiagnostics", .name = "Crash Diagnostics", .category = APP_CATEGORY_SYSTEM, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) }, + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, .flags = APP_MANIFEST_FLAG_HIDDEN, }; diff --git a/Tactility/Source/app/crashdiagnostics/QrUrl.cpp b/Tactility/Source/app/crashdiagnostics/QrUrl.cpp index 576eaef6..e98785ed 100644 --- a/Tactility/Source/app/crashdiagnostics/QrUrl.cpp +++ b/Tactility/Source/app/crashdiagnostics/QrUrl.cpp @@ -1,7 +1,6 @@ #ifdef ESP_PLATFORM #include -#include #include #include @@ -14,8 +13,7 @@ #include -std::string getUrlFromCrashData() { - auto crash_data = getRtcCrashData(); +std::string getUrlFromCrashData(const CrashData& crash_data) { std::vector stack_buffer(crash_data.callstackLength * 2); for (int i = 0; i < crash_data.callstackLength; ++i) { const CallstackFrame&frame = crash_data.callstack[i]; diff --git a/Tactility/Source/app/development/Development.cpp b/Tactility/Source/app/development/Development.cpp index 6f69dc67..b4fae679 100644 --- a/Tactility/Source/app/development/Development.cpp +++ b/Tactility/Source/app/development/Development.cpp @@ -46,12 +46,7 @@ void updateViewState(Context* ctx); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onEnableSwitchChanged(lv_event_t* event) { diff --git a/Tactility/Source/app/display/Display.cpp b/Tactility/Source/app/display/Display.cpp index 51fb52bb..8bf2d332 100644 --- a/Tactility/Source/app/display/Display.cpp +++ b/Tactility/Source/app/display/Display.cpp @@ -61,12 +61,7 @@ Device* getBacklightDevice() { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onBacklightSliderEvent(lv_event_t* event) { diff --git a/Tactility/Source/app/files/View.cpp b/Tactility/Source/app/files/View.cpp index fafb3f39..58292d10 100644 --- a/Tactility/Source/app/files/View.cpp +++ b/Tactility/Source/app/files/View.cpp @@ -376,12 +376,7 @@ void View::createDirEntryWidget(lv_obj_t* list, dirent& dir_entry) { } void View::onBackPressed() { - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(appInstanceId, &event); + app_event_emit_close(appInstanceId); } void View::onNavigateUpPressed() { diff --git a/Tactility/Source/app/fileselection/FileSelection.cpp b/Tactility/Source/app/fileselection/FileSelection.cpp index 96f4f31b..be89e961 100644 --- a/Tactility/Source/app/fileselection/FileSelection.cpp +++ b/Tactility/Source/app/fileselection/FileSelection.cpp @@ -62,8 +62,7 @@ int32_t appMain(int argc, char* argv[]) { // app_manager_stop() after it receives the APP_EVENT_RESULT instead. lastPath = path; ctx.result = 0; - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(appInstanceId, &closeEvent); + app_event_emit_close(appInstanceId); }); TaskEventGroup event_group {}; diff --git a/Tactility/Source/app/fileselection/View.cpp b/Tactility/Source/app/fileselection/View.cpp index b0aa85be..a5776bad 100644 --- a/Tactility/Source/app/fileselection/View.cpp +++ b/Tactility/Source/app/fileselection/View.cpp @@ -41,12 +41,7 @@ static void onNavigateUpPressedCallback(lv_event_t* event) { void View::onBackPressedCallback(lv_event_t* event) { auto* view = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(view->appInstanceId, &closeEvent); + app_event_emit_close(view->appInstanceId); } void View::onTapFile(const std::string& path, const std::string& filename) { diff --git a/Tactility/Source/app/gpssettings/GpsSettings.cpp b/Tactility/Source/app/gpssettings/GpsSettings.cpp index 8bb66e2b..d24624cc 100644 --- a/Tactility/Source/app/gpssettings/GpsSettings.cpp +++ b/Tactility/Source/app/gpssettings/GpsSettings.cpp @@ -64,12 +64,7 @@ void createWidgets(lv_obj_t* parent, void* userData); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onAddGpsPressed(lv_event_t* event) { diff --git a/Tactility/Source/app/grovesettings/GroveSettings.cpp b/Tactility/Source/app/grovesettings/GroveSettings.cpp index 84614357..730a92b1 100644 --- a/Tactility/Source/app/grovesettings/GroveSettings.cpp +++ b/Tactility/Source/app/grovesettings/GroveSettings.cpp @@ -45,12 +45,7 @@ void onModeChanged(lv_event_t* e) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/i2cscanner/I2cScanner.cpp b/Tactility/Source/app/i2cscanner/I2cScanner.cpp index 7f7f76b1..0f49507e 100644 --- a/Tactility/Source/app/i2cscanner/I2cScanner.cpp +++ b/Tactility/Source/app/i2cscanner/I2cScanner.cpp @@ -284,12 +284,7 @@ void selectBus(Context* ctx, int32_t selected) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onSelectBus(lv_event_t* event) { diff --git a/Tactility/Source/app/imageviewer/ImageViewer.cpp b/Tactility/Source/app/imageviewer/ImageViewer.cpp index bf47bda6..a26bbbd0 100644 --- a/Tactility/Source/app/imageviewer/ImageViewer.cpp +++ b/Tactility/Source/app/imageviewer/ImageViewer.cpp @@ -32,12 +32,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/inputdialog/InputDialog.cpp b/Tactility/Source/app/inputdialog/InputDialog.cpp index 459dc099..4e3e5724 100644 --- a/Tactility/Source/app/inputdialog/InputDialog.cpp +++ b/Tactility/Source/app/inputdialog/InputDialog.cpp @@ -59,10 +59,7 @@ void onButtonPressed(lv_event_t* e) { LOG_I(TAG, "Cancel pressed"); btnCtx->ctx->result = 1; } - // Async, non-blocking - see AlertDialog.cpp's onButtonPressed() for why this must not - // call app_manager_stop() directly (would deadlock against the LVGL lock). - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(btnCtx->ctx->appInstanceId, &event); + app_event_emit_close(btnCtx->ctx->appInstanceId); } void createButton(Context* ctx, lv_obj_t* parent, const std::string& text, lv_obj_t* textarea) { diff --git a/Tactility/Source/app/keyboard/KeyboardSettings.cpp b/Tactility/Source/app/keyboard/KeyboardSettings.cpp index 8738bbbf..86b32223 100644 --- a/Tactility/Source/app/keyboard/KeyboardSettings.cpp +++ b/Tactility/Source/app/keyboard/KeyboardSettings.cpp @@ -59,12 +59,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onBacklightSwitch(lv_event_t* e) { diff --git a/Tactility/Source/app/launcher/Launcher.cpp b/Tactility/Source/app/launcher/Launcher.cpp index 99553803..f899a5e4 100644 --- a/Tactility/Source/app/launcher/Launcher.cpp +++ b/Tactility/Source/app/launcher/Launcher.cpp @@ -1,6 +1,3 @@ -#include "tactility/drivers/pointer.h" - - #include #include #include @@ -17,8 +14,10 @@ #include #include +#include #include #include +#include #include #include @@ -273,8 +272,10 @@ extern const ::AppManifest manifest = { .id = "tactility.launcher", .name = "Launcher", .category = APP_CATEGORY_SYSTEM, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) }, + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, .flags = APP_MANIFEST_FLAG_HIDDEN, + // No file IO, so callstack can be in external RAM + .stack = { .depth = 3072 , .desired_memory_capability = MEMORY_CAPABILITY_EXTERNAL } }; // Kept for Tactility/Private/Tactility/app/launcher/Launcher.h's existing declaration (still diff --git a/Tactility/Source/app/localesettings/LocaleSettings.cpp b/Tactility/Source/app/localesettings/LocaleSettings.cpp index 186eee5a..46767fff 100644 --- a/Tactility/Source/app/localesettings/LocaleSettings.cpp +++ b/Tactility/Source/app/localesettings/LocaleSettings.cpp @@ -99,12 +99,7 @@ void onLanguageSet(lv_event_t* event) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/power/Power.cpp b/Tactility/Source/app/power/Power.cpp index 6b1e28af..f193817e 100644 --- a/Tactility/Source/app/power/Power.cpp +++ b/Tactility/Source/app/power/Power.cpp @@ -104,12 +104,7 @@ void updateUi(Context* ctx) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onPowerEnabledChanged(lv_event_t* event) { diff --git a/Tactility/Source/app/poweroff/PowerOff.cpp b/Tactility/Source/app/poweroff/PowerOff.cpp index 81ec9451..f8252203 100644 --- a/Tactility/Source/app/poweroff/PowerOff.cpp +++ b/Tactility/Source/app/poweroff/PowerOff.cpp @@ -99,12 +99,7 @@ void onYesPressed(lv_event_t* /*event*/) { void onNoPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/screenshot/Screenshot.cpp b/Tactility/Source/app/screenshot/Screenshot.cpp index 93fe098f..6e2a7916 100644 --- a/Tactility/Source/app/screenshot/Screenshot.cpp +++ b/Tactility/Source/app/screenshot/Screenshot.cpp @@ -66,12 +66,7 @@ void updateScreenshotMode(Context* ctx) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onStartPressed(lv_event_t* event) { diff --git a/Tactility/Source/app/selectiondialog/SelectionDialog.cpp b/Tactility/Source/app/selectiondialog/SelectionDialog.cpp index c0b5d7b5..96442721 100644 --- a/Tactility/Source/app/selectiondialog/SelectionDialog.cpp +++ b/Tactility/Source/app/selectiondialog/SelectionDialog.cpp @@ -47,13 +47,7 @@ void onItemSelected(lv_event_t* e) { auto* itemCtx = static_cast(lv_event_get_user_data(e)); LOG_I(TAG, "Selected item at index %d", (int)itemCtx->index); itemCtx->ctx->result = itemCtx->index; - // Async, non-blocking - just wakes this dialog's own thread. Must NOT call - // app_manager_stop() here: that bound-waits (thread_join) for the dialog's thread to - // finish, which needs the LVGL lock (window_manager_remove()) - but this callback is - // running ON the LVGL task, which would deadlock against itself. The caller reaps this - // instance via app_manager_stop() after it receives the APP_EVENT_RESULT instead. - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(itemCtx->ctx->appInstanceId, &event); + app_event_emit_close(itemCtx->ctx->appInstanceId); } void createChoiceItem(Context* ctx, lv_obj_t* list, const std::string& title, int32_t index) { @@ -67,8 +61,7 @@ void createChoiceItem(Context* ctx, lv_obj_t* list, const std::string& title, in // mirrors the original's 0-items (error) and 1-item (auto-select) shortcuts. void closeWithResult(Context* ctx, int32_t result) { ctx->result = result; - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &event); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/settings/Settings.cpp b/Tactility/Source/app/settings/Settings.cpp index 22ea8513..8b05f4cc 100644 --- a/Tactility/Source/app/settings/Settings.cpp +++ b/Tactility/Source/app/settings/Settings.cpp @@ -20,7 +20,9 @@ namespace tt::app::settings { namespace { -uint32_t settingsInstanceId = 0; +struct Context { + uint32_t appInstanceId; +}; void onAppPressed(lv_event_t* e) { // Fire-and-forget top-level navigation, same as AppList's own app-launch buttons. @@ -29,13 +31,9 @@ void onAppPressed(lv_event_t* e) { app_manager_start(manifest->id, &instanceId); } -void onBackPressed(lv_event_t*) { - // The global toolbar nav callback only knows how to stop old-model apps, so this - // new-model app overrides its own toolbar's nav action to close itself instead. Async, - // non-blocking - see AppList.cpp's onBackPressed() for why this must not call - // app_manager_stop() directly (would deadlock against the LVGL lock). - AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(settingsInstanceId, &event); +void onBackPressed(lv_event_t* event) { + auto* ctx = static_cast(lv_event_get_user_data(event)); + app_event_emit_close(ctx->appInstanceId); } void createWidget(const ::AppManifest* manifest, lv_obj_t* list) { @@ -53,12 +51,14 @@ void collectManifest(const ::AppManifest* manifest, void* context) { manifests->push_back(manifest); } -void createWidgets(lv_obj_t* parent, void*) { +void createWidgets(lv_obj_t* parent, void* userData) { + auto* ctx = static_cast(userData); + lv_obj_set_flex_flow(parent, LV_FLEX_FLOW_COLUMN); lv_obj_set_style_pad_row(parent, 0, LV_STATE_DEFAULT); auto* toolbar = lvgl_toolbar_create(parent, "Settings"); - lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, nullptr); + lvgl_toolbar_set_nav_action(toolbar, LV_SYMBOL_CLOSE, onBackPressed, ctx); auto* list = lv_list_create(parent); lv_obj_set_width(list, LV_PCT(100)); @@ -79,7 +79,7 @@ void createWidgets(lv_obj_t* parent, void*) { int32_t appMain(int argc, char* argv[]) { uint32_t appInstanceId = app_scheduler_current_app_id(); - settingsInstanceId = appInstanceId; + Context ctx { appInstanceId }; TaskEventGroup event_group {}; task_event_group_construct(&event_group); @@ -87,7 +87,7 @@ int32_t appMain(int argc, char* argv[]) { AppEventSubscription sub {}; check(app_event_subscribe(&sub, &event_group) == ERROR_NONE); - WindowId window = window_manager_create(appInstanceId, createWidgets, nullptr); + WindowId window = window_manager_create(appInstanceId, createWidgets, &ctx); while (true) { task_event_group_wait_any(&event_group, nullptr, portMAX_DELAY); @@ -115,8 +115,9 @@ extern const ::AppManifest manifest = { .id = "tactility.settings", .name = "Settings", .category = APP_CATEGORY_SYSTEM, - .location = { APP_LOCATION_MEMORY, reinterpret_cast(appMain) }, + .location = { .type = APP_LOCATION_MEMORY, .location = reinterpret_cast(appMain) }, .flags = APP_MANIFEST_FLAG_HIDDEN, + .stack = { .depth = 2400, .desired_memory_capability = 0 }, }; } // namespace diff --git a/Tactility/Source/app/setup/Setup.cpp b/Tactility/Source/app/setup/Setup.cpp index 6382df63..cc209553 100644 --- a/Tactility/Source/app/setup/Setup.cpp +++ b/Tactility/Source/app/setup/Setup.cpp @@ -159,11 +159,7 @@ void onContinueClicked(lv_event_t* event) { break; case Phase::Done: { markCompleted(); - // Async, non-blocking - must NOT call app_manager_stop() directly here: this - // callback runs ON the LVGL task, and app-lifecycle transitions must happen on this - // app's own thread (woken via app_event_poll()), which closes by returning. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); break; } } diff --git a/Tactility/Source/app/systeminfo/SystemInfo.cpp b/Tactility/Source/app/systeminfo/SystemInfo.cpp index 912b261f..635291d1 100644 --- a/Tactility/Source/app/systeminfo/SystemInfo.cpp +++ b/Tactility/Source/app/systeminfo/SystemInfo.cpp @@ -305,12 +305,7 @@ void updateTasks(Context* ctx) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/timedatesettings/TimeDateSettings.cpp b/Tactility/Source/app/timedatesettings/TimeDateSettings.cpp index e706460b..8a5da22d 100644 --- a/Tactility/Source/app/timedatesettings/TimeDateSettings.cpp +++ b/Tactility/Source/app/timedatesettings/TimeDateSettings.cpp @@ -34,12 +34,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onTimeFormatChanged(lv_event_t* event) { diff --git a/Tactility/Source/app/timezone/TimeZone.cpp b/Tactility/Source/app/timezone/TimeZone.cpp index 77318c57..9ef2f9f3 100644 --- a/Tactility/Source/app/timezone/TimeZone.cpp +++ b/Tactility/Source/app/timezone/TimeZone.cpp @@ -109,8 +109,7 @@ void createListItem(Context* ctx, lv_obj_t* list, const std::string& title, size lastCode = entry.code; ctx->result = 0; // Ok - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); }, LV_EVENT_SHORT_CLICKED, buttonCtx); } @@ -186,12 +185,7 @@ void updateList(Context* ctx) { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/touchcalibration/TouchCalibration.cpp b/Tactility/Source/app/touchcalibration/TouchCalibration.cpp index dd0f0047..96867584 100644 --- a/Tactility/Source/app/touchcalibration/TouchCalibration.cpp +++ b/Tactility/Source/app/touchcalibration/TouchCalibration.cpp @@ -174,12 +174,7 @@ void onPress(lv_event_t* event) { return; } - // Async, non-blocking - must NOT call app_manager_stop() directly here: this callback runs - // ON the LVGL task, and app-lifecycle transitions must happen on this app's own thread - // (woken up via app_event_poll() below), which closes by returning. The result (Ok/Error) - // is reported by appMain() itself when it returns, based on ctx.calibrationApplied. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void createWidgets(lv_obj_t* parent, void* userData) { diff --git a/Tactility/Source/app/trackball/TrackballSettings.cpp b/Tactility/Source/app/trackball/TrackballSettings.cpp index 90ef53bb..6a60dc32 100644 --- a/Tactility/Source/app/trackball/TrackballSettings.cpp +++ b/Tactility/Source/app/trackball/TrackballSettings.cpp @@ -72,12 +72,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void applyLive(Context* ctx) { diff --git a/Tactility/Source/app/usbsettings/UsbSettings.cpp b/Tactility/Source/app/usbsettings/UsbSettings.cpp index c517a314..51ea92e2 100644 --- a/Tactility/Source/app/usbsettings/UsbSettings.cpp +++ b/Tactility/Source/app/usbsettings/UsbSettings.cpp @@ -27,12 +27,7 @@ struct Context { void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onRebootMassStorageSdmmc(lv_event_t* event) { diff --git a/Tactility/Source/app/webserversettings/WebServerSettings.cpp b/Tactility/Source/app/webserversettings/WebServerSettings.cpp index c5efd258..e7753a82 100644 --- a/Tactility/Source/app/webserversettings/WebServerSettings.cpp +++ b/Tactility/Source/app/webserversettings/WebServerSettings.cpp @@ -54,12 +54,7 @@ void createWidgets(lv_obj_t* parent, void* userData); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onWifiModeChanged(lv_event_t* e) { diff --git a/Tactility/Source/app/wifiapsettings/WifiApSettings.cpp b/Tactility/Source/app/wifiapsettings/WifiApSettings.cpp index 13cc95ad..cf4b697f 100644 --- a/Tactility/Source/app/wifiapsettings/WifiApSettings.cpp +++ b/Tactility/Source/app/wifiapsettings/WifiApSettings.cpp @@ -47,11 +47,7 @@ void updateViews(Context* ctx); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // but this callback runs ON the LVGL task, which would deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onPressForget(lv_event_t* event) { diff --git a/Tactility/Source/app/wificonnect/WifiConnect.cpp b/Tactility/Source/app/wificonnect/WifiConnect.cpp index 70b7160c..08ccf333 100644 --- a/Tactility/Source/app/wificonnect/WifiConnect.cpp +++ b/Tactility/Source/app/wificonnect/WifiConnect.cpp @@ -60,11 +60,7 @@ void setLoading(Context* ctx, bool loading); void onBackPressed(lv_event_t* event) { auto* ctx = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // but this callback runs ON the LVGL task, which would deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } void onWifiEvent(Context* ctx, WifiEvent event) { @@ -89,8 +85,7 @@ void onWifiEvent(Context* ctx, WifiEvent event) { lvgl_unlock(); if (shouldClose) { - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(ctx->appInstanceId, &closeEvent); + app_event_emit_close(ctx->appInstanceId); } } diff --git a/Tactility/Source/app/wifimanage/View.cpp b/Tactility/Source/app/wifimanage/View.cpp index 2deb6407..6fbf9695 100644 --- a/Tactility/Source/app/wifimanage/View.cpp +++ b/Tactility/Source/app/wifimanage/View.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -23,12 +22,7 @@ constexpr auto* TAG = "WifiManageView"; static void onBackPressed(lv_event_t* event) { auto* appInstanceId = static_cast(lv_event_get_user_data(event)); - // Async, non-blocking - must NOT call app_manager_stop() directly here: that bound-waits - // (thread_join) for this app's own thread to finish, which needs the LVGL lock - // (window_manager_remove()) - but this callback runs ON the LVGL task, which would - // deadlock against itself. - AppEvent closeEvent { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} }; - app_event_emit(*appInstanceId, &closeEvent); + app_event_emit_close(*appInstanceId); } static uint8_t mapRssiToPercentage(int rssi) { @@ -40,7 +34,7 @@ static uint8_t mapRssiToPercentage(int rssi) { } auto percentage = (float)(90U - abs_rssi) / 60.f * 100.f; - return (uint8_t)percentage; + return static_cast(percentage); } static void onEnableSwitchChanged(lv_event_t* event) { diff --git a/TactilityKernel/include/tactility/concurrent/thread.h b/TactilityKernel/include/tactility/concurrent/thread.h index b78a5714..230bf86d 100644 --- a/TactilityKernel/include/tactility/concurrent/thread.h +++ b/TactilityKernel/include/tactility/concurrent/thread.h @@ -50,7 +50,7 @@ Thread* thread_alloc(void); /** * @brief Creates a new thread instance with specified parameters. * @param[in] name The name of the thread. - * @param[in] stack_size The size of the thread stack in bytes. + * @param[in] stack_size The size of the task stack in bytes. * @param[in] function The main function to be executed by the thread. * @param[in] function_context A pointer to the context to be passed to the main function. * @param[in] affinity The CPU core affinity for the thread (e.g., tskNO_AFFINITY). @@ -58,7 +58,7 @@ Thread* thread_alloc(void); */ Thread* thread_alloc_full( const char* name, - configSTACK_DEPTH_TYPE stack_size, + size_t stack_size, thread_main_fn_t function, void* function_context, portBASE_TYPE affinity @@ -82,7 +82,7 @@ void thread_set_name(Thread* thread, const char* name); /** * @brief Sets the stack size for the thread. * @param[in] thread The thread instance. - * @param[in] stack_size The stack size in bytes. Must be a multiple of 4. + * @param[in] stack_size The stack size in bytes, must be a multiple of StackType_t, which varies per platform. * @note Can only be called when the thread is in the STOPPED state. */ void thread_set_stack_size(Thread* thread, size_t stack_size); diff --git a/TactilityKernel/source/concurrent/thread.cpp b/TactilityKernel/source/concurrent/thread.cpp index bb4a2be9..5a3295db 100644 --- a/TactilityKernel/source/concurrent/thread.cpp +++ b/TactilityKernel/source/concurrent/thread.cpp @@ -6,6 +6,8 @@ #include #include +#include + #include #include #include @@ -23,9 +25,9 @@ struct Thread { thread_state_callback_t stateCallback = nullptr; void* stateCallbackContext = nullptr; std::string name = "unnamed"; - enum ThreadPriority priority = THREAD_PRIORITY_NORMAL; - struct Mutex mutex = { 0 }; - configSTACK_DEPTH_TYPE stackSize = 4096; + ThreadPriority priority = THREAD_PRIORITY_NORMAL; + Mutex mutex = { 0 }; + configSTACK_DEPTH_TYPE stackDepth = 4096; portBASE_TYPE affinity = -1; Thread() { @@ -94,7 +96,7 @@ Thread* thread_alloc(void) { Thread* thread_alloc_full( const char* name, - configSTACK_DEPTH_TYPE stack_size, + size_t stack_size, thread_main_fn_t function, void* function_context, portBASE_TYPE affinity @@ -128,7 +130,8 @@ void thread_set_stack_size(Thread* thread, size_t stack_size) { thread->lock(); check(stack_size > 0); check(thread->state == THREAD_STATE_STOPPED); - thread->stackSize = stack_size; + + thread->stackDepth = stack_size / sizeof(StackType_t); thread->unlock(); } @@ -176,14 +179,13 @@ error_t thread_start(Thread* thread) { thread->lock(); check(thread->mainFunction != nullptr); check(thread->state == THREAD_STATE_STOPPED); - check(thread->stackSize); + check(thread->stackDepth); thread->unlock(); thread_set_state_internal(thread, THREAD_STATE_STARTING); thread->lock(); - uint32_t stack_depth = thread->stackSize / sizeof(StackType_t); - enum ThreadPriority priority = thread->priority; + auto priority = static_cast(thread->priority); portBASE_TYPE affinity = thread->affinity; thread->unlock(); @@ -193,9 +195,9 @@ error_t thread_start(Thread* thread) { result = xTaskCreatePinnedToCore( thread_main_body, thread->name.c_str(), - stack_depth, + thread->stackDepth, thread, - (UBaseType_t)priority, + priority, &thread->taskHandle, affinity ); @@ -203,9 +205,9 @@ error_t thread_start(Thread* thread) { result = xTaskCreate( thread_main_body, thread->name.c_str(), - stack_depth, + thread->stackDepth, thread, - (UBaseType_t)priority, + priority, &thread->taskHandle ); #endif @@ -213,9 +215,9 @@ error_t thread_start(Thread* thread) { result = xTaskCreate( thread_main_body, thread->name.c_str(), - stack_depth, + thread->stackDepth, thread, - (UBaseType_t)priority, + priority, &thread->taskHandle ); } diff --git a/TactilityKernel/source/memory_esp32.cpp b/TactilityKernel/source/memory_esp32.cpp index 6228779b..82d4f0bb 100644 --- a/TactilityKernel/source/memory_esp32.cpp +++ b/TactilityKernel/source/memory_esp32.cpp @@ -34,20 +34,25 @@ void* memory_alloc_with_policy(size_t size, const struct MemoryPolicy* policy) { uint32_t required_caps = toHeapCaps(policy->required); uint32_t desired_caps = toHeapCaps(policy->desired); - // heap_caps matches heaps via (heap->caps[prio] & caps) != 0 - a caps value of 0 (e.g. - // required_caps when policy->required wasn't set) can never match any heap, so the fallback - // must OR in MALLOC_CAP_DEFAULT to actually reach a general-purpose heap, same as ESP-IDF's - // own heap_caps_malloc_default() does. + // heap_caps_match() tests (heap->caps & caps) == caps - a caps value of 0 is trivially true + // for every heap, not none, so an unconstrained request must be steered to MALLOC_CAP_DEFAULT + // explicitly (same as ESP-IDF's own heap_caps_malloc_default()) or it can land on a heap + // that's unsuitable for the caller's actual use (e.g. not valid as a FreeRTOS task stack). + uint32_t combined_caps = required_caps | desired_caps; + if (combined_caps == 0) { + combined_caps = MALLOC_CAP_DEFAULT; + } + void* ptr; if (policy->alignment > 0) { - ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | desired_caps); + ptr = heap_caps_aligned_alloc(policy->alignment, size, combined_caps); if (ptr == nullptr && desired_caps != 0) { // Desired caps couldn't be satisfied alongside the required ones - retry with // required only, since desired is explicitly optional. ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | MALLOC_CAP_DEFAULT); } } else { - ptr = heap_caps_malloc(size, required_caps | desired_caps); + ptr = heap_caps_malloc(size, combined_caps); if (ptr == nullptr && desired_caps != 0) { ptr = heap_caps_malloc(size, required_caps | MALLOC_CAP_DEFAULT); } @@ -59,9 +64,14 @@ void* memory_realloc_with_policy(void* ptr, size_t size, const struct MemoryPoli uint32_t required_caps = toHeapCaps(policy->required); uint32_t desired_caps = toHeapCaps(policy->desired); + uint32_t combined_caps = required_caps | desired_caps; + if (combined_caps == 0) { + combined_caps = MALLOC_CAP_DEFAULT; + } + // No aligned-realloc counterpart in the heap_caps API - policy->alignment is only honored // on fresh allocations (memory_alloc_with_policy/memory_calloc_with_policy). - void* result = heap_caps_realloc(ptr, size, required_caps | desired_caps); + void* result = heap_caps_realloc(ptr, size, combined_caps); if (result == nullptr && desired_caps != 0) { result = heap_caps_realloc(ptr, size, required_caps | MALLOC_CAP_DEFAULT); } @@ -72,14 +82,19 @@ void* memory_calloc_with_policy(size_t count, size_t size, const struct MemoryPo uint32_t required_caps = toHeapCaps(policy->required); uint32_t desired_caps = toHeapCaps(policy->desired); + uint32_t combined_caps = required_caps | desired_caps; + if (combined_caps == 0) { + combined_caps = MALLOC_CAP_DEFAULT; + } + void* ptr; if (policy->alignment > 0) { - ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps | desired_caps); + ptr = heap_caps_aligned_calloc(policy->alignment, count, size, combined_caps); if (ptr == nullptr && desired_caps != 0) { ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps | MALLOC_CAP_DEFAULT); } } else { - ptr = heap_caps_calloc(count, size, required_caps | desired_caps); + ptr = heap_caps_calloc(count, size, combined_caps); if (ptr == nullptr && desired_caps != 0) { ptr = heap_caps_calloc(count, size, required_caps | MALLOC_CAP_DEFAULT); }