diff --git a/Buildscripts/TactilitySDK/TactilitySDK.cmake b/Buildscripts/TactilitySDK/TactilitySDK.cmake index fe60e8c7..19af8674 100644 --- a/Buildscripts/TactilitySDK/TactilitySDK.cmake +++ b/Buildscripts/TactilitySDK/TactilitySDK.cmake @@ -7,6 +7,18 @@ endfunction() macro(tactility_project project_name) set(TACTILITY_SKIP_SPIFFS 1) + # Tactility's PanicHandler.cpp needs s0 to stay a frame pointer to capture a callstack for + # a RISC-V app's crashes, which GCC does not guarantee without this flag. The firmware sets the + # same flag for its own code, but a crash usually happens in app code, built separately here. + # Gated to RISC-V since Xtensa never reads s0 this way. idf_build_set_property(), not + # add_compile_options(): the app's code compiles as an idf_component_register() component + # (Apps/*/main/CMakeLists.txt), which reads ESP-IDF's own COMPILE_OPTIONS build property rather + # than plain directory-scoped flags. project_elf() below uses the same property for its own + # flags for the same reason. + if(CONFIG_IDF_TARGET_ARCH_RISCV) + idf_build_set_property(COMPILE_OPTIONS "-fno-omit-frame-pointer" APPEND) + endif() + include("${TACTILITY_SDK_PATH}/Libraries/elf_loader/elf_loader.cmake") project_elf($project_name) diff --git a/CMakeLists.txt b/CMakeLists.txt index a2ef51b2..3ead6975 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,10 +58,9 @@ if (DEFINED ENV{ESP_IDF_VERSION}) set(EXCLUDE_COMPONENTS "Simulator") - # Panic handler wrapping is only available on Xtensa architecture - if (CONFIG_IDF_TARGET_ARCH_XTENSA) - idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=esp_panic_handler" APPEND) - endif () + # panic_info_t is architecture-independent (esp_private/panic_internal.h) so this wrap applies + # to every target; PanicHandler.cpp branches internally per architecture. + idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=esp_panic_handler" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=read" APPEND) idf_build_set_property(LINK_OPTIONS "-Wl,--wrap=write" APPEND) @@ -90,6 +89,21 @@ endif () project(Tactility) +if (DEFINED ENV{ESP_IDF_VERSION}) + # PanicHandler.cpp's RISC-V callstack walker requires s0 to stay a frame pointer, which GCC + # does not guarantee without this flag. CONFIG_ESP_SYSTEM_USE_FRAME_POINTER does not add it + # (it only selects which ESP-IDF backtrace-printing function gets compiled), and can't be used + # instead: the bootloader shares this project's sdkconfig with no per-subproject override, and + # enabling it there overflows the bootloader's fixed partition budget. Setting the flag here via + # idf_build_set_property() only affects this project's own configure, not the bootloader's + # separate one, so it's naturally excluded. Gated to RISC-V since Xtensa never reads s0 this way. + # Must run after project(Tactility) above - project() initializes default build specifications + # that would otherwise overwrite this. + if(CONFIG_IDF_TARGET_ARCH_RISCV) + idf_build_set_property(COMPILE_OPTIONS "-fno-omit-frame-pointer" APPEND) + endif() +endif () + # Defined as regular project for PC and component for ESP if (NOT DEFINED ENV{ESP_IDF_VERSION}) add_subdirectory(Tactility) diff --git a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp index 9cfb2771..a478e299 100644 --- a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp +++ b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.cpp @@ -29,8 +29,8 @@ static constexpr uint8_t I2C_ADDRESS = 0x6D; static constexpr uint32_t REPEAT_INITIAL_MS = 400; static constexpr uint32_t REPEAT_RATE_MS = 80; -// I2C event-poll interval - mirrors the old deprecated-HAL's 20ms Timer period. Drives both -// REG_INT_STAT polling (when no IRQ pin) and software key-repeat ticking. +// I2C event-poll interval +// Drives both REG_INT_STAT polling (when no IRQ pin) and software key-repeat ticking. static constexpr uint32_t POLL_INTERVAL_MS = 20; // Upper bound on events consumed per drain_events() call. Since the loop re-reads REG_EVENT_NUM @@ -193,6 +193,7 @@ static uint32_t now_ms() { // the event - and software key-repeat replays this same struct, so a held chord keeps its modifiers. struct Tab5KeyEvent { uint32_t key; + bool pressed; bool ctrl; bool alt; uint8_t hid_keycode; @@ -216,11 +217,20 @@ struct Tab5KeyboardInternal { gpio_num_t irq_pin; // Poll throttling (real-time based, since read_key() is called at whatever rate LVGL's indev - // timer and its own drain-loop - via continue_reading - happen to run at, unlike the old - // deprecated-HAL's fixed 20ms Timer) + // timer and its own drain-loop, via continue_reading, happen to run at). uint32_t last_poll_ms; - // Software key-repeat state (tracked by position to survive modifier changes) + // Original press event for every currently-held key, indexed by matrix position (row*14+col), + // so a release can recover the exact event its press queued (modifiers captured at press + // time) even when another key was pressed and released in between. held_event[i].key can + // legitimately be 0 (e.g. F1-F12, see drain_events()), so held[i] tracks validity separately + // rather than using a sentinel key value. + Tab5KeyEvent held_event[70]; + bool held[70]; + + // Software key-repeat state: tracks only the most recently pressed key, independent of the + // per-position storage above (repeats stop as soon as a different key is pressed, matching + // typical keyboard behavior, and don't need to survive that key's release). Tab5KeyEvent repeat_event; uint8_t repeat_row; uint8_t repeat_col; @@ -258,15 +268,17 @@ bool tab5_keyboard_is_attached(Device* device) { } // --------------------------------------------------------------------------- -// LED helpers - LED0 = Sym indicator (green), LED1 = Aa indicator (red) +// LED helpers - LED0 = Sym indicator (blue), LED1 = Aa indicator (red) // RGB register layout: [B, G, R] per LED, stride 4 (byte 3 reserved) // --------------------------------------------------------------------------- -static void update_leds(Device* device, const Tab5KeyboardInternal* internal) { +static constexpr uint8_t LED_SYM_ON_BLUE = 0xC0; +static constexpr uint8_t LED_AA_ON_RED = 0x90; + +static void update_leds(Device* device, Tab5KeyboardInternal* internal) { auto* parent = device_get_parent(device); - // [LED0: B,G,R, reserved, LED1: B,G,R] uint8_t buf[7] = { - 0x00, internal->sym_active ? uint8_t(0xA0) : uint8_t(0x00), 0x00, 0x00, - 0x00, 0x00, internal->aa_sticky ? uint8_t(0xA0) : uint8_t(0x00), + internal->sym_active ? LED_SYM_ON_BLUE : uint8_t(0x00), 0x00, 0x00, 0x00, + 0x00, 0x00, internal->aa_sticky ? LED_AA_ON_RED : uint8_t(0x00), }; i2c_controller_write_register(parent, I2C_ADDRESS, REG_RGB_BASE, buf, 7, pdMS_TO_TICKS(50)); } @@ -389,9 +401,14 @@ static void drain_events(Device* device, Tab5KeyboardInternal* internal) { // no business reaching into, so ESC is now just queued as a normal key // like everything else (LVGL/app code already handles ESC via focus/group // navigation the same way a dedicated ESC key on any other keyboard would). - const Tab5KeyEvent event = { lv_key, internal->ctrl_held, internal->alt_held, + const Tab5KeyEvent event = { lv_key, true, internal->ctrl_held, internal->alt_held, m.keycode, modifier }; xQueueSend(internal->queue, &event, 0); + // Remember this key's press event by position so its release (whenever it + // comes, regardless of what else is pressed in between) reports the same value. + const uint8_t idx = row * 14U + col; + internal->held_event[idx] = event; + internal->held[idx] = true; // Arm software repeat tracking by row/col to survive modifier changes const uint32_t now = now_ms(); internal->repeat_event = event; @@ -405,9 +422,31 @@ static void drain_events(Device* device, Tab5KeyboardInternal* internal) { internal->aa_held = false; update_leds(device, internal); } - } else if (row == internal->repeat_row && col == internal->repeat_col) { - // Match release by position, not translated value — survives sticky Aa clear - internal->repeat_event.key = 0; + } else { + // Always queue the release: callers key their own "is this held" state off + // (key, pressed) pairs, and a dropped release leaves that key stuck down forever. + // + // Reuse the matching press's key/modifier (via held_event, tracked by matrix + // position) rather than recomputing from current modifier state: aa_active + // above is read fresh, but sticky Aa is consumed right after the press fires + // (above), so recomputing here would give the release a different key value + // than its press whenever Aa was sticky (e.g. shifted vs unshifted). Only + // recompute as a fallback for the case where no press was ever recorded for + // this position (e.g. driver just started while the key was already held). + const uint8_t idx = row * 14U + col; + Tab5KeyEvent event; + if (internal->held[idx]) { + event = internal->held_event[idx]; + internal->held[idx] = false; + } else { + event = { lv_key, false, internal->ctrl_held, internal->alt_held, m.keycode, modifier }; + } + event.pressed = false; + xQueueSend(internal->queue, &event, 0); + + if (row == internal->repeat_row && col == internal->repeat_col) { + internal->repeat_event.key = 0; + } } } } @@ -429,7 +468,7 @@ void tab5_keyboard_reinit(Device* device) { write_reg_fast(device, REG_EVENT_NUM, 0x00); // flush event queue write_reg_fast(device, REG_INT_STAT, 0x00); // clear pending INT write_reg_fast(device, REG_RGB_MODE, 0x01); // Custom RGB mode (manual LED control) - write_reg_fast(device, REG_BRIGHTNESS, 50); // 50% brightness + write_reg_fast(device, REG_BRIGHTNESS, 30); // 30% brightness update_leds(device, internal); // restore current LED state if (internal->irq_configured) { @@ -437,9 +476,39 @@ void tab5_keyboard_reinit(Device* device) { } } +void tab5_keyboard_reset_state(Device* device) { + auto* internal = static_cast(device_get_driver_data(device)); + + for (uint8_t idx = 0; idx < 70U; idx++) { + if (internal->held[idx]) { + Tab5KeyEvent event = internal->held_event[idx]; + event.pressed = false; + // This runs under lvgl_lock() (apply_state()'s caller), and the consumer that drains + // this queue is LVGL's own indev read callback - blocking here for queue space could + // deadlock against that consumer needing the same lock. Best-effort send only, same as + // drain_events()'s hot path; internal->held[idx] is still cleared on a dropped send so + // this reset doesn't get stuck retrying a release that already lost its only chance to + // be delivered before the state it referred to (an unplugged keyboard) is gone anyway. + xQueueSend(internal->queue, &event, 0); + internal->held[idx] = false; + } + } + + internal->repeat_event.key = 0; + internal->repeat_row = 0xFF; + internal->repeat_col = 0xFF; + + internal->sym_active = false; + internal->aa_sticky = false; + internal->aa_held = false; + internal->aa_tapped = false; + internal->ctrl_held = false; + internal->alt_held = false; + update_leds(device, internal); +} + // --------------------------------------------------------------------------- -// poll_if_due - the closest equivalent to the old deprecated-HAL's 20ms-Timer-driven -// processKeyboard(): drains new key events (IRQ-gated or polled) and ticks software key-repeat. +// poll_if_due - drains new key events (IRQ-gated or polled) and ticks software key-repeat. // Called from read_key(), throttled to real elapsed time rather than call count, since read_key() // can be called back-to-back multiple times per LVGL indev timer tick while draining an // already-queued burst (continue_reading). Hot-plug attach detection lives outside the driver - @@ -566,7 +635,7 @@ static error_t tab5_keyboard_read_key(Device* device, KeyboardKeyData* data) { Tab5KeyEvent event = {}; if (xQueueReceive(internal->queue, &event, 0) == pdTRUE) { data->key = event.key; - data->pressed = true; + data->pressed = event.pressed; data->continue_reading = uxQueueMessagesWaiting(internal->queue) > 0; data->ctrl = event.ctrl; data->alt = event.alt; diff --git a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.h b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.h index 34f79553..77fae5df 100644 --- a/Devices/m5stack-tab5/Source/devices/tab5_keyboard.h +++ b/Devices/m5stack-tab5/Source/devices/tab5_keyboard.h @@ -37,6 +37,12 @@ bool tab5_keyboard_is_attached(struct Device* device); // tab5_keyboard_attach_detect.cpp). void tab5_keyboard_reinit(struct Device* device); +// Emits a release for every currently-held key (a hardware release can't arrive once the keyboard +// is unplugged), then clears held-key/software-repeat/modifier state. Callers must call this on +// confirmed detach (see tab5_keyboard_attach_detect.cpp) so a key held across an unplug doesn't +// leave consumers with a stuck key or spurious repeats/modifiers after reattach. +void tab5_keyboard_reset_state(struct Device* device); + #ifdef __cplusplus } #endif diff --git a/Devices/m5stack-tab5/Source/devices/tab5_keyboard_attach_detect.cpp b/Devices/m5stack-tab5/Source/devices/tab5_keyboard_attach_detect.cpp index 882e6e4a..7c8555b6 100644 --- a/Devices/m5stack-tab5/Source/devices/tab5_keyboard_attach_detect.cpp +++ b/Devices/m5stack-tab5/Source/devices/tab5_keyboard_attach_detect.cpp @@ -63,6 +63,11 @@ static bool apply_state(Device* keyboard_device, bool attached) { lv_display_set_rotation(display, LV_DISPLAY_ROTATION_90); } } else { + // A key held at the moment of unplug can never get its hardware release - reset tracked + // state so it doesn't read as stuck or feed stale modifiers into whatever's pressed next + // after reattach. + tab5_keyboard_reset_state(keyboard_device); + // Only restore if rotation is still what we set it to - if the user manually changed it // since attaching, respect their choice instead. if (rotation_override_active && lv_display_get_rotation(display) == LV_DISPLAY_ROTATION_90) { @@ -84,13 +89,24 @@ static void attach_detect_callback(TimerHandle_t /*timer*/) { // LVGL restarting is a distinct event from the keyboard physically attaching/detaching: the // accessory may never have moved, but whatever apply_state() last set (rotation) may have // been reset in the meantime by the restart. Forcing was_attached false makes the block below - // see a fresh "attached" transition (still going through the normal 2-check debounce) so + // see a fresh "attached" transition, still going through the normal two-check debounce, so // apply_state() re-announces the current state instead of staying silent forever, waiting for // an edge that will never come because the keyboard was never actually unplugged. // lvgl_is_running() is safe to call unlocked (unlike lv_display_get_default(), resolved inside // the lock in apply_state() instead). const bool lvgl_ready = lvgl_is_running(); if (lvgl_ready && !was_lvgl_ready) { + // If the keyboard is (and, per was_attached, already was) physically detached, forcing + // was_attached false below means the detach transition below will never fire again for + // this unplug - it already happened before this restart. Reset software state here + // instead, since apply_state()'s own detach path may never have run: it could have bailed + // out early (LVGL lock busy, or display not ready yet) before reaching its + // tab5_keyboard_reset_state() call, and now never will, because that transition is about + // to be erased. This reset doesn't touch LVGL/display state, so it doesn't need + // apply_state()'s lock/display gating. + if (was_attached && !tab5_keyboard_is_attached(keyboard_device)) { + tab5_keyboard_reset_state(keyboard_device); + } was_attached = false; pending_attach_confirm_count = 0; } @@ -98,7 +114,7 @@ static void attach_detect_callback(TimerHandle_t /*timer*/) { const bool attached = tab5_keyboard_is_attached(keyboard_device); if (attached != was_attached) { - // Require the new state to be confirmed on a second consecutive check before acting - a + // Require the new state to be confirmed on a second consecutive check before acting: a // single probe on a floating/half-connected bus (e.g. mid-unplug) can false-positive. if (attached != pending_attach_state || pending_attach_confirm_count == 0) { pending_attach_state = attached; @@ -108,7 +124,7 @@ static void attach_detect_callback(TimerHandle_t /*timer*/) { if (apply_state(keyboard_device, attached)) { was_attached = attached; } - // else: not handled yet (e.g. LVGL lock busy) - retry on the next confirmed check + // else: not handled yet (e.g. LVGL lock busy); retry on the next confirmed check } } else { pending_attach_confirm_count = 0; diff --git a/Devices/m5stack-tab5/m5stack,tab5.dts b/Devices/m5stack-tab5/m5stack,tab5.dts index 0e908427..644d2fb1 100644 --- a/Devices/m5stack-tab5/m5stack,tab5.dts +++ b/Devices/m5stack-tab5/m5stack,tab5.dts @@ -174,7 +174,7 @@ i2c_keyboard: i2c2 { compatible = "espressif,esp32-i2c-master"; port = ; - clock-frequency = <100000>; + clock-frequency = <400000>; clock-source = ; pin-sda = <&gpio0 0 GPIO_FLAG_PULL_UP>; pin-scl = <&gpio0 1 GPIO_FLAG_PULL_UP>; diff --git a/Modules/c-symbols-module/source/module.cpp b/Modules/c-symbols-module/source/module.cpp index 4b6ee1f4..e7b26ea0 100644 --- a/Modules/c-symbols-module/source/module.cpp +++ b/Modules/c-symbols-module/source/module.cpp @@ -148,6 +148,11 @@ static const ModuleSymbol SYMBOLS[] = { DEFINE_MODULE_SYMBOL(memcmp), DEFINE_MODULE_SYMBOL(memmove), // ctype.h +#ifdef ESP_PLATFORM + // _ctype_ is newlib's internal lookup table backing isalnum() etc.; + // glibc/macOS libc don't export a symbol by this name. + DEFINE_MODULE_SYMBOL(_ctype_), +#endif DEFINE_MODULE_SYMBOL(isalnum), DEFINE_MODULE_SYMBOL(isalpha), DEFINE_MODULE_SYMBOL(iscntrl), diff --git a/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_msc.cpp b/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_msc.cpp index f90b3696..e9e7d5ce 100644 --- a/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_msc.cpp +++ b/Platforms/platform-esp32/source/drivers/usb/esp32_usb_device_msc.cpp @@ -187,11 +187,15 @@ static error_t msc_device_stop(struct Device* device) { return ERROR_NONE; } - tinyusb_msc_storage_deinit(); - + // Disconnect and fully stop the TinyUSB device task before freeing storage state: the task + // keeps servicing SCSI commands (Windows polls TEST UNIT READY continuously, even on an + // ejected-but-still-enumerated device) until tinyusb_driver_uninstall() actually stops it, so + // freeing first left tud_msc_*_cb() callbacks dereferencing an already-freed storage handle. auto* controller = device_get_parent(device); usb_device_controller_release(controller, USB_DEVICE_CLASS_MSC); + tinyusb_msc_storage_deinit(); + ctx->storage_active = false; ctx->mount_changed_cb = nullptr; ctx->mount_changed_context = nullptr; diff --git a/Tactility/Source/PanicHandler.cpp b/Tactility/Source/PanicHandler.cpp index 375bc998..914cc891 100644 --- a/Tactility/Source/PanicHandler.cpp +++ b/Tactility/Source/PanicHandler.cpp @@ -2,17 +2,26 @@ #include #endif -#if defined(ESP_PLATFORM) && defined(CONFIG_IDF_TARGET_ARCH_XTENSA) +#if defined(ESP_PLATFORM) #include #include +#include +#include + +#if defined(CONFIG_IDF_TARGET_ARCH_XTENSA) #include #include #include -#include -#include #include +#elif defined(CONFIG_IDF_TARGET_ARCH_RISCV) +#include + +// The walker below reads s0 as a frame pointer. GCC only guarantees this with +// -fno-omit-frame-pointer (set project-wide, excluding the bootloader, in the top-level +// CMakeLists.txt). +#endif #include @@ -28,13 +37,6 @@ void __real_esp_panic_handler(void* info); void __wrap_esp_panic_handler(void* info) { - esp_backtrace_frame_t frame = { - .pc = 0, - .sp = 0, - .next_pc = 0, - .exc_frame = nullptr - }; - const auto* panic_info = static_cast(info); switch (panic_info->exception) { @@ -51,6 +53,7 @@ void __wrap_esp_panic_handler(void* info) { } crashData.callstackLength = 0; + crashData.callstackCorrupted = false; crashData.faultAddress = reinterpret_cast(panic_info->addr); // g_panic_abort_details carries the actual assert()/abort() message when present; panic_info->reason @@ -64,6 +67,16 @@ void __wrap_esp_panic_handler(void* info) { crashData.reason[sizeof(crashData.reason) - 1] = '\0'; } +#if defined(CONFIG_IDF_TARGET_ARCH_XTENSA) + // Xtensa's register-windowing hardware lets ESP-IDF walk the stack via + // esp_backtrace_get_start()/esp_backtrace_get_next_frame(). + esp_backtrace_frame_t frame = { + .pc = 0, + .sp = 0, + .next_pc = 0, + .exc_frame = nullptr + }; + esp_backtrace_get_start(&frame.pc, &frame.sp, &frame.next_pc); crashData.callstack[0].pc = frame.pc; #if CRASH_DATA_INCLUDES_SP @@ -105,6 +118,70 @@ void __wrap_esp_panic_handler(void* info) { break; } } +#elif defined(CONFIG_IDF_TARGET_ARCH_RISCV) + // RISC-V has no register-windowing hardware, so the stack has to be walked by hand via the + // frame-pointer (s0) chain. Algorithm ported from esp-rs/esp-hal's esp-backtrace crate + // (Apache-2.0): https://github.com/esp-rs/esp-hal/blob/main/esp-backtrace/src/riscv.rs + // + // s0 for a frame points just past that frame's saved {ra, s0} pair: the caller's return + // address is at fp-4, the caller's own frame pointer at fp-8. + const auto* exc_frame = static_cast(panic_info->frame); + + // mepc is the exact faulting instruction, not a return address, so it's used directly rather + // than read from the stack like the rest of the walk. + crashData.callstack[0].pc = exc_frame->mepc; +#if CRASH_DATA_INCLUDES_SP + crashData.callstack[0].sp = exc_frame->sp; +#endif + crashData.callstackLength++; + + uint32_t fp = exc_frame->s0; + + crashData.callstackCorrupted = !(esp_stack_ptr_is_sane(exc_frame->sp) && esp_ptr_executable(reinterpret_cast(exc_frame->mepc))); + + while ( + !crashData.callstackCorrupted + && crashData.callstackLength < CRASH_DATA_CALLSTACK_LIMIT + ) { + // esp_stack_ptr_is_sane() also requires 16-byte alignment, a property of sp at call + // boundaries but not of a frame pointer (fp only needs word alignment). esp_ptr_in_dram() + // is the same range check without that assumption. + // + // Checked against fp-8, not just fp: the record about to be read is [fp-8, fp), and an fp + // near the very start of the DRAM range can itself pass esp_ptr_in_dram() while fp-8 + // underflows below it, so validate the whole record before dereferencing any of it. + // + // Every task's root frame is vPortTaskWrapper() (FreeRTOS-Kernel/portable/riscv/port.c), + // which marks itself `.cfi_undefined ra`: no valid frame exists below it, so an invalid fp + // here is the expected end of the walk once at least one real frame has been captured, not + // corruption. An invalid fp on the very first iteration is a real problem. + if (!esp_ptr_in_dram(reinterpret_cast(fp - 8)) || !esp_ptr_in_dram(reinterpret_cast(fp)) || (fp & 0x3) != 0) { + crashData.callstackCorrupted = (crashData.callstackLength <= 1); + break; + } + + uint32_t ra = *reinterpret_cast(fp - 4); + uint32_t prev_fp = *reinterpret_cast(fp - 8); + + // A zero return address marks the outermost frame (startup code zero-initialises it). + if (ra == 0) { + break; + } + + if (!esp_ptr_executable(reinterpret_cast(ra))) { + crashData.callstackCorrupted = (crashData.callstackLength <= 1); + break; + } + + crashData.callstack[crashData.callstackLength].pc = ra; +#if CRASH_DATA_INCLUDES_SP + crashData.callstack[crashData.callstackLength].sp = fp; +#endif + crashData.callstackLength++; + + fp = prev_fp; + } +#endif // CONFIG_IDF_TARGET_ARCH_XTENSA / CONFIG_IDF_TARGET_ARCH_RISCV // TODO: Handle corrupted logic @@ -115,15 +192,4 @@ void __wrap_esp_panic_handler(void* info) { const CrashData& getRtcCrashData() { return crashData; } -#elif defined(ESP_PLATFORM) - -// Stub implementation for RISC-V and other architectures -// TODO: Implement crash data collection for RISC-V using frame pointer or EH frame - -#include - -static CrashData emptyCrashData = {}; - -const CrashData& getRtcCrashData() { return emptyCrashData; } - -#endif \ No newline at end of file +#endif diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index cf9a5c79..6c51c5a2 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -415,6 +416,18 @@ static void applySavedTouchCalibration() { #endif // CONFIG_TT_TOUCH_CALIBRATION_SUPPORTED static void onLvglStarted() { + // lv_display_create() (inside lvgl_devices_attach(), which already ran by this point) always + // resets rotation to LV_DISPLAY_ROTATION_0. The only other code that ever applies a saved + // orientation is the display settings app's dropdown change handler, so without this, every + // LVGL restart (not just first boot) silently drops back to unrotated. Must run before + // window_manager_start() below builds the window tree against the display's current size. + lvgl_lock(); + if (auto* display = lv_display_get_default(); display != nullptr) { + auto displaySettings = settings::display::loadOrGetDefault(); + lv_display_set_rotation(display, settings::display::toLvglDisplayRotation(displaySettings.orientation)); + } + lvgl_unlock(); + window_manager_configure(windowManagerScreenInit); check(module_ensure_started(&lvgl_window_manager_module) == ERROR_NONE); @@ -461,7 +474,13 @@ static void onLvglStopped() { check(service::removeService(service::statusbar::manifest.id)); if (softwareKeyboard.object != nullptr) { + // lv_obj_delete() walks/mutates the object graph (event lists, group membership, + // parent/child links). Without the LVGL lock this can race the LVGL port task's own + // concurrent traversal (input dispatch, timers, animations), producing an intermittent + // double-free/use-after-free inside lv_obj_destructor/lv_event_mark_deleted. + lvgl_lock(); lvgl_software_keyboard_destruct(&softwareKeyboard); + lvgl_unlock(); } module_stop(&lvgl_window_manager_module); diff --git a/Tactility/Source/app/applist/AppList.cpp b/Tactility/Source/app/applist/AppList.cpp index d2495132..c7b0effd 100644 --- a/Tactility/Source/app/applist/AppList.cpp +++ b/Tactility/Source/app/applist/AppList.cpp @@ -53,17 +53,18 @@ void collectManifest(const ::AppManifest* manifest, void* context) { void createWidgets(lv_obj_t* parent, void* userData) { auto* ctx = static_cast(userData); + // Flex column + flex_grow so LVGL recomputes the toolbar/list split on every layout pass, + // rather than a fixed height computed once from lv_obj_get_content_height(parent) that would + // go stale after a later display resolution/rotation change. + 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, "Apps"); 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); lv_obj_set_width(list, LV_PCT(100)); - lv_obj_align_to(list, toolbar, LV_ALIGN_OUT_BOTTOM_MID, 0, 0); - - auto toolbar_height = lv_obj_get_height(toolbar); - auto parent_content_height = lv_obj_get_content_height(parent); - lv_obj_set_height(list, parent_content_height - toolbar_height); + lv_obj_set_flex_grow(list, 1); std::vector manifests; app_manager_for_each_manifest(collectManifest, &manifests); diff --git a/Tactility/Source/app/appsettings/AppSettings.cpp b/Tactility/Source/app/appsettings/AppSettings.cpp index e767bbdf..acede4e3 100644 --- a/Tactility/Source/app/appsettings/AppSettings.cpp +++ b/Tactility/Source/app/appsettings/AppSettings.cpp @@ -55,18 +55,18 @@ void collectManifest(const ::AppManifest* manifest, void* context) { void createWidgets(lv_obj_t* parent, void* userData) { auto* ctx = static_cast(userData); + // Flex column + flex_grow; see AppList.cpp's createWidgets() for why a fixed height computed + // once from lv_obj_get_content_height(parent) goes stale. + 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, "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, ctx); - lv_obj_align(toolbar, LV_ALIGN_TOP_MID, 0, 0); lv_obj_t* list = lv_list_create(parent); lv_obj_set_width(list, LV_PCT(100)); - lv_obj_align_to(list, toolbar, LV_ALIGN_OUT_BOTTOM_MID, 0, 0); - - auto toolbar_height = lv_obj_get_height(toolbar); - auto parent_content_height = lv_obj_get_content_height(parent); - lv_obj_set_height(list, parent_content_height - toolbar_height); + lv_obj_set_flex_grow(list, 1); std::vector manifests; app_manager_for_each_manifest(collectManifest, &manifests); @@ -83,9 +83,20 @@ void createWidgets(lv_obj_t* parent, void* userData) { } if (app_count == 0) { - auto* no_apps_label = lv_label_create(parent); + // lv_obj_align() is ignored for children of a flex-managed parent, so the empty-state + // label needs its own flex-growing wrapper to center within; the (empty) list is hidden + // rather than deleted so the wrapper can just take its place in the flex flow. + lv_obj_add_flag(list, LV_OBJ_FLAG_HIDDEN); + lv_obj_set_flex_grow(list, 0); + + auto* empty_wrapper = lv_obj_create(parent); + lv_obj_set_width(empty_wrapper, LV_PCT(100)); + lv_obj_set_flex_grow(empty_wrapper, 1); + lv_obj_set_flex_align(empty_wrapper, LV_FLEX_ALIGN_CENTER, LV_FLEX_ALIGN_CENTER, LV_FLEX_ALIGN_CENTER); + lv_obj_set_style_border_width(empty_wrapper, 0, LV_STATE_DEFAULT); + + auto* no_apps_label = lv_label_create(empty_wrapper); lv_label_set_text(no_apps_label, "No apps installed"); - lv_obj_align(no_apps_label, LV_ALIGN_CENTER, 0, 0); } } diff --git a/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp b/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp index 4172af50..e20ec93a 100644 --- a/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp +++ b/Tactility/Source/app/crashdiagnostics/CrashDiagnostics.cpp @@ -31,6 +31,7 @@ #include +#include #include #include #include @@ -178,12 +179,11 @@ void createWidgets(lv_obj_t* parent, void* userData) { int32_t available_height = parent_height - top_label_height - bottom_label_height; int32_t available_width = lv_display_get_horizontal_resolution(display); int32_t smallest_size = std::min(available_height, available_width); - int32_t pixel_size; - if (qrcode.size * 2 <= smallest_size) { - pixel_size = 2; - } else if (qrcode.size <= smallest_size) { - pixel_size = 1; - } else { + // Target ~60% of the available space so the code scales with screen size but keeps a margin + // from the labels/screen edges. + int32_t target_size = smallest_size * 6 / 10; + int32_t pixel_size = std::max(1, target_size / qrcode.size); + if (pixel_size * qrcode.size > smallest_size) { LOG_E(TAG, "QR code won't fit screen"); ctx->hasFatalError = true; return; diff --git a/TactilityFreeRtos/Include/Tactility/Timer.h b/TactilityFreeRtos/Include/Tactility/Timer.h index 4a748a63..fa7ea68e 100644 --- a/TactilityFreeRtos/Include/Tactility/Timer.h +++ b/TactilityFreeRtos/Include/Tactility/Timer.h @@ -4,6 +4,13 @@ #include "Thread.h" #include "freertoscompat/Timers.h" +#ifdef ESP_PLATFORM +#include +#else +#include +#endif + +#include #include #include @@ -36,6 +43,15 @@ private: Callback callback; std::unique_ptr, TimerHandleDeleter> handle; + // Set for the duration of a callback invocation. xTimerStop()/xTimerDelete() only prevent + // *future* dispatches: if a callback was already dispatched by the timer daemon task, they + // return immediately without waiting for it to finish. Callers that stop a timer and then + // immediately destroy state the callback reads/writes (e.g. a Service destructing itself + // right after stopping its own update timer) can otherwise race an in-flight callback against + // that destruction. stop() below spins on this flag so it only returns once no callback is + // executing and none can start afterward. + std::atomic callbackRunning {false}; + static TimerHandle_t createTimer(Type type, TickType_t ticks, void* timerId, TimerCallbackFunction_t callback) { assert(timerId != nullptr); assert(callback != nullptr); @@ -47,10 +63,20 @@ private: static void onCallback(TimerHandle_t hTimer) { auto* timer = static_cast(pvTimerGetTimerID(hTimer)); if (timer != nullptr) { + timer->callbackRunning.store(true, std::memory_order_release); timer->callback(); + timer->callbackRunning.store(false, std::memory_order_release); } } + // Signals a SemaphoreHandle_t (passed as context) from the timer daemon task. Used by stop() + // as a barrier: FreeRTOS timer commands are processed FIFO, so queuing this via + // xTimerPendFunctionCall right after xTimerStop() guarantees it only runs once the daemon has + // drained everything queued ahead of it - including an expiry command that raced the stop. + static void onStopBarrier(void* context, uint32_t /*arg*/) { + xSemaphoreGive(static_cast(context)); + } + public: /** @@ -78,13 +104,35 @@ public: return xTimerStart(handle.get(), kernel::FREERTOS_MAX_TICKS) == pdPASS; } - /** Stop the timer - * @warning If the timer was just triggered, the callback might still be going on after stop() was called + /** + * Stop the timer. Unlike a bare xTimerStop(), this blocks until any + * callback invocation already queued or dispatched by the timer daemon + * task at the time of the call has finished running, so it is safe to + * destroy state the callback reads or writes as soon as this returns. + * @warning Do not call this from within the timer's own callback - it + * would deadlock waiting on itself. * @return success result */ bool stop() const { assert(xPortInIsrContext() == pdFALSE); - return xTimerStop(handle.get(), kernel::FREERTOS_MAX_TICKS) == pdPASS; + bool result = xTimerStop(handle.get(), kernel::FREERTOS_MAX_TICKS) == pdPASS; + if (result) { + // xTimerStop() only queues tmrCOMMAND_STOP - the daemon may not have processed it yet, + // and an expiry command already ahead of it in the queue can still dispatch a callback + // after this returns. Queuing a pend-function-call barrier right after the stop command + // guarantees (FIFO command processing) that it only runs once everything queued ahead + // of it - including such an expiry - has been handled. + SemaphoreHandle_t barrier = xSemaphoreCreateBinary(); + assert(barrier != nullptr); + if (setPendingCallback(onStopBarrier, barrier, 0, kernel::FREERTOS_MAX_TICKS)) { + xSemaphoreTake(barrier, kernel::FREERTOS_MAX_TICKS); + } + vSemaphoreDelete(barrier); + } + while (callbackRunning.load(std::memory_order_acquire)) { + vTaskDelay(1); + } + return result; } /**