Fixes and improvements (#620)

- Standardized keyboard input using Unicode-based key codes across supported devices and the simulator. Keyboards don't emit `LV_KEY_*` anymore.
- Refactored lilygo encoder driver into a reusable GPIO rotary encoder driver (see `Drivers/gpio-encoder-module/`). Added more features to the config file.
- Improved LVGL keyboard device management, including duplicate prevention and reliable reconnects.
- LVGL file mutex now registers with lvgl start/stop
- Improved LVGL startup/shutdown stability and memory allocation reliability.
- Increased simulator LVGL memory capacity and improved USB device-class handling.

TactilityKernel/include/tactility/drivers/keyboard.h

@@ -11,11 +11,43 @@ extern "C" {
 #include <tactility/device.h>
 #include <tactility/error.h>
 
+/**
+ * @brief Named Unicode codepoints for KeyboardKeyData::key.
+ *
+ * Keys that produce an ordinary character use that character's own codepoint directly (e.g. 'a',
+ * ' ') and don't need a name here. These are the standard Unicode symbol codepoints used instead
+ * of LVGL's LV_KEY_* sentinels for keys with no character of their own (arrows, Home/End,
+ * Tab/Shift+Tab-as-focus-navigation), plus names for the handful of C0 control keys every driver
+ * needs (Enter/Escape/Backspace/Delete/Tab). Modules/lvgl-module/source/devices/keyboard.cpp
+ * translates these back into LV_KEY_* internally for LVGL.
+ */
+typedef enum {
+    CODEPOINT_ENTER       = '\r',
+    CODEPOINT_ESCAPE      = '\x1B',
+    CODEPOINT_BACKSPACE   = '\b',
+    CODEPOINT_DELETE      = '\x7F',
+    CODEPOINT_TAB         = '\t',
+    CODEPOINT_ARROW_LEFT  = 0x2190,
+    CODEPOINT_ARROW_UP    = 0x2191,
+    CODEPOINT_ARROW_RIGHT = 0x2192,
+    CODEPOINT_ARROW_DOWN  = 0x2193,
+    CODEPOINT_HOME        = 0x21F1,
+    CODEPOINT_END         = 0x21F2,
+} CodePoint;
+
 /**
  * @brief A single key event read from a keyboard device.
  */
 struct KeyboardKeyData {
-    /** @brief The key code. Driver-defined (e.g. ASCII/UTF-8 codepoint, scan code, or LVGL key code). */
+    /**
+     * @brief The key. Always a Unicode codepoint - never a raw scan code.
+     *
+     * For a key that produces a character, this is that character's codepoint (e.g. 'a', ' '). For
+     * a key with no character representation, or one of the handful of C0 control keys every
+     * driver needs, this is one of the CodePoint enum values above - never an LVGL LV_KEY_*
+     * constant directly. See ctrl's doc comment below for the CodePoint values that numerically
+     * collide with real C0 control codes.
+     */
     uint32_t key;
     /** @brief True if the key was pressed, false if released. */
     bool pressed;
@@ -29,9 +61,9 @@ struct KeyboardKeyData {
      *
      * Reported separately rather than folded into `key` because the two encodings collide: the C0
      * control codes a terminal expects for Ctrl chords (Ctrl+C is 0x03, Ctrl+K is 0x0B, ...) overlap
-     * the LVGL key constants drivers emit in the same field (LV_KEY_END is 3, LV_KEY_PREV is 11,
-     * LV_KEY_UP is 17, ...), so a single uint32_t cannot express both. Consumers that want control
-     * codes derive them here, e.g.
+     * the four CodePoint values that remain in true C0 range (CODEPOINT_ENTER is Ctrl+M/13,
+     * CODEPOINT_BACKSPACE is Ctrl+H/8, CODEPOINT_TAB is Ctrl+I/9, CODEPOINT_ESCAPE is Ctrl+[/27), so
+     * a single uint32_t cannot express both. Consumers that want control codes derive them here, e.g.
      * `((key >= 'a' && key <= 'z') || (key >= 'A' && key <= 'Z')) ? (key & 0x1F) : key`
      * when ctrl is set.
      *
@@ -48,8 +80,8 @@ struct KeyboardKeyData {
      * or 0 if this driver doesn't compute one (most don't - `key` is the only field most consumers
      * need). Populated by drivers whose hardware layout maps cleanly onto HID usage codes, so
      * consumers that want to mirror physical key presses as real HID reports (e.g. USB HID output)
-     * don't have to reverse-engineer one out of `key`'s LVGL/ASCII encoding - which is lossy for
-     * keys with no ASCII/LVGL representation at all, e.g. F1-F12.
+     * don't have to reverse-engineer one out of `key`'s codepoint encoding - which is lossy for
+     * keys with no Unicode/LVGL representation at all, e.g. F1-F12.
      */
     uint8_t hid_keycode;
     /**

TactilityKernel/include/tactility/filesystem/file_mutex.h

@@ -19,13 +19,25 @@ struct FileMutex {
     void (*unlock)();
 };
 
+typedef uint32_t FileMutexId;
+
+#define FILE_MUTEX_ID_INVALID ((FileMutexId)0)
+
 /**
  * @brief Registers a mutex for a mount path (e.g. "/sdcard") and its descendants.
  * @param[in] mutex callbacks to associate with the path; a copy is stored
  * @param[in] path mount path this mutex serializes access to
- * @note No-op if a mutex is already registered for this exact path.
+ * @return the id of the new entry, or the id of the existing entry if path is already registered
+ * @note If a mutex is already registered for this exact path, no new entry is created and the
+ *     existing entry's id is returned; the existing callbacks are left unchanged.
  */
-void file_mutex_register(const struct FileMutex* mutex, const char* path);
+FileMutexId file_mutex_add(const struct FileMutex* mutex, const char* path);
+
+/**
+ * @brief Removes a previously added mutex registration.
+ * @param[in] id id returned by file_mutex_add(); a stale or unknown id is a no-op
+ */
+void file_mutex_remove(FileMutexId id);
 
 /**
  * @brief Looks up the mutex registered for path or one of its ancestor mount paths.

TactilityKernel/source/filesystem/file_mutex.cpp

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 #include <tactility/filesystem/file_mutex.h>
+#include <tactility/concurrent/mutex.h>
 
+#include <algorithm>
 #include <cstring>
 #include <string>
 #include <vector>
@@ -12,41 +14,89 @@ static const FileMutex no_mutex = {
 };
 
 struct FileMutexEntry {
+    FileMutexId id;
     std::string path;
     FileMutex mutex;
 };
 
-static std::vector<FileMutexEntry> mutex_entries;
+// Guards mutex_entries against concurrent add/get/remove; unrelated to whether a FileMutex's own
+// lock/unlock is currently held (file_mutex_get() hands out a copy that stays valid regardless of
+// later registry changes - see file_mutex_remove()).
+struct FileMutexLedger {
+    std::vector<FileMutexEntry> entries;
+    FileMutexId next_id = 1;
+    Mutex mutex {};
+
+    FileMutexLedger() { mutex_construct(&mutex); }
+    ~FileMutexLedger() { mutex_destruct(&mutex); }
+
+    void lock() { mutex_lock(&mutex); }
+    void unlock() { mutex_unlock(&mutex); }
+};
+
+static FileMutexLedger& get_ledger() {
+    static FileMutexLedger ledger;
+    return ledger;
+}
 
 extern "C" {
 
-void file_mutex_register(const FileMutex* mutex, const char* path) {
-    // Skip if entry for path exists
-    for (auto& entry : mutex_entries) {
+FileMutexId file_mutex_add(const FileMutex* mutex, const char* path) {
+    auto& ledger = get_ledger();
+    ledger.lock();
+
+    for (auto& entry : ledger.entries) {
         if (entry.path == path) {
-            return;
+            FileMutexId existing_id = entry.id;
+            ledger.unlock();
+            return existing_id;
         }
     }
 
-    // Store a copy of the entry
-    mutex_entries.push_back({
+    FileMutexId new_id = ledger.next_id++;
+    ledger.entries.push_back({
+        .id = new_id,
         .path = path,
         .mutex = *mutex
     });
+
+    ledger.unlock();
+    return new_id;
+}
+
+void file_mutex_remove(FileMutexId id) {
+    auto& ledger = get_ledger();
+    ledger.lock();
+
+    const auto iterator = std::ranges::find_if(ledger.entries, [id](const FileMutexEntry& entry) {
+        return entry.id == id;
+    });
+    if (iterator != ledger.entries.end()) {
+        // Plain erase, not swap-and-pop: file_mutex_get() matches first-registered-wins, so
+        // removal must preserve the relative order of the remaining entries.
+        ledger.entries.erase(iterator);
+    }
+
+    ledger.unlock();
 }
 
 void file_mutex_get(FileMutex* mutex, const char* path) {
+    auto& ledger = get_ledger();
     std::string path_string = path;
-    for (auto& entry : mutex_entries) {
+
+    ledger.lock();
+    for (auto& entry : ledger.entries) {
         // Match the mount path itself, or a descendant (e.g. "/sdcard" registered, "/sdcard/config.json" requested).
         bool is_match = path_string == entry.path ||
             (entry.path == "/" && !path_string.empty() && path_string[0] == '/') ||
             (path_string.rfind(entry.path, 0) == 0 && path_string[entry.path.size()] == '/');
         if (is_match) {
             memcpy(mutex, &entry.mutex, sizeof(FileMutex));
+            ledger.unlock();
             return;
         }
     }
+    ledger.unlock();
 
     *mutex = no_mutex;
 }

TactilityKernel/source/memory_esp32.cpp

@@ -9,7 +9,16 @@ namespace {
 
 uint32_t toHeapCaps(uint16_t capabilityFlags) {
     uint32_t caps = 0;
-    if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) caps |= MALLOC_CAP_INTERNAL;
+    if (capabilityFlags & MEMORY_CAPABILITY_INTERNAL) {
+        caps |= MALLOC_CAP_INTERNAL;
+        // MALLOC_CAP_INTERNAL alone can be satisfied by IRAM (tagged INTERNAL on ESP32's heap
+        // layout), which is word-only and fails FreeRTOS's byte-accessibility checks for things
+        // like a static task's TCB. 8BIT keeps this capability meaning genuinely byte-accessible
+        // internal RAM - but skip it when EXECUTABLE is also requested, since executable IRAM
+        // isn't 8-bit accessible on some ESP32 targets and combining both caps could make an
+        // otherwise-satisfiable request (plain executable internal memory) fail outright.
+        if (!(capabilityFlags & MEMORY_CAPABILITY_EXECUTABLE)) caps |= MALLOC_CAP_8BIT;
+    }
     if (capabilityFlags & MEMORY_CAPABILITY_EXTERNAL) caps |= MALLOC_CAP_SPIRAM;
     if (capabilityFlags & MEMORY_CAPABILITY_EXECUTABLE) caps |= MALLOC_CAP_EXEC;
     if (capabilityFlags & MEMORY_CAPABILITY_DMA) caps |= MALLOC_CAP_DMA;
@@ -25,18 +34,22 @@ 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.
     void* ptr;
     if (policy->alignment > 0) {
         ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | desired_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);
+            ptr = heap_caps_aligned_alloc(policy->alignment, size, required_caps | MALLOC_CAP_DEFAULT);
         }
     } else {
         ptr = heap_caps_malloc(size, required_caps | desired_caps);
         if (ptr == nullptr && desired_caps != 0) {
-            ptr = heap_caps_malloc(size, required_caps);
+            ptr = heap_caps_malloc(size, required_caps | MALLOC_CAP_DEFAULT);
         }
     }
     return ptr;
@@ -50,7 +63,7 @@ void* memory_realloc_with_policy(void* ptr, size_t size, const struct MemoryPoli
     // on fresh allocations (memory_alloc_with_policy/memory_calloc_with_policy).
     void* result = heap_caps_realloc(ptr, size, required_caps | desired_caps);
     if (result == nullptr && desired_caps != 0) {
-        result = heap_caps_realloc(ptr, size, required_caps);
+        result = heap_caps_realloc(ptr, size, required_caps | MALLOC_CAP_DEFAULT);
     }
     return result;
 }
@@ -63,12 +76,12 @@ void* memory_calloc_with_policy(size_t count, size_t size, const struct MemoryPo
     if (policy->alignment > 0) {
         ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps | desired_caps);
         if (ptr == nullptr && desired_caps != 0) {
-            ptr = heap_caps_aligned_calloc(policy->alignment, count, size, required_caps);
+            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);
         if (ptr == nullptr && desired_caps != 0) {
-            ptr = heap_caps_calloc(count, size, required_caps);
+            ptr = heap_caps_calloc(count, size, required_caps | MALLOC_CAP_DEFAULT);
         }
     }
     return ptr;

TactilityKernel/source/symbols.c

@@ -169,7 +169,8 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = {
     DEFINE_MODULE_SYMBOL(display_get_backlight),
     DEFINE_MODULE_SYMBOL(DISPLAY_TYPE),
     // file_mutex
-    DEFINE_MODULE_SYMBOL(file_mutex_register),
+    DEFINE_MODULE_SYMBOL(file_mutex_add),
+    DEFINE_MODULE_SYMBOL(file_mutex_remove),
     DEFINE_MODULE_SYMBOL(file_mutex_get),
     DEFINE_MODULE_SYMBOL(file_mutex_lock),
     DEFINE_MODULE_SYMBOL(file_mutex_try_lock),

TactilityKernel/tests/source/file_mutex_test.cpp

@@ -48,10 +48,10 @@ TEST_CASE("file_mutex_get with zero registrations returns a no-op mutex") {
     file_mutex_unlock(&mutex);
 }
 
-TEST_CASE("file_mutex_register/get with a single registration") {
+TEST_CASE("file_mutex_add/get with a single registration") {
     reset_mocks();
     FileMutex registered = { .lock = mock_lock, .try_lock = mock_try_lock, .unlock = mock_unlock };
-    file_mutex_register(&registered, "/mock1");
+    file_mutex_add(&registered, "/mock1");
 
     FileMutex mutex;
 
@@ -87,19 +87,19 @@ TEST_CASE("file_mutex_register/get with a single registration") {
 
     // Re-registering the same path is a no-op: original callbacks remain in place.
     FileMutex replacement = { .lock = nullptr, .try_lock = nullptr, .unlock = nullptr };
-    file_mutex_register(&replacement, "/mock1");
+    file_mutex_add(&replacement, "/mock1");
     file_mutex_get(&mutex, "/mock1");
     CHECK_EQ(mutex.lock, mock_lock);
 }
 
-TEST_CASE("file_mutex_register/get with two registrations resolves to the matching path") {
+TEST_CASE("file_mutex_add/get with two registrations resolves to the matching path") {
     reset_mocks();
 
     FileMutex mutex_a = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr };
     FileMutex mutex_b = { .lock = mock_lock_b, .try_lock = nullptr, .unlock = nullptr };
 
-    file_mutex_register(&mutex_a, "/mock2a");
-    file_mutex_register(&mutex_b, "/mock2b");
+    file_mutex_add(&mutex_a, "/mock2a");
+    file_mutex_add(&mutex_b, "/mock2b");
 
     FileMutex resolved;
 
@@ -116,7 +116,75 @@ TEST_CASE("file_mutex_register/get with two registrations resolves to the matchi
     // Registration order matters: the first matching entry wins, not the longest
     // prefix. A mount nested under an earlier one is shadowed by it.
     FileMutex mutex_nested = { .lock = nullptr, .try_lock = nullptr, .unlock = nullptr };
-    file_mutex_register(&mutex_nested, "/mock2a/nested");
+    file_mutex_add(&mutex_nested, "/mock2a/nested");
     file_mutex_get(&resolved, "/mock2a/nested/file.txt");
     CHECK_EQ(resolved.lock, mock_lock_a); // still /mock2a, registered first
 }
+
+TEST_CASE("file_mutex_add returns a valid id") {
+    reset_mocks();
+    FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr };
+    FileMutexId id = file_mutex_add(&registered, "/mockA");
+    CHECK_NE(id, FILE_MUTEX_ID_INVALID);
+}
+
+TEST_CASE("file_mutex_add with a duplicate path returns the existing id") {
+    reset_mocks();
+    FileMutex mutex_1 = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr };
+    FileMutex mutex_2 = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr };
+
+    FileMutexId id_1 = file_mutex_add(&mutex_1, "/mockB");
+    FileMutexId id_2 = file_mutex_add(&mutex_2, "/mockB");
+    CHECK_EQ(id_1, id_2);
+
+    FileMutex resolved;
+    file_mutex_get(&resolved, "/mockB");
+    CHECK_EQ(resolved.lock, mock_lock); // first registration's callbacks win, unchanged
+}
+
+TEST_CASE("file_mutex_remove removes a registration") {
+    reset_mocks();
+    FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr };
+    FileMutexId id = file_mutex_add(&registered, "/mockC");
+
+    FileMutex before;
+    file_mutex_get(&before, "/mockC");
+    CHECK_EQ(before.lock, mock_lock);
+
+    file_mutex_remove(id);
+
+    FileMutex after;
+    file_mutex_get(&after, "/mockC");
+    CHECK_EQ(after.lock, nullptr);
+}
+
+TEST_CASE("file_mutex_remove with an unknown id is a safe no-op") {
+    reset_mocks();
+    FileMutex registered = { .lock = mock_lock, .try_lock = nullptr, .unlock = nullptr };
+    file_mutex_add(&registered, "/mockD");
+
+    file_mutex_remove(999999); // never issued
+    file_mutex_remove(FILE_MUTEX_ID_INVALID);
+
+    FileMutex resolved;
+    file_mutex_get(&resolved, "/mockD");
+    CHECK_EQ(resolved.lock, mock_lock); // untouched
+}
+
+TEST_CASE("file_mutex_remove of one registration leaves others intact") {
+    reset_mocks();
+    FileMutex mutex_a = { .lock = mock_lock_a, .try_lock = nullptr, .unlock = nullptr };
+    FileMutex mutex_b = { .lock = mock_lock_b, .try_lock = nullptr, .unlock = nullptr };
+    FileMutexId id_a = file_mutex_add(&mutex_a, "/mockE1");
+    file_mutex_add(&mutex_b, "/mockE2");
+
+    file_mutex_remove(id_a);
+
+    FileMutex resolved_a;
+    file_mutex_get(&resolved_a, "/mockE1");
+    CHECK_EQ(resolved_a.lock, nullptr);
+
+    FileMutex resolved_b;
+    file_mutex_get(&resolved_b, "/mockE2");
+    CHECK_EQ(resolved_b.lock, mock_lock_b);
+}