From 6fb2bb736c9b8a69017d4650a0484926270261dc Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Wed, 15 Jul 2026 22:56:21 +0200 Subject: [PATCH] Drivers, device migrations and other improvements (#566) - RGB display driver added - display_driver API: - Added software-based display rotation for screens without hardware rotation support. - Improved display capability detection across supported display drivers. - lvgl_display driver: - Improved framebuffer handling for smoother, more reliable display updates. - Display gap and rotation behavior now adapts more accurately to hardware capabilities. - Migration Crowpanel 5" basic & advance to kernel drivers - Improve KernelDisplay app: brightness controls are hidden for displays that only support on/off operation - device.py now allows for (non-ambiguous) partial device IDs - TactilityKernel: Implement more tests --- Documentation/ideas.md | 20 +- Drivers/hx8357-module/source/hx8357.cpp | 27 +-- Drivers/ili9341-module/source/ili9341.cpp | 4 + Drivers/ili9488-module/source/ili9488.cpp | 4 + .../rgb-display-module/source/rgb_display.cpp | 68 +++++-- .../source/st7789_i8080.cpp | 4 + Drivers/st7789-module/source/st7789.cpp | 4 + .../include/tactility/lvgl_display.h | 7 + Modules/lvgl-module/source/lvgl_display.c | 130 ++++++++++++- .../app/kerneldisplay/KernelDisplay.cpp | 39 ++-- .../include/tactility/drivers/display.h | 44 +++++ TactilityKernel/source/drivers/display.cpp | 9 + .../Source/DeviceListenerTest.cpp | 119 ++++++++++++ .../TactilityKernel/Source/FileSystemTest.cpp | 182 ++++++++++++++++++ Tests/TactilityKernel/Source/PathsTest.cpp | 31 +++ device.py | 21 +- 16 files changed, 627 insertions(+), 86 deletions(-) create mode 100644 Tests/TactilityKernel/Source/DeviceListenerTest.cpp create mode 100644 Tests/TactilityKernel/Source/FileSystemTest.cpp create mode 100644 Tests/TactilityKernel/Source/PathsTest.cpp diff --git a/Documentation/ideas.md b/Documentation/ideas.md index 59e41d27..c19f848f 100644 --- a/Documentation/ideas.md +++ b/Documentation/ideas.md @@ -12,20 +12,12 @@ ## Higher Priority -- When display orientation is changed, show a dialog that asks if it succeeded. If the user doesn't click "ok" then restore the previous settings. +- Get rid of TactilityC in favour of TactilityKernel and kernel modules +- Improve SPI kernel driver (implement read, write, transactions) - Add font design tokens such as "regular", "title" and "smaller". Perhaps via the LVGL kernel module. -- Add kernel listening mechanism so that the root device init can be notified when a device becomes available: - Callback for device/start stop with filtering on device type: - - on_before_start: e.g. to do the CS pin hack for SD card on a shared bus - - on_after_start: e.g. to initialize an I2S device via its I2C connection, once I2C becomes available - - on_before_stop: e.g. to stop using a device that was started before - - on_after_stop: ? -- SPI kernel driver - Kernel concepts for ELF loading (generic approach for GUI apps, console apps, libraries). - Fix glitches when installing app via App Hub with 4.3" Waveshare - TCA9534 keyboards should use interrupts -- Fix Cardputer (original): use LV_KEY_NEXT and _PREV in keyboard mapping instead of encoder driver hack (and check GPIO app if it then hangs too) -- Expose http::download() and main dispatcher to TactiltyC. - External app loading: Check the version of Tactility and check ESP target hardware to check for compatibility Check during installation process, but also when starting (SD card might have old app install from before Tactility OS update) - Make a URL handler. Use it for handling local files. Match file types with apps. @@ -49,7 +41,6 @@ - Make WiFi setup app that starts an access point and hosts a webpage to set up the device. This will be useful for devices without a screen, a small screen or a non-touch screen. - Unify the way displays are dimmed. Some implementations turn off the display when it's fully dimmed. Make this a separate functionality. -- Bug: Turn on WiFi (when testing it wasn't connected/connecting - just active). Open chat. Observe crash. - Bug: Crash handling app cannot be exited with an EncoderDevice. (current work-around is to manually reset the device) ## Lower Priority @@ -65,9 +56,6 @@ - Fix system time to not be 1980 (use build year as a minimum). Consider keeping track of the last known time. - Use std::span or string_view in StringUtils https://youtu.be/FRkJCvHWdwQ?t=2754 - Mutex: Implement give/take from ISR support (works only for non-recursive ones) -- Extend unPhone power driver: add charging status, usb connection status, etc. -- Clear screen before turning on blacklight (e.g. T-Deck, CYD 2432S028R, etc.) -- T-Deck: Use trackball as input device (with optional mouse functionality for LVGL) - Show a warning screen if firmware encryption or secure boot are off when saving WiFi credentials. - Remove flex_flow from app_container in Gui.cpp - ElfAppManifest: change name (remove "manifest" as it's confusing), remove icon and title, publish snapshot SDK on CDN @@ -82,11 +70,7 @@ - Audio player app - Audio recording app - OTA updates -- T-Deck Plus: Create separate device config (pre-requisite: needs more kernel drivers so the driver count in the device project is reduced) - If present, use LED to show boot/wifi status -- Capacity based on voltage: estimation for various devices uses a linear voltage curve, but it should use a battery discharge curve. -- Wrapper for lvgl slider widget that shows "+" and "-" buttons, and also the value in a label. - Note: consider Spinbox - On crash, try to save the current log to flash or SD card? (this is risky, though, so ask in Discord first) - Support more than 1 hardware keyboard (see lvgl::hardware_keyboard_set_indev()). LVGL init currently calls keyboard init, but that part should probably be done from the KeyboardDevice base class. diff --git a/Drivers/hx8357-module/source/hx8357.cpp b/Drivers/hx8357-module/source/hx8357.cpp index 6db46c09..65ab3aac 100644 --- a/Drivers/hx8357-module/source/hx8357.cpp +++ b/Drivers/hx8357-module/source/hx8357.cpp @@ -54,11 +54,7 @@ constexpr uint8_t MADCTL_BIT_PAGE_COLUMN_ORDER = 5; // swap-xy constexpr uint8_t MADCTL_BIT_COLUMN_ADDRESS_ORDER = 6; // mirror-x constexpr uint8_t MADCTL_BIT_PAGE_ADDRESS_ORDER = 7; // mirror-y -// Bring-up command list, ported verbatim (byte-for-byte) from the deprecated HAL's hx8357.c -// initd[] table (Devices/unphone/Source/hx8357/hx8357.c) - the HX8357B variant (initb[]) is -// dead code there (displayType is hardcoded to HX8357D) and was not ported. Format matches the -// original: cmd, then a length byte (bit7 set means "this is actually a delay in 5ms units, no -// data follows"), then that many data bytes. A single 0 cmd byte ends the list. +// Bring-up command list constexpr uint8_t INIT_CMDS[] = { HX8357_SWRESET, 0x80 + 100 / 5, // Soft reset, then delay 100 ms HX8357D_SETC, 3, @@ -118,11 +114,6 @@ struct Hx8357Internal { spi_device_handle_t spi_handle; gpio_num_t dc_pin; size_t max_transfer_size; - // Live MADCTL orientation bits, seeded from config at start() and updated in place by - // mirror()/swap_xy() - NOT reconstructed from the static config each call. swap_xy() and - // mirror() are invoked back-to-back by lvgl_display_apply_rotation() on every rotation - // change, so rebuilding from config would silently discard whichever bit the other call - // just set. bool swap_xy; bool mirror_x; bool mirror_y; @@ -379,13 +370,6 @@ static bool hx8357_get_mirror_y(Device* device) { return GET_CONFIG(device)->mirror_y; } -static error_t hx8357_set_gap(Device*, int32_t, int32_t) { - // Not supported by this controller's fixed CASET/PASET-per-draw protocol beyond the static - // gap-x/gap-y devicetree config already folded into draw_bitmap(); matches the deprecated - // HAL, which never exposed a runtime gap either. - return ERROR_NOT_SUPPORTED; -} - static error_t hx8357_invert_color(Device* device, bool invert_color_data) { auto* internal = static_cast(device_get_driver_data(device)); send_cmd(internal, invert_color_data ? HX8357_INVON : HX8357_INVOFF); @@ -436,6 +420,9 @@ static error_t hx8357_get_backlight(Device* device, Device** backlight) { // endregion static const DisplayApi hx8357_display_api = { + .capabilities = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | DISPLAY_CAPABILITY_SLEEP | + DISPLAY_CAPABILITY_BACKLIGHT, .reset = hx8357_reset, .init = hx8357_init, .draw_bitmap = hx8357_draw_bitmap, @@ -444,7 +431,10 @@ static const DisplayApi hx8357_display_api = { .get_swap_xy = hx8357_get_swap_xy, .get_mirror_x = hx8357_get_mirror_x, .get_mirror_y = hx8357_get_mirror_y, - .set_gap = hx8357_set_gap, + // set_gap is not exposed: this controller's fixed CASET/PASET-per-draw protocol only supports the + // static gap-x/gap-y devicetree config already folded into draw_bitmap(); matches the deprecated + // HAL, which never exposed a runtime gap either. + .set_gap = nullptr, .invert_color = hx8357_invert_color, .disp_on_off = hx8357_disp_on_off, .disp_sleep = hx8357_disp_sleep, @@ -454,6 +444,7 @@ static const DisplayApi hx8357_display_api = { .get_frame_buffer = hx8357_get_frame_buffer, .get_frame_buffer_count = hx8357_get_frame_buffer_count, .get_backlight = hx8357_get_backlight, + .has_capability = nullptr, }; Driver hx8357_driver = { diff --git a/Drivers/ili9341-module/source/ili9341.cpp b/Drivers/ili9341-module/source/ili9341.cpp index 8971480d..8f7caaa3 100644 --- a/Drivers/ili9341-module/source/ili9341.cpp +++ b/Drivers/ili9341-module/source/ili9341.cpp @@ -300,6 +300,9 @@ static error_t ili9341_get_backlight(Device* device, Device** backlight) { // endregion static const DisplayApi ili9341_display_api = { + .capabilities = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_CAP_SET_GAP | DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | + DISPLAY_CAPABILITY_SLEEP | DISPLAY_CAPABILITY_BACKLIGHT, .reset = ili9341_reset, .init = ili9341_init, .draw_bitmap = ili9341_draw_bitmap, @@ -318,6 +321,7 @@ static const DisplayApi ili9341_display_api = { .get_frame_buffer = ili9341_get_frame_buffer, .get_frame_buffer_count = ili9341_get_frame_buffer_count, .get_backlight = ili9341_get_backlight, + .has_capability = nullptr, }; Driver ili9341_driver = { diff --git a/Drivers/ili9488-module/source/ili9488.cpp b/Drivers/ili9488-module/source/ili9488.cpp index 952295fc..96cc1dec 100644 --- a/Drivers/ili9488-module/source/ili9488.cpp +++ b/Drivers/ili9488-module/source/ili9488.cpp @@ -291,6 +291,9 @@ static error_t ili9488_get_backlight(Device* device, Device** backlight) { // endregion static const DisplayApi ili9488_display_api = { + .capabilities = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_CAP_SET_GAP | DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | + DISPLAY_CAPABILITY_SLEEP | DISPLAY_CAPABILITY_BACKLIGHT, .reset = ili9488_reset, .init = ili9488_init, .draw_bitmap = ili9488_draw_bitmap, @@ -309,6 +312,7 @@ static const DisplayApi ili9488_display_api = { .get_frame_buffer = ili9488_get_frame_buffer, .get_frame_buffer_count = ili9488_get_frame_buffer_count, .get_backlight = ili9488_get_backlight, + .has_capability = nullptr, }; Driver ili9488_driver = { diff --git a/Drivers/rgb-display-module/source/rgb_display.cpp b/Drivers/rgb-display-module/source/rgb_display.cpp index 93cdccbe..631877b3 100644 --- a/Drivers/rgb-display-module/source/rgb_display.cpp +++ b/Drivers/rgb-display-module/source/rgb_display.cpp @@ -34,8 +34,11 @@ struct RgbDisplayInternal { esp_lcd_panel_handle_t panel_handle; void* frame_buffers[MAX_CACHED_FRAME_BUFFERS]; uint8_t frame_buffer_count; + // Size of each buffer in frame_buffers, in bytes - used to range-check whether a given + // draw_bitmap() color_data pointer is actually one of them (see rgb_display_draw_bitmap()). + size_t frame_buffer_size_bytes; // Signaled by on_frame_buf_complete once per real DMA scan-out of a whole frame. Only - // waited on in draw_bitmap() when frame_buffer_count > 0 - see the comment there for why. + // waited on in draw_bitmap() when color_data is one of frame_buffers - see the comment there for why. SemaphoreHandle_t frame_complete_semaphore; }; @@ -89,6 +92,8 @@ static error_t perform_hardware_reset(const RgbDisplayConfig* config) { static error_t cache_frame_buffers(RgbDisplayInternal* internal, const RgbDisplayConfig* config) { internal->frame_buffer_count = 0; + internal->frame_buffer_size_bytes = (size_t)config->horizontal_resolution * config->vertical_resolution * + ((config->bits_per_pixel + 7) / 8); if (config->num_fbs == 0) { return ERROR_NONE; } @@ -277,16 +282,26 @@ static error_t rgb_display_init(Device* device) { return esp_lcd_panel_init(internal->panel_handle) == ESP_OK ? ERROR_NONE : ERROR_RESOURCE; } +// Only block for scan-out completion when color_data is actually one of the panel's own frame +// buffers (see on_frame_buf_complete's comment above for why that matters) - i.e. this specific +// call is a zero-copy flip, not a plain CPU copy into the panel's buffer from a caller-owned one +// (e.g. LVGL bound in owned-buffer mode), which has no reuse race to guard against and shouldn't +// pay the up-to-one-frame latency cost for every partial update. +static bool rgb_display_color_data_is_frame_buffer(const RgbDisplayInternal* internal, const void* color_data) { + const auto* ptr = static_cast(color_data); + for (uint8_t i = 0; i < internal->frame_buffer_count; i++) { + const auto* base = static_cast(internal->frame_buffers[i]); + if (ptr >= base && ptr < base + internal->frame_buffer_size_bytes) { + return true; + } + } + return false; +} + static error_t rgb_display_draw_bitmap(Device* device, int32_t x_start, int32_t y_start, int32_t x_end, int32_t y_end, const void* color_data) { auto* internal = static_cast(device_get_driver_data(device)); - // Only block for scan-out completion when the caller could be writing straight into one of - // the panel's own frame buffers (see on_frame_buf_complete's comment above for why that - // matters). With no real frame buffer of ours involved (frame_buffer_count == 0, e.g. LVGL - // partial-render mode with its own separate buffer), draw_bitmap does a real memcpy into the - // panel's buffer and there's no reuse race to guard against, so don't pay the up-to-one-frame - // latency cost for every small partial update. - bool wait_for_scanout = internal->frame_buffer_count > 0; + bool wait_for_scanout = rgb_display_color_data_is_frame_buffer(internal, color_data); if (wait_for_scanout) { xSemaphoreTake(internal->frame_complete_semaphore, 0); // clear any already-pending signal } @@ -324,11 +339,8 @@ static bool rgb_display_get_mirror_y(Device* device) { return GET_CONFIG(device)->mirror_y; } -// RGB panels are raw scan-out framebuffers with no addressable-window concept the way MIPI/SPI -// panels have, so there's no gap to set. -static error_t rgb_display_set_gap(Device*, int32_t, int32_t) { - return ERROR_NOT_SUPPORTED; -} +// set_gap is not exposed: RGB panels are raw scan-out framebuffers with no addressable-window +// concept the way MIPI/SPI panels have, so there's no gap to set. static error_t rgb_display_invert_color(Device* device, bool invert_color_data) { auto* internal = static_cast(device_get_driver_data(device)); @@ -340,10 +352,8 @@ static error_t rgb_display_disp_on_off(Device* device, bool on_off) { return esp_lcd_panel_disp_on_off(internal->panel_handle, on_off) == ESP_OK ? ERROR_NONE : ERROR_RESOURCE; } -// RGB panels have no MIPI DCS command interface, so there's no sleep mode to enter. -static error_t rgb_display_disp_sleep(Device*, bool) { - return ERROR_NOT_SUPPORTED; -} +// disp_sleep is not exposed: RGB panels have no MIPI DCS command interface, so there's no sleep +// mode to enter. static enum DisplayColorFormat rgb_display_get_color_format(Device*) { return DISPLAY_COLOR_FORMAT_RGB565; @@ -376,9 +386,28 @@ static error_t rgb_display_get_backlight(Device* device, Device** backlight) { return ERROR_NONE; } +constexpr uint32_t RGB_DISPLAY_CAPABILITIES = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | DISPLAY_CAPABILITY_BACKLIGHT; + +// esp_lcd_panel_rgb only applies mirror()/swap_xy()'s rotate_mask when draw_bitmap()'s color_data +// is copied into the frame buffer by CPU. When frame_buffer_count > 0, LVGL is bound directly onto +// the panel's own frame buffers (see lvgl_display.c), so color_data always already *is* the frame +// buffer and that copy - and with it the rotation - never happens. Report those two capabilities as +// unavailable in that configuration so callers (e.g. lvgl_display.c) don't rely on a rotation that +// silently does nothing. +static bool rgb_display_has_capability(Device* device, uint32_t capability) { + auto* internal = static_cast(device_get_driver_data(device)); + uint32_t capabilities = RGB_DISPLAY_CAPABILITIES; + if (internal->frame_buffer_count > 0) { + capabilities &= ~(DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY); + } + return (capabilities & capability) == capability; +} + // endregion static const DisplayApi rgb_display_api = { + .capabilities = RGB_DISPLAY_CAPABILITIES, .reset = rgb_display_reset, .init = rgb_display_init, .draw_bitmap = rgb_display_draw_bitmap, @@ -387,16 +416,17 @@ static const DisplayApi rgb_display_api = { .get_swap_xy = rgb_display_get_swap_xy, .get_mirror_x = rgb_display_get_mirror_x, .get_mirror_y = rgb_display_get_mirror_y, - .set_gap = rgb_display_set_gap, + .set_gap = nullptr, .invert_color = rgb_display_invert_color, .disp_on_off = rgb_display_disp_on_off, - .disp_sleep = rgb_display_disp_sleep, + .disp_sleep = nullptr, .get_color_format = rgb_display_get_color_format, .get_resolution_x = rgb_display_get_resolution_x, .get_resolution_y = rgb_display_get_resolution_y, .get_frame_buffer = rgb_display_get_frame_buffer, .get_frame_buffer_count = rgb_display_get_frame_buffer_count, .get_backlight = rgb_display_get_backlight, + .has_capability = rgb_display_has_capability, }; Driver rgb_display_driver = { diff --git a/Drivers/st7789-i8080-module/source/st7789_i8080.cpp b/Drivers/st7789-i8080-module/source/st7789_i8080.cpp index 6d0d12ef..38c121f0 100644 --- a/Drivers/st7789-i8080-module/source/st7789_i8080.cpp +++ b/Drivers/st7789-i8080-module/source/st7789_i8080.cpp @@ -324,6 +324,9 @@ static error_t st7789_i8080_get_backlight(Device* device, Device** backlight) { // endregion static const DisplayApi st7789_i8080_display_api = { + .capabilities = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_CAP_SET_GAP | DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | + DISPLAY_CAPABILITY_SLEEP | DISPLAY_CAPABILITY_BACKLIGHT, .reset = st7789_i8080_reset, .init = st7789_i8080_init, .draw_bitmap = st7789_i8080_draw_bitmap, @@ -342,6 +345,7 @@ static const DisplayApi st7789_i8080_display_api = { .get_frame_buffer = st7789_i8080_get_frame_buffer, .get_frame_buffer_count = st7789_i8080_get_frame_buffer_count, .get_backlight = st7789_i8080_get_backlight, + .has_capability = nullptr, }; Driver st7789_i8080_driver = { diff --git a/Drivers/st7789-module/source/st7789.cpp b/Drivers/st7789-module/source/st7789.cpp index c76742a7..2c090642 100644 --- a/Drivers/st7789-module/source/st7789.cpp +++ b/Drivers/st7789-module/source/st7789.cpp @@ -294,6 +294,9 @@ static error_t st7789_get_backlight(Device* device, Device** backlight) { // endregion static const DisplayApi st7789_display_api = { + .capabilities = DISPLAY_CAPABILITY_CAP_MIRROR | DISPLAY_CAPABILITY_CAP_SWAP_XY | + DISPLAY_CAPABILITY_CAP_SET_GAP | DISPLAY_CAPABILITY_INVERT_COLOR | DISPLAY_CAPABILITY_ON_OFF | + DISPLAY_CAPABILITY_SLEEP | DISPLAY_CAPABILITY_BACKLIGHT, .reset = st7789_reset, .init = st7789_init, .draw_bitmap = st7789_draw_bitmap, @@ -312,6 +315,7 @@ static const DisplayApi st7789_display_api = { .get_frame_buffer = st7789_get_frame_buffer, .get_frame_buffer_count = st7789_get_frame_buffer_count, .get_backlight = st7789_get_backlight, + .has_capability = nullptr, }; Driver st7789_driver = { diff --git a/Modules/lvgl-module/include/tactility/lvgl_display.h b/Modules/lvgl-module/include/tactility/lvgl_display.h index 106ae56d..eee2f8c7 100644 --- a/Modules/lvgl-module/include/tactility/lvgl_display.h +++ b/Modules/lvgl-module/include/tactility/lvgl_display.h @@ -28,6 +28,13 @@ struct LvglDisplayConfig { * Ignored when the device exposes its own frame buffer(s). */ bool double_buffer; + + /** + * Rotate LVGL_rendered content in software instead of calling display_swap_xy()/display_mirror() + * on the device. Use this for panels whose driver can't rotate in hardware (e.g. RGB/DPI panels). + * Allocates one extra buffer sized like the primary draw buffer. + */ + bool sw_rotate; }; /** diff --git a/Modules/lvgl-module/source/lvgl_display.c b/Modules/lvgl-module/source/lvgl_display.c index d67fafb4..dcc91749 100644 --- a/Modules/lvgl-module/source/lvgl_display.c +++ b/Modules/lvgl-module/source/lvgl_display.c @@ -21,6 +21,17 @@ struct LvglDisplayCtx { bool base_swap_xy; bool base_mirror_x; bool base_mirror_y; + // When true, rotation is done in software in the flush callback instead of via display_swap_xy()/ + // display_mirror(); rotate_buf holds the rotated pixels and is sized like buf1. + bool sw_rotate; + void* rotate_buf; + // Size of buf1/buf2 (each) - used by lvgl_display_fb_base() to range-check which real buffer + // (for the fb-direct case) a given color_map pointer falls into. + size_t buf_size_bytes; + // Cached DISPLAY_CAPABILITY_CAP_SWAP_XY/CAP_MIRROR: swap_xy()/mirror() and their getters are + // null on drivers that don't support them, so rotation handling must not call through blindly. + bool has_swap_xy_cap; + bool has_mirror_cap; // True for DISPLAY_COLOR_FORMAT_RGB565_SWAPPED: the panel wants each 16-bit pixel high-byte-first // over SPI, which this little-endian CPU's native RGB565 buffer isn't. LVGL's own // LV_COLOR_FORMAT_RGB565_SWAPPED display format looked like the fit but isn't fully supported by @@ -74,6 +85,11 @@ static bool lvgl_display_map_color_format(enum DisplayColorFormat in, lv_color_f } static void lvgl_display_apply_rotation(struct LvglDisplayCtx* ctx, lv_display_rotation_t rotation) { + // SW-rotated displays stay in their base orientation; rotation is applied per-flush instead. + if (ctx->sw_rotate) { + return; + } + bool swap_xy = ctx->base_swap_xy; bool mirror_x = ctx->base_mirror_x; bool mirror_y = ctx->base_mirror_y; @@ -103,8 +119,12 @@ static void lvgl_display_apply_rotation(struct LvglDisplayCtx* ctx, lv_display_r break; } - display_swap_xy(ctx->device, swap_xy); - display_mirror(ctx->device, mirror_x, mirror_y); + if (ctx->has_swap_xy_cap) { + display_swap_xy(ctx->device, swap_xy); + } + if (ctx->has_mirror_cap) { + display_mirror(ctx->device, mirror_x, mirror_y); + } } static void lvgl_display_rotation_event_cb(lv_event_t* e) { @@ -113,13 +133,69 @@ static void lvgl_display_rotation_event_cb(lv_event_t* e) { lvgl_display_apply_rotation(ctx, lv_display_get_rotation(disp)); } +// Returns which of buf1/buf2 (the real frame buffers, when !owns_buffers) color_map falls inside. +// Defaults to buf1, which also covers the single-frame-buffer (buf2 == NULL) case. +static void* lvgl_display_fb_base(struct LvglDisplayCtx* ctx, const uint8_t* color_map) { + if (ctx->buf2 != NULL && color_map >= (uint8_t*)ctx->buf2 && + color_map < (uint8_t*)ctx->buf2 + ctx->buf_size_bytes) { + return ctx->buf2; + } + return ctx->buf1; +} + static void lvgl_display_flush_cb(lv_display_t* disp, const lv_area_t* area, uint8_t* color_map) { struct LvglDisplayCtx* ctx = (struct LvglDisplayCtx*)lv_display_get_driver_data(disp); - if (ctx->byte_swap) { - lv_draw_sw_rgb565_swap(color_map, lv_area_get_size(area)); + + int32_t x1 = area->x1; + int32_t y1 = area->y1; + int32_t x2 = area->x2; + int32_t y2 = area->y2; + uint32_t area_size_px = (uint32_t)(x2 - x1 + 1) * (uint32_t)(y2 - y1 + 1); + + lv_display_rotation_t rotation = lv_display_get_rotation(disp); + if (ctx->sw_rotate && rotation != LV_DISPLAY_ROTATION_0) { + // sw_rotate is only ever requested for displays lacking real HW mirror/swap_xy capability + // (see lvgl_devices.c), and lvgl_display_add() only binds fb-direct (owns_buffers == false) + // when that capability is present - so this is always the owns_buffers == true case. + lv_color_format_t color_format = lv_display_get_color_format(disp); + int32_t w = x2 - x1 + 1; + int32_t h = y2 - y1 + 1; + uint32_t w_stride = lv_draw_buf_width_to_stride(w, color_format); + uint32_t h_stride = lv_draw_buf_width_to_stride(h, color_format); + if (rotation == LV_DISPLAY_ROTATION_180) { + lv_draw_sw_rotate(color_map, ctx->rotate_buf, w, h, w_stride, w_stride, rotation, color_format); + } else { + lv_draw_sw_rotate(color_map, ctx->rotate_buf, w, h, w_stride, h_stride, rotation, color_format); + } + color_map = (uint8_t*)ctx->rotate_buf; + lv_area_t rotated_area = { x1, y1, x2, y2 }; + lv_display_rotate_area(disp, &rotated_area); + x1 = rotated_area.x1; + y1 = rotated_area.y1; + x2 = rotated_area.x2; + y2 = rotated_area.y2; + } + + if (ctx->byte_swap) { + lv_draw_sw_rgb565_swap(color_map, area_size_px); + } + + if (ctx->owns_buffers) { + // LVGL's area is inclusive; DisplayApi's draw_bitmap wants an exclusive end. + display_draw_bitmap(ctx->device, x1, y1, x2 + 1, y2 + 1, color_map); + } else if (lv_display_flush_is_last(disp)) { + // fb-direct: a refresh cycle can call this flush_cb once per still-unjoined invalidated area + // (lv_refr.c's refr_invalid_areas()), each writing its own tile into the *same* shared frame + // buffer - but display_draw_bitmap() (see rgb_display_draw_bitmap()) blocks for a full + // scan-out period whenever frame_buffer_count > 0. Presenting after every tile would pay + // that wait N times per refresh instead of once; defer to the last flush of the cycle and + // present the whole buffer in one call, mirroring esp_lvgl_port_disp.c's + // lv_disp_flush_is_last() gate for its own direct/full render mode. + uint8_t* fb_base = (uint8_t*)lvgl_display_fb_base(ctx, color_map); + uint16_t hres = display_get_resolution_x(ctx->device); + uint16_t vres = display_get_resolution_y(ctx->device); + display_draw_bitmap(ctx->device, 0, 0, hres, vres, fb_base); } - // LVGL's area is inclusive; DisplayApi's draw_bitmap wants an exclusive end. - display_draw_bitmap(ctx->device, area->x1, area->y1, area->x2 + 1, area->y2 + 1, color_map); // DisplayApi has no async completion callback, so draw_bitmap is synchronous. lv_display_flush_ready(disp); } @@ -151,14 +227,30 @@ error_t lvgl_display_add(struct Device* device, const struct LvglDisplayConfig* } ctx->device = device; ctx->byte_swap = byte_swap; - ctx->base_swap_xy = display_get_swap_xy(device); - ctx->base_mirror_x = display_get_mirror_x(device); - ctx->base_mirror_y = display_get_mirror_y(device); + ctx->sw_rotate = config->sw_rotate; + ctx->has_swap_xy_cap = display_has_capability(device, DISPLAY_CAPABILITY_CAP_SWAP_XY); + ctx->has_mirror_cap = display_has_capability(device, DISPLAY_CAPABILITY_CAP_MIRROR); + // fb_count > 0 with capability false means a driver reports mirror/swap_xy unavailable + // specifically because binding fb-direct would defeat them (see rgb_display_has_capability()). + // We're about to fall back to an owned buffer instead (right below) precisely because of that, + // at which point the concern doesn't apply anymore: frame_buffer_count > 0 mattering to + // has_capability() at all implies the driver's mirror()/swap_xy() are real, non-null + // implementations, and the copy-into-fb path honors them correctly once not fb-direct-bound. + // sw_rotate is excluded too: it writes rotated pixels into ctx->rotate_buf, which + // lvgl_display_fb_base() doesn't recognize, so fb-direct must stay off in that case as well. + bool would_bind_fb_direct = fb_count > 0 && ctx->has_swap_xy_cap && ctx->has_mirror_cap && !ctx->sw_rotate; + if (fb_count > 0 && !would_bind_fb_direct) { + ctx->has_swap_xy_cap = true; + ctx->has_mirror_cap = true; + } + ctx->base_swap_xy = ctx->has_swap_xy_cap ? display_get_swap_xy(device) : false; + ctx->base_mirror_x = ctx->has_mirror_cap ? display_get_mirror_x(device) : false; + ctx->base_mirror_y = ctx->has_mirror_cap ? display_get_mirror_y(device) : false; lv_display_render_mode_t render_mode; size_t buf_size_bytes; - if (fb_count > 0) { + if (would_bind_fb_direct) { display_get_frame_buffer(device, 0, &ctx->buf1); if (fb_count > 1) { display_get_frame_buffer(device, 1, &ctx->buf2); @@ -187,12 +279,27 @@ error_t lvgl_display_add(struct Device* device, const struct LvglDisplayConfig* render_mode = LV_DISPLAY_RENDER_MODE_PARTIAL; } + ctx->buf_size_bytes = buf_size_bytes; + + if (ctx->sw_rotate) { + ctx->rotate_buf = lvgl_display_alloc_buffer(buf_size_bytes); + if (ctx->rotate_buf == NULL) { + if (ctx->owns_buffers) { + lvgl_display_free_buffer(ctx->buf1); + lvgl_display_free_buffer(ctx->buf2); + } + free(ctx); + return ERROR_OUT_OF_MEMORY; + } + } + lv_display_t* disp = lv_display_create(hres, vres); if (disp == NULL) { if (ctx->owns_buffers) { lvgl_display_free_buffer(ctx->buf1); lvgl_display_free_buffer(ctx->buf2); } + lvgl_display_free_buffer(ctx->rotate_buf); free(ctx); return ERROR_OUT_OF_MEMORY; } @@ -227,6 +334,9 @@ void lvgl_display_remove(lv_display_t* display) { lvgl_display_free_buffer(ctx->buf2); } } + if (ctx->rotate_buf != NULL) { + lvgl_display_free_buffer(ctx->rotate_buf); + } free(ctx); } } diff --git a/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp b/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp index 77e42b31..6db58af5 100644 --- a/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp +++ b/Tactility/Source/app/kerneldisplay/KernelDisplay.cpp @@ -140,25 +140,28 @@ public: // DisplayApi has no gamma curve control yet. if (backlight != nullptr) { - auto* brightness_wrapper = lv_obj_create(main_wrapper); - lv_obj_set_size(brightness_wrapper, LV_PCT(100), LV_SIZE_CONTENT); - lv_obj_set_style_pad_hor(brightness_wrapper, 0, LV_STATE_DEFAULT); - lv_obj_set_style_border_width(brightness_wrapper, 0, LV_STATE_DEFAULT); - if (ui_density != LVGL_UI_DENSITY_COMPACT) { - lv_obj_set_style_pad_ver(brightness_wrapper, 4, LV_STATE_DEFAULT); + bool is_on_off_brightness = backlight_get_min_brightness(backlight) == 0 && backlight_get_max_brightness(backlight) == 1; + if (!is_on_off_brightness) { + auto* brightness_wrapper = lv_obj_create(main_wrapper); + lv_obj_set_size(brightness_wrapper, LV_PCT(100), LV_SIZE_CONTENT); + lv_obj_set_style_pad_hor(brightness_wrapper, 0, LV_STATE_DEFAULT); + lv_obj_set_style_border_width(brightness_wrapper, 0, LV_STATE_DEFAULT); + if (ui_density != LVGL_UI_DENSITY_COMPACT) { + lv_obj_set_style_pad_ver(brightness_wrapper, 4, LV_STATE_DEFAULT); + } + + auto* brightness_label = lv_label_create(brightness_wrapper); + lv_label_set_text(brightness_label, "Brightness"); + lv_obj_align(brightness_label, LV_ALIGN_LEFT_MID, 0, 0); + + auto* brightness_slider = lv_slider_create(brightness_wrapper); + lv_obj_set_width(brightness_slider, LV_PCT(50)); + lv_obj_align(brightness_slider, LV_ALIGN_RIGHT_MID, 0, 0); + lv_slider_set_range(brightness_slider, backlight_get_min_brightness(backlight), backlight_get_max_brightness(backlight)); + lv_obj_add_event_cb(brightness_slider, onBacklightSliderEvent, LV_EVENT_VALUE_CHANGED, this); + + lv_slider_set_value(brightness_slider, displaySettings.backlightDuty, LV_ANIM_OFF); } - - auto* brightness_label = lv_label_create(brightness_wrapper); - lv_label_set_text(brightness_label, "Brightness"); - lv_obj_align(brightness_label, LV_ALIGN_LEFT_MID, 0, 0); - - auto* brightness_slider = lv_slider_create(brightness_wrapper); - lv_obj_set_width(brightness_slider, LV_PCT(50)); - lv_obj_align(brightness_slider, LV_ALIGN_RIGHT_MID, 0, 0); - lv_slider_set_range(brightness_slider, backlight_get_min_brightness(backlight), backlight_get_max_brightness(backlight)); - lv_obj_add_event_cb(brightness_slider, onBacklightSliderEvent, LV_EVENT_VALUE_CHANGED, this); - - lv_slider_set_value(brightness_slider, displaySettings.backlightDuty, LV_ANIM_OFF); } // Orientation diff --git a/TactilityKernel/include/tactility/drivers/display.h b/TactilityKernel/include/tactility/drivers/display.h index 7f303a97..0355dd13 100644 --- a/TactilityKernel/include/tactility/drivers/display.h +++ b/TactilityKernel/include/tactility/drivers/display.h @@ -10,6 +10,19 @@ extern "C" { #include #include +/** + * @brief Optional capabilities of the display. + */ +enum DisplayCapability { + DISPLAY_CAPABILITY_CAP_MIRROR = 1 << 0, + DISPLAY_CAPABILITY_CAP_SWAP_XY = 1 << 1, + DISPLAY_CAPABILITY_CAP_SET_GAP = 1 << 2, + DISPLAY_CAPABILITY_INVERT_COLOR = 1 << 3, + DISPLAY_CAPABILITY_ON_OFF = 1 << 4, + DISPLAY_CAPABILITY_BACKLIGHT = 1 << 5, + DISPLAY_CAPABILITY_SLEEP = 1 << 6 +}; + /** * @brief Pixel color formats supported by display panel drivers. */ @@ -26,6 +39,8 @@ enum DisplayColorFormat { * @brief API for display panel drivers. */ struct DisplayApi { + uint32_t capabilities; + /** * @brief Performs a hardware reset of the panel (e.g. toggling a reset GPIO). * @param[in] device the display device @@ -54,6 +69,7 @@ struct DisplayApi { /** * @brief Mirrors the image along the X and/or Y axis. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -61,6 +77,7 @@ struct DisplayApi { /** * @brief Swaps the X and Y axes (rotates the coordinate system). + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -68,6 +85,7 @@ struct DisplayApi { /** * @brief Gets whether the X and Y axes are currently swapped. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @return true if swapped */ @@ -75,6 +93,7 @@ struct DisplayApi { /** * @brief Gets whether the X axis is currently mirrored. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @return true if mirrored */ @@ -82,6 +101,7 @@ struct DisplayApi { /** * @brief Gets whether the Y axis is currently mirrored. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @return true if mirrored */ @@ -89,6 +109,7 @@ struct DisplayApi { /** * @brief Sets a coordinate offset applied to all draw operations. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -96,6 +117,7 @@ struct DisplayApi { /** * @brief Inverts the panel's color output. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -103,6 +125,7 @@ struct DisplayApi { /** * @brief Turns the panel's display output on or off. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -110,6 +133,7 @@ struct DisplayApi { /** * @brief Puts the panel into or out of its low-power sleep mode. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @retval ERROR_NONE when the operation was successful */ @@ -153,14 +177,34 @@ struct DisplayApi { /** * @brief Gets the backlight device associated with this display, if any. + * @warning Function pointer should be null if capability not available. * @param[in] device the display device * @param[out] backlight the associated backlight device * @retval ERROR_NONE when a backlight is available and *backlight was set * @retval ERROR_NOT_SUPPORTED when this display has no associated backlight */ error_t (*get_backlight)(struct Device* device, struct Device** backlight); + + /** + * @brief Optional dynamic override for capability checks. + * @warning Nullable. When null, display_has_capability() checks the static `capabilities` field + * instead. When set, this is called in its place, letting a driver report capabilities that vary + * per device instance (e.g. depending on devicetree config) rather than being fixed for the driver. + * @param[in] device the display device + * @param[in] capability the bitset of DisplayCapability value(s) being queried + * @return true if all specified capabilities are available for this device instance + */ + bool (*has_capability)(struct Device* device, uint32_t capability); }; +/** + * @brief Check if this display has 1 or more capabilities. + * @param device the display device + * @param capability the bitset of DisplayCapability value(s) + * @return true if all specified capabilities are available + */ +bool display_has_capability(struct Device* device, uint32_t capability); + /** * @brief Performs a hardware reset of the panel using the specified display. */ diff --git a/TactilityKernel/source/drivers/display.cpp b/TactilityKernel/source/drivers/display.cpp index d0127b53..d8ae083a 100644 --- a/TactilityKernel/source/drivers/display.cpp +++ b/TactilityKernel/source/drivers/display.cpp @@ -6,6 +6,15 @@ extern "C" { +bool display_has_capability(struct Device* device, uint32_t capability) { + const auto* driver = device_get_driver(device); + const auto* api = DISPLAY_DRIVER_API(driver); + if (api->has_capability != nullptr) { + return api->has_capability(device, capability); + } + return (api->capabilities & capability) == capability; +} + error_t display_reset(Device* device) { const auto* driver = device_get_driver(device); return DISPLAY_DRIVER_API(driver)->reset(device); diff --git a/Tests/TactilityKernel/Source/DeviceListenerTest.cpp b/Tests/TactilityKernel/Source/DeviceListenerTest.cpp new file mode 100644 index 00000000..3ce3b14a --- /dev/null +++ b/Tests/TactilityKernel/Source/DeviceListenerTest.cpp @@ -0,0 +1,119 @@ +#include "doctest.h" + +#include + +#include + +// Declared in device_listener.cpp's private header; forward-declared here rather than +// including the private header, matching the pattern used for other internal-only hooks. +extern "C" void device_listener_notify(Device* dev, DeviceEvent event); + +static std::vector> calls_a; +static std::vector> calls_b; + +static void listener_a(Device* dev, DeviceEvent event, void* context) { + calls_a.push_back({ context, event }); +} + +static void listener_b(Device* dev, DeviceEvent event, void* context) { + calls_b.push_back({ context, event }); +} + +static void reset_calls() { + calls_a.clear(); + calls_b.clear(); +} + +TEST_CASE("device_listener_notify invokes every registered listener with its own context") { + reset_calls(); + int context_a = 1; + int context_b = 2; + + device_listener_add(listener_a, &context_a); + device_listener_add(listener_b, &context_b); + + auto* fake_device = reinterpret_cast(0x1000); + device_listener_notify(fake_device, DEVICE_EVENT_STARTED); + + CHECK_EQ(calls_a.size(), 1); + CHECK_EQ(calls_a[0].first, &context_a); + CHECK_EQ(calls_a[0].second, DEVICE_EVENT_STARTED); + + CHECK_EQ(calls_b.size(), 1); + CHECK_EQ(calls_b[0].first, &context_b); + CHECK_EQ(calls_b[0].second, DEVICE_EVENT_STARTED); + + device_listener_remove(listener_a); + device_listener_remove(listener_b); +} + +TEST_CASE("device_listener_remove stops further notifications for that callback only") { + reset_calls(); + int context_a = 1; + int context_b = 2; + + device_listener_add(listener_a, &context_a); + device_listener_add(listener_b, &context_b); + + device_listener_remove(listener_a); + + auto* fake_device = reinterpret_cast(0x1000); + device_listener_notify(fake_device, DEVICE_EVENT_STOPPED); + + CHECK_EQ(calls_a.size(), 0); + CHECK_EQ(calls_b.size(), 1); + + device_listener_remove(listener_b); +} + +TEST_CASE("device_listener_remove on an unregistered callback is a no-op") { + reset_calls(); + int context_b = 2; + device_listener_add(listener_b, &context_b); + + // listener_a was never added, so removing it must not disturb listener_b. + device_listener_remove(listener_a); + + auto* fake_device = reinterpret_cast(0x1000); + device_listener_notify(fake_device, DEVICE_EVENT_STARTED); + + CHECK_EQ(calls_b.size(), 1); + + device_listener_remove(listener_b); +} + +static bool reentrant_add_triggered = false; + +static void reentrant_listener(Device* dev, DeviceEvent event, void* context) { + calls_a.push_back({ context, event }); + if (!reentrant_add_triggered) { + reentrant_add_triggered = true; + // Adding a listener from within a notification must not deadlock: notify() takes a + // snapshot of the listener list under the lock, then invokes callbacks after unlocking. + device_listener_add(listener_b, context); + } +} + +TEST_CASE("device_listener_notify is safe when a listener adds another listener during notification") { + reset_calls(); + reentrant_add_triggered = false; + int context_a = 1; + + device_listener_add(reentrant_listener, &context_a); + + auto* fake_device = reinterpret_cast(0x1000); + device_listener_notify(fake_device, DEVICE_EVENT_STARTED); + + // The listener added during this round of notification was not part of the snapshot, + // so it should not have been invoked yet. + CHECK_EQ(calls_a.size(), 1); + CHECK_EQ(calls_b.size(), 0); + + // A second round picks up the newly-added listener. + device_listener_notify(fake_device, DEVICE_EVENT_STOPPED); + CHECK_EQ(calls_a.size(), 2); + CHECK_EQ(calls_b.size(), 1); + + device_listener_remove(reentrant_listener); + device_listener_remove(listener_b); +} diff --git a/Tests/TactilityKernel/Source/FileSystemTest.cpp b/Tests/TactilityKernel/Source/FileSystemTest.cpp new file mode 100644 index 00000000..ce5cb5df --- /dev/null +++ b/Tests/TactilityKernel/Source/FileSystemTest.cpp @@ -0,0 +1,182 @@ +#include "doctest.h" + +#include +#include +#include + +#include + +static int mount_called = 0; +static int unmount_called = 0; +static bool mounted_state = false; +static error_t mount_result = ERROR_NONE; +static error_t unmount_result = ERROR_NONE; + +static error_t test_mount(void*) { + mount_called++; + mounted_state = true; + return mount_result; +} + +static error_t test_unmount(void*) { + unmount_called++; + mounted_state = false; + return unmount_result; +} + +static bool test_is_mounted(void*) { + return mounted_state; +} + +static error_t test_get_path(void* data, char* out_path, size_t out_path_size) { + const char* path = static_cast(data); + if (std::strlen(path) + 1 > out_path_size) { + return ERROR_BUFFER_OVERFLOW; + } + std::strcpy(out_path, path); + return ERROR_NONE; +} + +static const FileSystemApi test_api = { + .mount = test_mount, + .unmount = test_unmount, + .is_mounted = test_is_mounted, + .get_path = test_get_path +}; + +static void reset_counters() { + mount_called = 0; + unmount_called = 0; + mounted_state = false; + mount_result = ERROR_NONE; + unmount_result = ERROR_NONE; +} + +TEST_CASE("file_system_mount/unmount delegate to the api and reflect is_mounted state") { + reset_counters(); + char path_data[] = "some/path"; + FileSystem* fs = file_system_add(&test_api, path_data); + + CHECK_EQ(file_system_is_mounted(fs), false); + CHECK_EQ(file_system_mount(fs), ERROR_NONE); + CHECK_EQ(mount_called, 1); + CHECK_EQ(file_system_is_mounted(fs), true); + + CHECK_EQ(file_system_unmount(fs), ERROR_NONE); + CHECK_EQ(unmount_called, 1); + CHECK_EQ(file_system_is_mounted(fs), false); + + file_system_remove(fs); +} + +TEST_CASE("file_system_mount propagates api failure without changing state on its own") { + reset_counters(); + mount_result = ERROR_RESOURCE; + char path_data[] = "some/path"; + FileSystem* fs = file_system_add(&test_api, path_data); + + CHECK_EQ(file_system_mount(fs), ERROR_RESOURCE); + // The fake api still flips mounted_state; file_system itself has no independent state, + // it always defers to the api's is_mounted(). + CHECK_EQ(file_system_is_mounted(fs), true); + + mounted_state = false; + file_system_remove(fs); +} + +TEST_CASE("file_system_get_path forwards to the api with the caller's buffer size") { + reset_counters(); + char path_data[] = "mount/point"; + FileSystem* fs = file_system_add(&test_api, path_data); + + char small_buffer[4]; + CHECK_EQ(file_system_get_path(fs, small_buffer, sizeof(small_buffer)), ERROR_BUFFER_OVERFLOW); + + char big_buffer[32]; + CHECK_EQ(file_system_get_path(fs, big_buffer, sizeof(big_buffer)), ERROR_NONE); + CHECK_EQ(std::strcmp(big_buffer, "mount/point"), 0); + + file_system_remove(fs); +} + +TEST_CASE("file_system_set_owner/get_owner round-trip and default to null") { + reset_counters(); + char path_data[] = "some/path"; + FileSystem* fs = file_system_add(&test_api, path_data); + + CHECK_EQ(file_system_get_owner(fs), nullptr); + + auto* fake_owner = reinterpret_cast(0x1234); + file_system_set_owner(fs, fake_owner); + CHECK_EQ(file_system_get_owner(fs), fake_owner); + + file_system_set_owner(fs, nullptr); + CHECK_EQ(file_system_get_owner(fs), nullptr); + + file_system_remove(fs); +} + +TEST_CASE("file_system_for_each visits every registered file system") { + reset_counters(); + char path_a[] = "a"; + char path_b[] = "b"; + char path_c[] = "c"; + FileSystem* fs_a = file_system_add(&test_api, path_a); + FileSystem* fs_b = file_system_add(&test_api, path_b); + FileSystem* fs_c = file_system_add(&test_api, path_c); + + std::vector visited; + file_system_for_each(&visited, [](FileSystem* fs, void* context) { + static_cast*>(context)->push_back(fs); + return true; + }); + + CHECK_EQ(visited.size(), 3); + CHECK(std::find(visited.begin(), visited.end(), fs_a) != visited.end()); + CHECK(std::find(visited.begin(), visited.end(), fs_b) != visited.end()); + CHECK(std::find(visited.begin(), visited.end(), fs_c) != visited.end()); + + file_system_remove(fs_a); + file_system_remove(fs_b); + file_system_remove(fs_c); +} + +TEST_CASE("file_system_for_each stops early when the callback returns false") { + reset_counters(); + char path_a[] = "a"; + char path_b[] = "b"; + FileSystem* fs_a = file_system_add(&test_api, path_a); + FileSystem* fs_b = file_system_add(&test_api, path_b); + + int visit_count = 0; + file_system_for_each(&visit_count, [](FileSystem*, void* context) { + (*static_cast(context))++; + return false; // stop after the first entry + }); + + CHECK_EQ(visit_count, 1); + + file_system_remove(fs_a); + file_system_remove(fs_b); +} + +TEST_CASE("file_system_remove drops the file system from subsequent iteration") { + reset_counters(); + char path_a[] = "a"; + char path_b[] = "b"; + FileSystem* fs_a = file_system_add(&test_api, path_a); + FileSystem* fs_b = file_system_add(&test_api, path_b); + + file_system_remove(fs_a); + + std::vector visited; + file_system_for_each(&visited, [](FileSystem* fs, void* context) { + static_cast*>(context)->push_back(fs); + return true; + }); + + CHECK_EQ(visited.size(), 1); + CHECK_EQ(visited[0], fs_b); + + file_system_remove(fs_b); +} diff --git a/Tests/TactilityKernel/Source/PathsTest.cpp b/Tests/TactilityKernel/Source/PathsTest.cpp new file mode 100644 index 00000000..7c83c70f --- /dev/null +++ b/Tests/TactilityKernel/Source/PathsTest.cpp @@ -0,0 +1,31 @@ +#include "doctest.h" + +#include + +#include + +// The simulator target is never built with ESP_PLATFORM, so paths_get_user_data_path() +// always takes the fixed "data" path branch here, guarded by a buffer-size check. + +TEST_CASE("paths_get_user_data_path succeeds when the buffer exactly fits") { + char buffer[16] = { 0 }; + CHECK_EQ(paths_get_user_data_path(buffer, sizeof(buffer)), ERROR_NONE); + CHECK_EQ(std::strcmp(buffer, "data"), 0); +} + +TEST_CASE("paths_get_user_data_path succeeds with a buffer sized to exactly fit the string and terminator") { + char buffer[5] = { 0 }; // strlen("data") + 1 + CHECK_EQ(paths_get_user_data_path(buffer, sizeof(buffer)), ERROR_NONE); + CHECK_EQ(std::strcmp(buffer, "data"), 0); +} + +TEST_CASE("paths_get_user_data_path reports a buffer overflow when the buffer is one byte too small") { + char buffer[4] = { 0 }; // strlen("data"), no room for the terminator + CHECK_EQ(paths_get_user_data_path(buffer, sizeof(buffer)), ERROR_BUFFER_OVERFLOW); +} + +TEST_CASE("paths_get_user_data_path reports a buffer overflow for a zero-size buffer") { + char buffer[1] = { 'x' }; + CHECK_EQ(paths_get_user_data_path(buffer, 0), ERROR_BUFFER_OVERFLOW); + CHECK_EQ(buffer[0], 'x'); // untouched +} diff --git a/device.py b/device.py index c3aa1f23..2b5ec4d6 100644 --- a/device.py +++ b/device.py @@ -450,10 +450,25 @@ def clean_build_dirs_on_platform_change(previous_target: str, new_target: str): print(f" {d}") shutil.rmtree(d) +def list_device_ids(): + return sorted( + name for name in os.listdir(DEVICES_DIRECTORY) + if os.path.isfile(get_properties_file_path(name)) + ) + +def resolve_device_id(device_id: str): + device_ids = list_device_ids() + if device_id in device_ids: + return device_id + matches = [d for d in device_ids if device_id in d] + if len(matches) == 1: + return matches[0] + if len(matches) > 1: + exit_with_error(f"Ambiguous device identifier '{device_id}', matches: {', '.join(matches)}") + exit_with_error(f"{device_id} is not a valid device identifier (no exact or partial match found in {DEVICES_DIRECTORY}/)") + def main(device_id: str, is_dev: bool): - device_properties_path = get_properties_file_path(device_id) - if not os.path.isfile(device_properties_path): - exit_with_error(f"{device_id} is not a valid device identifier (could not found {device_properties_path})") + device_id = resolve_device_id(device_id) output_file_path = "sdkconfig" # Clean build dirs if target changes device_properties = read_device_properties(device_id)