[PATCH 00/22] expo: Expand docs, dump and textlines in non-popup expos
From: Simon Glass <simon.glass@canonical.com> So far textlines are mostly used in cedits as a way to enter textual information. For non-popup expos, textlines are not yet fully plumbed in. This series adds a way to send keypresses to a highlighted textline, adds a test for this case and fixes various minor issues to make this all work. One noteable change is renumbering the BKEY enum. At present the values conflict with the control keys used by CLI processing, so for example, expo is unable to distinguish an up-arrow from a backspace. Tests which use textedits mostly need to run with the console active, since a silent console suppresses output of the text in the textedit. In fact, at present cedit_render_lineedit() does not work unless the previous test ran first. A new UTF_NO_SILENT test flag is added to make this problem easier to discover/debug. This series also resolves an issue where the 'cedit dump' is never enabled due to a typo in the Kconfig item. With that fixed, the dump format is converted to use hex (U-Boot convention). The expo menu and cedit implementations are updated to use better names for objects. This series also includes some documentation updates, since much of the debugging methods used are not explicitly described. This should make it easier for others to make improvements. With all of this complete, it is possible to have a password field in a menu item and to enter text into it, even with a non-popup expo. It also becomes easier to debug such issues in future. Simon Glass (22): CLAUDE.md: Update build and coding conventions cmd: cedit: Fix CONFIG_CMD_EDIT_DUMP typo expo: Use textline consistently in tests video: Document the quirk in truetype_get_cursor_info() test: Add a flag for test which need console output doc: expo: Move test-mode docs up a little doc: test: Document assertion macros doc: expo: Add documentation for writing and debugging tests expo: Use hex format when dumping the expo expo: Add prompt_id to struct scene expo: Use better names for child objects in expo_build expo: Use better names for objects in bootflow_menu_add() expo: Fix textline edit text not updating in bootflow menu test: cedit: Use UTF_NO_SILENT test: cedit: Allow cedit_render_lineedit() to run alone expo: Refactor scene_send_key() to use a current object expo: Add scene_render_obj() to render by object ID expo: Correct rendering of textlines when open menu: Start bootmenu_key values at 0x100 expo: Always send keys to highlighted textline expo: Consider an item selected when a password is entered test: expo: Add a test for textline rendering in an expo CLAUDE.md | 7 +- boot/bootflow_menu.c | 44 +++++-- boot/cedit.c | 11 +- boot/expo_build.c | 52 +++++--- boot/expo_dump.c | 51 ++++---- boot/scene.c | 63 +++++---- boot/scene_internal.h | 9 ++ boot/scene_textline.c | 29 ++--- cmd/cedit.c | 8 +- doc/arch/sandbox/sandbox.rst | 5 +- doc/develop/expo.rst | 211 +++++++++++++++++++++++++++++-- doc/develop/tests_writing.rst | 106 ++++++++++++++++ drivers/video/console_truetype.c | 6 +- include/expo.h | 2 + include/menu.h | 10 +- include/test/test.h | 17 +-- test/boot/cedit.c | 18 +-- test/boot/expo.c | 120 +++++++++++++++++- test/test-main.c | 3 +- 19 files changed, 630 insertions(+), 142 deletions(-) -- 2.43.0 base-commit: c4c9d1231df690a8ff716535703232987db77524 branch: sech
From: Simon Glass <simon.glass@canonical.com> Fix typo ("now" -> "no") and clarify notes: - Add reminder to avoid in-tree builds - Clarify Co-developed-by usage in commits Signed-off-by: Simon Glass <simon.glass@canonical.com> --- CLAUDE.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a13dd995b68..ca0571ecef8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,7 +59,9 @@ pyt <test_name> - Always run `make mrproper` if you encounter build issues - The sandbox build creates a test environment for U-Boot that runs on the host system - When using `git diff`, add `--no-ext-diff` to avoid external diff tools that may not work in this environment -- crosfw shows now output if everything was ok! +- crosfw shows no output if everything was ok! +- Remember not to cd into the build directory; run U-Boot directly in the source dir +- Do not run in-tree builds; always use the crosfw script or 'make O=/tmp/...' ## Coding Conventions @@ -69,5 +71,4 @@ pyt <test_name> - This follows U-Boot's established naming convention for output parameters - Keep commit messages concise - focus on the key change and essential details only - Code should be formatted to 80 columns and not have trailing spaces -- Remember to use Co-developed-by in commits -- Remember not to cd into the build directory; run U-Boot directly in the source dir +- Remember to use Co-developed-by instead of Co-Authored-By in commits -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The cedit dump command uses CONFIG_CMD_EDIT_DUMP but the Kconfig defines CONFIG_CMD_CEDIT_DUMP. Fix the code to use the correct name. With this, the command is actually enabled on sandbox. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/cedit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/cedit.c b/cmd/cedit.c index e446f61b3ba..fc477291be8 100644 --- a/cmd/cedit.c +++ b/cmd/cedit.c @@ -292,7 +292,7 @@ static int do_cedit_run(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } -#ifdef CONFIG_CMD_EDIT_DUMP +#ifdef CONFIG_CMD_CEDIT_DUMP static int do_cedit_dump(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -321,7 +321,7 @@ static int do_cedit_dump(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } -#endif /* CONFIG_CMD_EDIT_DUMP */ +#endif /* CONFIG_CMD_CEDIT_DUMP */ U_BOOT_LONGHELP(cedit, "load <interface> <dev[:part]> <filename> - load config editor\n" @@ -334,7 +334,7 @@ U_BOOT_LONGHELP(cedit, "cedit write_env [-v] - write settings to env vars\n" "cedit read_cmos [-v] [dev] - read settings from CMOS RAM\n" "cedit write_cmos [-v] [dev] - write settings to CMOS RAM\n" -#ifdef CONFIG_CMD_EDIT_DUMP +#ifdef CONFIG_CMD_CEDIT_DUMP "cedit dump - dump expo structure\n" #endif "cedit run - run config editor"); @@ -350,7 +350,7 @@ U_BOOT_CMD_WITH_SUBCMDS(cedit, "Configuration editor", cedit_help_text, U_BOOT_SUBCMD_MKENT(write_env, 2, 1, do_cedit_write_env), U_BOOT_SUBCMD_MKENT(read_cmos, 2, 1, do_cedit_read_cmos), U_BOOT_SUBCMD_MKENT(write_cmos, 2, 1, do_cedit_write_cmos), -#ifdef CONFIG_CMD_EDIT_DUMP +#ifdef CONFIG_CMD_CEDIT_DUMP U_BOOT_SUBCMD_MKENT(dump, 1, 1, do_cedit_dump), #endif U_BOOT_SUBCMD_MKENT(run, 1, 1, do_cedit_run), -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The word 'lineedit' has crept into one of the tests, but it is not correct. Use 'textline' instead. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/cedit.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/boot/cedit.c b/test/boot/cedit.c index 041da445459..7af3dfa2338 100644 --- a/test/boot/cedit.c +++ b/test/boot/cedit.c @@ -348,10 +348,10 @@ static int cedit_render(struct unit_test_state *uts) /* * Send some keypresses. Note that the console must be enabled so that * the characters actually reach the putc_xy() in console_truetype, - * since in scene_textline_send_key(), the lineedit restores the + * since in scene_textline_send_key(), the textline restores the * vidconsole state, outputs the character and then saves the state * again. If the character is never output, then the state won't be - * updated and the lineedit will be inconsistent. + * updated and the textline will be inconsistent. */ ut_unsilence_console(uts); for (i = 'a'; i < 'd'; i++) @@ -368,8 +368,8 @@ static int cedit_render(struct unit_test_state *uts) } BOOTSTD_TEST(cedit_render, UTF_DM | UTF_SCAN_FDT); -/* Check the cedit displays lineedits correctly */ -static int cedit_render_lineedit(struct unit_test_state *uts) +/* Check the cedit displays textlines correctly */ +static int cedit_render_textline(struct unit_test_state *uts) { struct scene_obj_textline *tline; struct video_priv *vid_priv; @@ -446,7 +446,7 @@ static int cedit_render_lineedit(struct unit_test_state *uts) return 0; } -BOOTSTD_TEST(cedit_render_lineedit, UTF_DM | UTF_SCAN_FDT); +BOOTSTD_TEST(cedit_render_textline, UTF_DM | UTF_SCAN_FDT); /* Check the cedit is arranged correctly */ static int cedit_position(struct unit_test_state *uts) @@ -654,7 +654,7 @@ static int cedit_mouse(struct unit_test_state *uts) ut_asserteq(SCENEOF_OPEN | SCENEOF_SIZE_VALID | SCENEOF_DIRTY, speed->obj.flags); - /* click on the lineedit */ + /* click on the textline */ ut_assertok(click_check(uts, scn, mach->edit_id, EXPOACT_REPOINT_OPEN, &act)); ut_asserteq(ID_MACHINE_NAME, act.select.id); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The current code is written to suggest that the cursor position (priv->pos_ptr) might be respected. In fact a bug means that it never is, since index is set to priv->pos_ptr, so the if() condition always fails. Further, when editing textline objects, the cursor index is always the same as priv->pos_ptr anyway. Fix the bug but keep the current behaviour, adding a comment for future readers. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- drivers/video/console_truetype.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index 4ddf133f2e0..7f5a2262b17 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -1097,9 +1097,13 @@ static int truetype_get_cursor_info(struct udevice *dev) * figure out where to place the cursor. This driver ignores the * passed-in values, since an entry_restore() must have been done before * calling this function. + * + * A current quirk is that the cursor is always at xcur_frac, since we + * output characters directly to the console as they are typed by the + * user. So we never bother with priv->pos[index] for now. */ index = priv->pos_ptr; - if (index < priv->pos_ptr) + if (0 && index < priv->pos_count) x = VID_TO_PIXEL(priv->pos[index].xpos_frac); else x = VID_TO_PIXEL(vc_priv->xcur_frac); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Some tests cannot run when the console is silent. An example is an expo test which checks text entry into a textline object. The console must be enabled so that the characters actually reach the putc_xy() in console_truetype, since in scene_textline_send_key(), the lineedit restores the vidconsole state, outputs the character and then saves the state again. If the character is never output, then the state won't be updated and the lineedit will be inconsistent. Rather than having individual tests handle this manually, add an explicit flag, in the hope that this quirk does not trip anyone else up. Put the flag next to the existing UTF_CONSOLE flag, since they are related. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/test/test.h | 17 +++++++++-------- test/test-main.c | 3 ++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/test/test.h b/include/test/test.h index f2d956e913c..086fff1ca26 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -90,20 +90,21 @@ enum ut_flags { UTF_FLAT_TREE = BIT(3), /* test needs flat DT */ UTF_LIVE_TREE = BIT(4), /* needs live device tree */ UTF_CONSOLE = BIT(5), /* needs console recording */ + UTF_NO_SILENT = BIT(6), /* console cannot be silent */ /* do extra driver model init and uninit */ - UTF_DM = BIT(6), - UTF_OTHER_FDT = BIT(7), /* read in other device tree */ + UTF_DM = BIT(7), + UTF_OTHER_FDT = BIT(8), /* read in other device tree */ /* * Only run if explicitly requested with 'ut -f <suite> <test>'. The * test name must end in "_norun" so that pytest detects this also, * since it cannot access the flags. */ - UTF_MANUAL = BIT(8), - UTF_ETH_BOOTDEV = BIT(9), /* enable Ethernet bootdevs */ - UTF_SF_BOOTDEV = BIT(10), /* enable SPI flash bootdevs */ - UFT_BLOBLIST = BIT(11), /* test changes gd->bloblist */ - UTF_INIT = BIT(12), /* test inits a suite */ - UTF_UNINIT = BIT(13), /* test uninits a suite */ + UTF_MANUAL = BIT(9), + UTF_ETH_BOOTDEV = BIT(10), /* enable Ethernet bootdevs */ + UTF_SF_BOOTDEV = BIT(11), /* enable SPI flash bootdevs */ + UFT_BLOBLIST = BIT(12), /* test changes gd->bloblist */ + UTF_INIT = BIT(13), /* test inits a suite */ + UTF_UNINIT = BIT(14), /* test uninits a suite */ }; /** diff --git a/test/test-main.c b/test/test-main.c index b27f892140c..941b883e156 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -410,7 +410,8 @@ static int test_pre_run(struct unit_test_state *uts, struct unit_test *test) gd_set_bloblist(NULL); } - ut_silence_console(uts); + if (!(test->flags & UTF_NO_SILENT)) + ut_silence_console(uts); return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Move the documentation about test mode up above the API documentation, which is quite lengthly. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/expo.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst index 9ce00e621e5..84d01b6313b 100644 --- a/doc/develop/expo.rst +++ b/doc/develop/expo.rst @@ -569,14 +569,6 @@ strings are provided inline in the nodes where they are used. }; -API documentation ------------------ - -.. kernel-doc:: include/expo.h - -Future ideas ------------- - Test Mode ~~~~~~~~~ @@ -613,6 +605,14 @@ These metrics help identify performance bottlenecks and verify that expo is operating efficiently. The timing information is particularly useful when optimizing display drivers or debugging slow rendering issues. +API documentation +----------------- + +.. kernel-doc:: include/expo.h + +Future ideas +------------ + Some ideas for future work: - Default menu item and a timeout -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add documentation for all the ut_assert*() macros and memory helpers available in include/test/ut.h. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/tests_writing.rst | 104 ++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/doc/develop/tests_writing.rst b/doc/develop/tests_writing.rst index f6f852c297d..4b0616376d0 100644 --- a/doc/develop/tests_writing.rst +++ b/doc/develop/tests_writing.rst @@ -373,6 +373,110 @@ existing suite or creating a new one. An example SPL test is spl_test_load(). +Assertions +---------- + +The test framework provides various assertion macros, defined in +``include/test/ut.h``. All of these return from the test function on failure, +unless ``uts->soft_fail`` is set. + +Basic assertions +~~~~~~~~~~~~~~~~ + +ut_assert(cond) + Assert that a condition is non-zero (true) + +ut_assertf(cond, fmt, args...) + Assert that a condition is non-zero, with a printf() message on failure + +ut_assertok(cond) + Assert that an operation succeeds (returns 0) + +ut_reportf(fmt, args...) + Report a failure with a printf() message (always fails) + +Value comparisons +~~~~~~~~~~~~~~~~~ + +ut_asserteq(expr1, expr2) + Assert that two int expressions are equal + +ut_asserteq_64(expr1, expr2) + Assert that two 64-bit expressions are equal + +ut_asserteq_str(expr1, expr2) + Assert that two strings are equal + +ut_asserteq_strn(expr1, expr2) + Assert that two strings are equal, up to the length of the first + +ut_asserteq_mem(expr1, expr2, len) + Assert that two memory areas are equal + +ut_asserteq_ptr(expr1, expr2) + Assert that two pointers are equal + +ut_asserteq_addr(expr1, expr2) + Assert that two addresses (converted from pointers via map_to_sysmem()) + are equal + +Pointer assertions +~~~~~~~~~~~~~~~~~~ + +ut_assertnull(expr) + Assert that a pointer is NULL + +ut_assertnonnull(expr) + Assert that a pointer is not NULL + +ut_assertok_ptr(expr) + Assert that a pointer is not an error pointer (checked with IS_ERR()) + +Console output assertions +~~~~~~~~~~~~~~~~~~~~~~~~~ + +These are used to check console output when ``UTF_CONSOLE`` flag is set. + +ut_assert_nextline(fmt, args...) + Assert that the next console output line matches the format string + +ut_assert_nextlinen(fmt, args...) + Assert that the next console output line matches up to the format + string length + +ut_assert_nextline_empty() + Assert that the next console output line is empty + +ut_assert_skipline() + Assert that there is a next console output line, and skip it + +ut_assert_skip_to_line(fmt, args...) + Skip console output until a matching line is found + +ut_assert_skip_to_linen(fmt, args...) + Skip console output until a partial match is found (compares up to the + format-string length) + +ut_assert_console_end() + Assert that there is no more console output + +ut_assert_nextlines_are_dump(total_bytes) + Assert that the next lines are a print_buffer() hex dump of the specified + size + +Memory helpers +~~~~~~~~~~~~~~ + +These help check for memory leaks: + +ut_check_free() + Return the number of bytes free in the malloc() pool + +ut_check_delta(last) + Return the change in free memory since ``last`` was obtained from + ``ut_check_free()``. A positive value means more memory has been allocated. + + Writing Python tests -------------------- -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new section covering how to write expo tests, including test structure, memory checking, creating test expos, testing rendering and input, building from devicetree, and using IDs. Also add a debugging section with sandbox command-line options useful for expo development. Add a bit more detail for --video_frames Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/arch/sandbox/sandbox.rst | 5 +- doc/develop/expo.rst | 195 ++++++++++++++++++++++++++++++++++ doc/develop/tests_writing.rst | 2 + 3 files changed, 201 insertions(+), 1 deletion(-) diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst index 0d94c5a49cf..b2f4d8913d2 100644 --- a/doc/arch/sandbox/sandbox.rst +++ b/doc/arch/sandbox/sandbox.rst @@ -253,7 +253,10 @@ available options. Some of these are described below: Show console output from tests. Normally this is suppressed. --video_frames <dir> - Write video frames to the specified directory for debugging purposes. + Write video frames to the specified directory for debugging purposes. Each + time video_compress_fb() is called, this writes a file called 'frame%d.bmp' + to the given directory where %d is the sequence number starting from 0. Note + that sandbox removes all 'frame%d.bmp' files in that directory on startup. -V, --video_test <ms> Enable video test mode with a delay (in milliseconds) between assertions. This diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst index 84d01b6313b..ca83b403621 100644 --- a/doc/develop/expo.rst +++ b/doc/develop/expo.rst @@ -605,6 +605,201 @@ These metrics help identify performance bottlenecks and verify that expo is operating efficiently. The timing information is particularly useful when optimizing display drivers or debugging slow rendering issues. +Writing expo tests +------------------ + +Expo has extensive tests in ``test/boot/expo.c`` and ``test/boot/cedit.c``. +These can be run under sandbox like any other test (see :doc:`testing`). + +Test structure +~~~~~~~~~~~~~~ + +Each test function follows a standard pattern:: + + static int expo_my_test(struct unit_test_state *uts) + { + struct expo *exp; + + /* Create expo and perform tests */ + + ut_assertok(expo_new("test", NULL, &exp)); + + /* ... test code ... */ + + expo_destroy(exp); + + return 0; + } + BOOTSTD_TEST(expo_my_test, UTF_DM | UTF_SCAN_FDT); + +The ``BOOTSTD_TEST()`` macro registers the test with the test framework. +Common flags include: + +UTF_DM + Requires driver model to be enabled (most expo tests need this) + +UTF_SCAN_FDT + Scans the device tree for devices (needed for video display) + +UTF_CONSOLE + Test needs to record console output (needed for commands) + +UTF_NO_SILENT + Don't silence console output (needed for tests that check rendering output + with user input) + +Memory checking +~~~~~~~~~~~~~~~ + +Tests should verify that no memory is leaked:: + + ulong start_mem; + + start_mem = ut_check_free(); + + /* ... create expo, test, destroy ... */ + + ut_assertok(ut_check_delta(start_mem)); + +For assertions, see :ref:`tests_writing_assertions`. + +Creating test expos +~~~~~~~~~~~~~~~~~~~ + +A common pattern is to create a helper function that sets up an expo with +scenes and objects for testing. See ``create_test_expo()`` in +``test/boot/expo.c`` for an example:: + + static int create_test_expo(struct unit_test_state *uts, struct expo **expp, + struct scene **scnp, struct scene_obj_menu **menup, + ...) + { + struct expo *exp; + struct scene *scn; + int id; + + ut_assertok(uclass_first_device_err(UCLASS_VIDEO, &dev)); + ut_assertok(expo_new(EXPO_NAME, NULL, &exp)); + + id = scene_new(exp, SCENE_NAME1, SCENE1, &scn); + ut_assert(id > 0); + + ut_assertok(expo_set_display(exp, dev)); + + /* Add objects to the scene */ + id = scene_txt_str(scn, "text", OBJ_TEXT, STR_TEXT, "my string", NULL); + ut_assert(id > 0); + + /* Return pointers */ + *expp = exp; + *scnp = scn; + + return 0; + } + +Testing rendering +~~~~~~~~~~~~~~~~~ + +For graphical rendering tests, use ``video_compress_fb()`` to get a checksum +of the framebuffer:: + + ut_assertok(expo_render(exp)); + ut_asserteq(expected_checksum, video_compress_fb(uts, dev, false)); + +For text-mode rendering, check console output lines:: + + expo_set_text_mode(exp, true); + ut_assertok(expo_render(exp)); + ut_assert_nextline("Expected line"); + ut_assert_console_end(); + +Testing input +~~~~~~~~~~~~~ + +To test keyboard input handling, use ``expo_send_key()``:: + + ut_assertok(expo_send_key(exp, BKEY_DOWN)); + ut_assertok(expo_action_get(exp, &act)); + ut_asserteq(EXPOACT_POINT_ITEM, act.type); + ut_asserteq(ITEM2, act.select.id); + +To test mouse clicks, use ``scene_send_click()``:: + + ut_assertok(scene_send_click(scn, x, y, &act)); + ut_asserteq(EXPOACT_SELECT, act.type); + +Building from devicetree +~~~~~~~~~~~~~~~~~~~~~~~~ + +To test building an expo from a devicetree description:: + + ofnode node; + + node = ofnode_path("/cedit"); + ut_assert(ofnode_valid(node)); + ut_assertok(expo_build(node, &exp)); + +The test devicetree is in ``test/boot/files/expo_layout.dts`` with IDs +defined in ``test/boot/files/expo_ids.h``. See ``setup_cedit_file()`` in +``test/py/img/cedit.py`` for how this is set up. + +Using IDs +~~~~~~~~~ + +Define an enum for all object IDs at the top of the test file:: + + enum { + /* scenes */ + SCENE1 = 7, + + /* objects */ + OBJ_LOGO, + OBJ_TEXT, + OBJ_MENU, + + /* strings */ + STR_TEXT, + STR_MENU_TITLE, + + /* menu items */ + ITEM1, + ITEM2, + }; + +Starting IDs from a value higher than ``EXPOID_BASE_ID`` avoids conflicts +with reserved expo IDs. + +Debugging tests +~~~~~~~~~~~~~~~ + +Running tests directly (without pytest) makes debugging easier. See +:doc:`tests_sandbox` for details on running sandbox tests with gdb. + +For example, to run a single expo test:: + + ./u-boot -T -c "ut bootstd expo_render_image" + +To debug with gdb:: + + gdb --args ./u-boot -T -c "ut bootstd expo_render_image" + (gdb) break expo_render_image + (gdb) run + +IDEs such as Visual Studio Code can also be used. + +Sandbox provides command-line options useful for debugging expo and video +tests, including ``-l`` (show LCD), ``-K`` (double LCD size), ``-V`` (video +test mode with delay), ``--video_frames`` (capture frames), ``-f`` (continue +after failure), and ``-F`` (skip flat-tree tests). See +:doc:`../arch/sandbox/sandbox` for full details. + +For example, to watch an expo test render with a visible display:: + + ./u-boot -T -l -V 500 --video_frames /tmp/good -c "ut bootstd expo_render_image" + +This will write each asserted expo frame to ``/tmp/good/frame0.bmp``, +``/tmp/good/frame1.bmp``, etc. + API documentation ----------------- diff --git a/doc/develop/tests_writing.rst b/doc/develop/tests_writing.rst index 4b0616376d0..31454aa4819 100644 --- a/doc/develop/tests_writing.rst +++ b/doc/develop/tests_writing.rst @@ -373,6 +373,8 @@ existing suite or creating a new one. An example SPL test is spl_test_load(). +.. _tests_writing_assertions: + Assertions ---------- -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Change expo_dump() to output object IDs in hex format. This matches the normal convention in U-Boot. Also add key_id and preview_id to the menu item dump output while we are here. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/expo_dump.c | 51 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/boot/expo_dump.c b/boot/expo_dump.c index 711b26eb220..eb0c7bce6fc 100644 --- a/boot/expo_dump.c +++ b/boot/expo_dump.c @@ -67,14 +67,15 @@ static void dump_menu(struct dump_ctx *ctx, struct scene_obj_menu *menu) struct scene_obj *obj = &menu->obj; struct scene_menitem *item; - outf(ctx, "Menu: pointer_id %d title_id %d manual %d\n", + outf(ctx, "Menu: pointer_id %x title_id %x manual %d\n", menu->pointer_id, menu->title_id, !!(obj->flags & SCENEOF_MANUAL)); ctx->indent += 2; list_for_each_entry(item, &menu->item_head, sibling) { - outf(ctx, "Item %d: name '%s' label_id %d desc_id %d\n", - item->id, item->name, item->label_id, item->desc_id); + outf(ctx, "Item %x: name '%s' key_id %x label_id %x desc_id %x preview_id %x\n", + item->id, item->name, item->key_id, item->label_id, + item->desc_id, item->preview_id); } ctx->indent -= 2; } @@ -83,7 +84,7 @@ static void dump_text(struct dump_ctx *ctx, struct scene_obj_txt *txt) { const char *str = expo_get_str(ctx->scn->expo, txt->gen.str_id); - outf(ctx, "Text: str_id %d font_name '%s' font_size %d\n", + outf(ctx, "Text: str_id %x font_name '%s' font_size %x\n", txt->gen.str_id, txt->gen.font_name ? txt->gen.font_name : "(default)", txt->gen.font_size); @@ -94,7 +95,7 @@ static void dump_text(struct dump_ctx *ctx, struct scene_obj_txt *txt) static void dump_box(struct dump_ctx *ctx, struct scene_obj_box *box) { - outf(ctx, "Box: fill %d width %d\n", box->fill, box->width); + outf(ctx, "Box: fill %d width %x\n", box->fill, box->width); } static void dump_image(struct dump_ctx *ctx, struct scene_obj_img *img) @@ -105,17 +106,17 @@ static void dump_image(struct dump_ctx *ctx, struct scene_obj_img *img) static void dump_textline(struct dump_ctx *ctx, struct scene_obj_textline *tline) { - outf(ctx, "Textline: label_id %d edit_id %d\n", + outf(ctx, "Textline: label_id %x edit_id %x\n", tline->label_id, tline->edit_id); ctx->indent += 2; - outf(ctx, "max_chars %d pos %d\n", tline->max_chars, tline->pos); + outf(ctx, "max_chars %x pos %x\n", tline->max_chars, tline->pos); ctx->indent -= 2; } static void dump_textedit(struct dump_ctx *ctx, struct scene_obj_txtedit *tedit) { - outf(ctx, "Textedit: str_id %d font_name '%s' font_size %d\n", + outf(ctx, "Textedit: str_id %x font_name '%s' font_size %x\n", tedit->gen.str_id, tedit->gen.font_name ? tedit->gen.font_name : "(default)", tedit->gen.font_size); @@ -128,7 +129,7 @@ static void obj_dump_(struct dump_ctx *ctx, struct scene_obj *obj) int bit; int pos = 0; - outf(ctx, "Object %d (%s): type %s\n", obj->id, obj->name, + outf(ctx, "Object %x (%s): type %s\n", obj->id, obj->name, scene_obj_type_name(obj->type)); ctx->indent += 2; @@ -144,9 +145,9 @@ static void obj_dump_(struct dump_ctx *ctx, struct scene_obj *obj) } } outf(ctx, "flags %s\n", pos > 0 ? flags_buf : ""); - outf(ctx, "bbox: (%d,%d)-(%d,%d)\n", + outf(ctx, "bbox: (%x,%x)-(%x,%x)\n", obj->bbox.x0, obj->bbox.y0, obj->bbox.x1, obj->bbox.y1); - outf(ctx, "dims: %dx%d\n", obj->dims.x, obj->dims.y); + outf(ctx, "dims: %xx%x\n", obj->dims.x, obj->dims.y); switch (obj->type) { case SCENEOBJT_NONE: @@ -177,11 +178,11 @@ static void scene_dump_(struct dump_ctx *ctx) { struct scene_obj *obj; - outf(ctx, "Scene %d: name '%s'\n", ctx->scn->id, ctx->scn->name); + outf(ctx, "Scene %x: name '%s'\n", ctx->scn->id, ctx->scn->name); ctx->indent += 2; - outf(ctx, "title_id %d (%s)\n", + outf(ctx, "title_id %x (%s)\n", ctx->scn->title_id, obj_name(ctx, ctx->scn->title_id)); - outf(ctx, "highlight_id %d (%s)\n", + outf(ctx, "highlight_id %x (%s)\n", ctx->scn->highlight_id, obj_name(ctx, ctx->scn->highlight_id)); list_for_each_entry(obj, &ctx->scn->obj_head, sibling) { @@ -215,20 +216,20 @@ static void expo_dump_(struct dump_ctx *ctx, struct expo *exp) exp->display ? exp->display->name : "(null)"); outf(ctx, "cons %s\n", exp->cons ? exp->cons->name : "(none)"); outf(ctx, "mouse %s\n", exp->mouse ? exp->mouse->name : "(none)"); - outf(ctx, "scene_id %d\n", exp->scene_id); - outf(ctx, "next_id %d\n", exp->next_id); - outf(ctx, "req_width %d\n", exp->req_width); - outf(ctx, "req_height %d\n", exp->req_height); + outf(ctx, "scene_id %x\n", exp->scene_id); + outf(ctx, "next_id %x\n", exp->next_id); + outf(ctx, "req_width %x\n", exp->req_width); + outf(ctx, "req_height %x\n", exp->req_height); outf(ctx, "text_mode %d\n", exp->text_mode); outf(ctx, "popup %d\n", exp->popup); outf(ctx, "show_highlight %d\n", exp->show_highlight); outf(ctx, "mouse_enabled %d\n", exp->mouse_enabled); outf(ctx, "mouse_ptr %p\n", exp->mouse_ptr); - outf(ctx, "mouse_size %dx%d\n", exp->mouse_size.w, + outf(ctx, "mouse_size %xx%x\n", exp->mouse_size.w, exp->mouse_size.h); - outf(ctx, "mouse_pos (%d,%d)\n", exp->mouse_pos.x, + outf(ctx, "mouse_pos (%x,%x)\n", exp->mouse_pos.x, exp->mouse_pos.y); - outf(ctx, "damage (%d,%d)-(%d,%d)\n", exp->damage.x0, exp->damage.y0, + outf(ctx, "damage (%x,%x)-(%x,%x)\n", exp->damage.x0, exp->damage.y0, exp->damage.x1, exp->damage.y1); outf(ctx, "done %d\n", exp->done); outf(ctx, "save %d\n", exp->save); @@ -237,17 +238,17 @@ static void expo_dump_(struct dump_ctx *ctx, struct expo *exp) if (exp->display) { struct video_priv *vid_priv = dev_get_uclass_priv(exp->display); - outf(ctx, "video: %dx%d white_on_black %d\n", + outf(ctx, "video: %xx%x white_on_black %d\n", vid_priv->xsize, vid_priv->ysize, vid_priv->white_on_black); } outf(ctx, "Theme:\n"); ctx->indent = 4; - outf(ctx, "font_size %d\n", theme->font_size); + outf(ctx, "font_size %x\n", theme->font_size); outf(ctx, "white_on_black %d\n", theme->white_on_black); - outf(ctx, "menu_inset %d\n", theme->menu_inset); - outf(ctx, "menuitem_gap_y %d\n", theme->menuitem_gap_y); + outf(ctx, "menu_inset %x\n", theme->menu_inset); + outf(ctx, "menuitem_gap_y %x\n", theme->menuitem_gap_y); ctx->indent = 0; outf(ctx, "\nScenes:\n"); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a prompt_id field to struct scene to track the prompt text object. Update cedit_arange() to use title_id and prompt_id directly instead of looking up objects by name, which is more robust. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/cedit.c | 11 ++++------- include/expo.h | 2 ++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/boot/cedit.c b/boot/cedit.c index c82519a0eb3..d58ae0ba51c 100644 --- a/boot/cedit.c +++ b/boot/cedit.c @@ -54,7 +54,6 @@ int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id) { struct expo_theme *theme = &exp->theme; struct expo_arrange_info arr; - struct scene_obj_txt *txt; struct scene_obj *obj; struct scene *scn; int y, ret; @@ -63,13 +62,11 @@ int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id) if (!scn) return log_msg_ret("scn", -ENOENT); - txt = scene_obj_find_by_name(scn, "prompt"); - if (txt) - scene_obj_set_pos(scn, txt->obj.id, 0, vpriv->ysize - 50); + if (scn->prompt_id) + scene_obj_set_pos(scn, scn->prompt_id, 0, vpriv->ysize - 50); - txt = scene_obj_find_by_name(scn, "title"); - if (txt) - scene_obj_set_pos(scn, txt->obj.id, 200, 10); + if (scn->title_id) + scene_obj_set_pos(scn, scn->title_id, 200, 10); memset(&arr, '\0', sizeof(arr)); ret = scene_calc_arrange(scn, &arr); diff --git a/include/expo.h b/include/expo.h index e6093769421..f9f85b38b9c 100644 --- a/include/expo.h +++ b/include/expo.h @@ -204,6 +204,7 @@ struct expo_string { * @name: Name of the scene (allocated) * @id: ID number of the scene * @title_id: String ID of title of the scene (allocated) + * @prompt_id: String ID of prompt of the scene (allocated) * @highlight_id: ID of highlighted object, if any * @cls: cread state to use for input * @buf: Buffer for input @@ -216,6 +217,7 @@ struct scene { char *name; uint id; uint title_id; + uint prompt_id; uint highlight_id; struct cli_line_state cls; struct abuf buf; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Use the parent object name as a prefix for child objects, so they have descriptive names like "cpu-speed.title" instead of generic "title". This makes debugging easier when multiple objects exist. Update add_txt_str() and add_txt_str_list() to take separate property name and object name parameters. Also set the scene's prompt_id when building from devicetree. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/expo_build.c | 52 +++++++++++++++++++++++++++++++---------------- test/boot/expo.c | 4 ++-- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/boot/expo_build.c b/boot/expo_build.c index f8ae5bcbbf7..60a9cd71b7e 100644 --- a/boot/expo_build.c +++ b/boot/expo_build.c @@ -38,12 +38,13 @@ struct build_info { * @info: Build information * @node: Node describing scene * @scn: Scene to add to - * @find_name: Name to look for (e.g. "title"). This will find a property called - * "title" if it exists, else will look up the string for "title-id" + * @find_name: Property to look for (e.g. "title"). This will find a property + * called "title" if it exists, else will look up the string for "title-id" + * @obj_name: Name for the object (e.g. "cpu-speed.title") * Return: ID of added string, or -ve on error */ int add_txt_str(struct build_info *info, ofnode node, struct scene *scn, - const char *find_name, uint obj_id) + const char *find_name, const char *obj_name, uint obj_id) { const char *text; int ret; @@ -66,7 +67,7 @@ int add_txt_str(struct build_info *info, ofnode node, struct scene *scn, return log_msg_ret("id", -EINVAL); } - ret = scene_txt_str(scn, find_name, obj_id, 0, text, NULL); + ret = scene_txt_str(scn, obj_name, obj_id, 0, text, NULL); if (ret < 0) return log_msg_ret("add", ret); @@ -79,13 +80,15 @@ int add_txt_str(struct build_info *info, ofnode node, struct scene *scn, * @info: Build information * @node: Node describing scene * @scn: Scene to add to - * @find_name: Name to look for (e.g. "title"). This will find a string-list + * @find_name: Property to look for (e.g. "title"). This will find a string-list * property called "title" if it exists, else will look up the string in the * "title-id" string list. + * @obj_name: Name for the object (e.g. "cpu-speed.title") * Return: ID of added string, or -ve on error */ int add_txt_str_list(struct build_info *info, ofnode node, struct scene *scn, - const char *find_name, int index, uint obj_id) + const char *find_name, const char *obj_name, int index, + uint obj_id) { const char *text; int ret; @@ -107,7 +110,7 @@ int add_txt_str_list(struct build_info *info, ofnode node, struct scene *scn, return log_msg_ret("id", -EINVAL); } - ret = scene_txt_str(scn, find_name, obj_id, 0, text, NULL); + ret = scene_txt_str(scn, obj_name, obj_id, 0, text, NULL); if (ret < 0) return log_msg_ret("add", ret); @@ -220,6 +223,7 @@ static int menu_build(struct build_info *info, ofnode node, struct scene *scn, int ret, size, i, num_items; uint title_id, menu_id; const char *name; + char buf[80]; name = ofnode_get_name(node); @@ -229,7 +233,8 @@ static int menu_build(struct build_info *info, ofnode node, struct scene *scn, menu_id = ret; /* Set the title */ - ret = add_txt_str(info, node, scn, "title", 0); + snprintf(buf, sizeof(buf), "%s.title", name); + ret = add_txt_str(info, node, scn, "title", buf, 0); if (ret < 0) return log_msg_ret("tit", ret); title_id = ret; @@ -254,22 +259,28 @@ static int menu_build(struct build_info *info, ofnode node, struct scene *scn, struct scene_menitem *item; uint label, key, desc; - ret = add_txt_str_list(info, node, scn, "item-label", i, 0); + snprintf(buf, sizeof(buf), "%s.item-%x.label", name, i); + ret = add_txt_str_list(info, node, scn, "item-label", buf, i, + 0); if (ret < 0 && ret != -ENOENT) return log_msg_ret("lab", ret); label = max(0, ret); - ret = add_txt_str_list(info, node, scn, "key-label", i, 0); + snprintf(buf, sizeof(buf), "%s.item-%x.key", name, i); + ret = add_txt_str_list(info, node, scn, "key-label", buf, i, 0); if (ret < 0 && ret != -ENOENT) return log_msg_ret("key", ret); key = max(0, ret); - ret = add_txt_str_list(info, node, scn, "desc-label", i, 0); + snprintf(buf, sizeof(buf), "%s.item-%x.desc", name, i); + ret = add_txt_str_list(info, node, scn, "desc-label", buf, i, + 0); if (ret < 0 && ret != -ENOENT) return log_msg_ret("lab", ret); desc = max(0, ret); - ret = scene_menuitem(scn, menu_id, simple_xtoa(i), + snprintf(buf, sizeof(buf), "%s.item-%x", name, i); + ret = scene_menuitem(scn, menu_id, buf, fdt32_to_cpu(item_ids[i]), key, label, desc, 0, 0, &item); if (ret < 0) @@ -316,6 +327,7 @@ static int textline_build(struct build_info *info, ofnode node, struct scene_obj_textline *ted; uint edit_id; const char *name; + char buf[80]; u32 max_chars; int ret; @@ -330,8 +342,9 @@ static int textline_build(struct build_info *info, ofnode node, if (ret < 0) return log_msg_ret("ted", ret); - /* Set the title */ - ret = add_txt_str(info, node, scn, "title", 0); + /* Set the label */ + snprintf(buf, sizeof(buf), "%s.label", name); + ret = add_txt_str(info, node, scn, "title", buf, 0); if (ret < 0) return log_msg_ret("tit", ret); ted->label_id = ret; @@ -342,7 +355,8 @@ static int textline_build(struct build_info *info, ofnode node, if (ret) return log_msg_ret("id", -ENOENT); - ret = scene_txt_str(scn, "edit", edit_id, 0, abuf_data(&ted->buf), + snprintf(buf, sizeof(buf), "%s.edit", name); + ret = scene_txt_str(scn, buf, edit_id, 0, abuf_data(&ted->buf), NULL); if (ret < 0) return log_msg_ret("add", ret); @@ -414,6 +428,7 @@ static int scene_build(struct build_info *info, ofnode scn_node, const char *name; struct scene *scn; uint id, title_id; + char buf[80]; ofnode node; int ret; @@ -428,15 +443,18 @@ static int scene_build(struct build_info *info, ofnode scn_node, if (ret < 0) return log_msg_ret("scn", ret); - ret = add_txt_str(info, scn_node, scn, "title", 0); + snprintf(buf, sizeof(buf), "%s.title", name); + ret = add_txt_str(info, scn_node, scn, "title", buf, 0); if (ret < 0) return log_msg_ret("tit", ret); title_id = ret; scn->title_id = title_id; - ret = add_txt_str(info, scn_node, scn, "prompt", 0); + snprintf(buf, sizeof(buf), "%s.prompt", name); + ret = add_txt_str(info, scn_node, scn, "prompt", buf, 0); if (ret < 0) return log_msg_ret("pr", ret); + scn->prompt_id = ret; ofnode_for_each_subnode(node, scn_node) { info->err_node = node; diff --git a/test/boot/expo.c b/test/boot/expo.c index 66fd5a2873f..f98a1e46854 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -888,7 +888,7 @@ static int expo_test_build(struct unit_test_state *uts) ut_assertnonnull(txt); obj = &txt->obj; ut_asserteq_ptr(scn, obj->scene); - ut_asserteq_str("title", obj->name); + ut_asserteq_str("main.title", obj->name); ut_asserteq(scn->title_id, obj->id); ut_asserteq(SCENEOBJT_TEXT, obj->type); ut_asserteq(0, obj->flags); @@ -913,7 +913,7 @@ static int expo_test_build(struct unit_test_state *uts) /* check the items */ item = list_first_entry(&menu->item_head, struct scene_menitem, sibling); - ut_asserteq_str("00", item->name); + ut_asserteq_str("cpu-speed.item-0", item->name); ut_asserteq(ID_CPU_SPEED_1, item->id); ut_asserteq(0, item->key_id); ut_asserteq(0, item->desc_id); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Use descriptive names like "item0.label", "item0.desc", "item0.pass" instead of generic "label", "desc", "passphrase". This makes debugging easier when multiple bootflow items exist. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/bootflow_menu.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/boot/bootflow_menu.c b/boot/bootflow_menu.c index 17fab284ef7..84ae0223ba0 100644 --- a/boot/bootflow_menu.c +++ b/boot/bootflow_menu.c @@ -238,6 +238,7 @@ int bootflow_menu_add(struct expo *exp, struct bootflow *bflow, int seq, uint preview_id; uint scene_id; bool add_gap; + char name[40]; int ret; ret = expo_first_scene_id(exp); @@ -259,23 +260,30 @@ int bootflow_menu_add(struct expo *exp, struct bootflow *bflow, int seq, priv->last_bootdev = bflow->dev; ret = expo_str(exp, "prompt", STR_POINTER, ">"); - ret |= scene_txt_str(scn, "label", ITEM_LABEL + seq, + snprintf(name, sizeof(name), "item%d.label", seq); + ret |= scene_txt_str(scn, name, ITEM_LABEL + seq, STR_LABEL + seq, "", NULL); - ret |= scene_txt_str(scn, "desc", ITEM_DESC + seq, STR_DESC + seq, + snprintf(name, sizeof(name), "item%d.desc", seq); + ret |= scene_txt_str(scn, name, ITEM_DESC + seq, STR_DESC + seq, "", NULL); - ret |= scene_txt_str(scn, "key", ITEM_KEY + seq, STR_KEY + seq, key, + snprintf(name, sizeof(name), "item%d.key", seq); + ret |= scene_txt_str(scn, name, ITEM_KEY + seq, STR_KEY + seq, key, NULL); - ret |= scene_box(scn, "item-box", ITEM_BOX + seq, 1, false, NULL); - ret |= scene_txt_str(scn, "version", ITEM_VERSION_NAME + seq, + snprintf(name, sizeof(name), "item%d.box", seq); + ret |= scene_box(scn, name, ITEM_BOX + seq, 1, false, NULL); + snprintf(name, sizeof(name), "item%d.version", seq); + ret |= scene_txt_str(scn, name, ITEM_VERSION_NAME + seq, STR_VERSION_NAME + seq, "", NULL); preview_id = 0; if (bflow->logo) { preview_id = ITEM_PREVIEW + seq; - ret |= scene_img(scn, "preview", preview_id, + snprintf(name, sizeof(name), "item%d.preview", seq); + ret |= scene_img(scn, name, preview_id, bflow->logo, NULL); } - ret |= scene_menuitem(scn, OBJ_MENU, "item", ITEM + seq, + snprintf(name, sizeof(name), "item%d", seq); + ret |= scene_menuitem(scn, OBJ_MENU, name, ITEM + seq, ITEM_KEY + seq, ITEM_LABEL + seq, ITEM_DESC + seq, preview_id, add_gap ? SCENEMIF_GAP_BEFORE : 0, @@ -287,24 +295,27 @@ int bootflow_menu_add(struct expo *exp, struct bootflow *bflow, int seq, * Create passphrase textline with label and edit field (12 chars). Show * characters as asterisks */ - ret = scene_textline(scn, "passphrase", ITEM_PASS + seq, 12, &tline); + snprintf(name, sizeof(name), "item%d.pass", seq); + ret = scene_textline(scn, name, ITEM_PASS + seq, 12, &tline); if (ret < 0) return log_msg_ret("itp", -EINVAL); - tline->obj.flags |= SCENEOF_PASSWORD; - ret = scene_txt_str(scn, "pass_label", ITEM_PASS_LABEL + seq, 0, + snprintf(name, sizeof(name), "item%d.pass.label", seq); + ret = scene_txt_str(scn, name, ITEM_PASS_LABEL + seq, 0, "Passphrase:", NULL); if (ret < 0) return log_msg_ret("itl", -EINVAL); tline->label_id = ret; - ret = scene_txt_str(scn, "pass_edit", ITEM_PASS_EDIT + seq, 0, + snprintf(name, sizeof(name), "item%d.pass.edit", seq); + ret = scene_txt_str(scn, name, ITEM_PASS_EDIT + seq, 0, "", NULL); if (ret < 0) return log_msg_ret("ite", -EINVAL); tline->edit_id = ret; /* Create message text (hidden by default) for success/error feedback */ - ret = scene_txt_str(scn, "pass_msg", ITEM_PASS_MSG + seq, + snprintf(name, sizeof(name), "item%d.pass_msg", seq); + ret = scene_txt_str(scn, name, ITEM_PASS_MSG + seq, STR_PASS_MSG + seq, "", NULL); if (ret < 0) return log_msg_ret("ipm", -EINVAL); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The edit text object for the passphrase textline is created with an empty string literal "", rather than pointing to the textline's buffer. This means that when the user types, tline->buf is updated but the text object still displays the original empty string. Fix this by creating the edit text object with abuf_data(&tline->buf) so it points to the textline's buffer. This matches how expo_build.c creates textlines for cedit. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/bootflow_menu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/bootflow_menu.c b/boot/bootflow_menu.c index 84ae0223ba0..ee3150afef4 100644 --- a/boot/bootflow_menu.c +++ b/boot/bootflow_menu.c @@ -308,7 +308,7 @@ int bootflow_menu_add(struct expo *exp, struct bootflow *bflow, int seq, snprintf(name, sizeof(name), "item%d.pass.edit", seq); ret = scene_txt_str(scn, name, ITEM_PASS_EDIT + seq, 0, - "", NULL); + abuf_data(&tline->buf), NULL); if (ret < 0) return log_msg_ret("ite", -EINVAL); tline->edit_id = ret; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Use the UTF_NO_SILENT flag for cedit_render() instead of manually calling ut_unsilence_console()/ut_silence_console(). This is makes it more obvious that the test needs this handling. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/cedit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/boot/cedit.c b/test/boot/cedit.c index 7af3dfa2338..c4d402343fa 100644 --- a/test/boot/cedit.c +++ b/test/boot/cedit.c @@ -352,11 +352,11 @@ static int cedit_render(struct unit_test_state *uts) * vidconsole state, outputs the character and then saves the state * again. If the character is never output, then the state won't be * updated and the textline will be inconsistent. + * + * This is why this test enables UTF_NO_SILENT */ - ut_unsilence_console(uts); for (i = 'a'; i < 'd'; i++) ut_assertok(scene_send_key(scn, i, &evt)); - ut_silence_console(uts); ut_assertok(cedit_arange(exp, vid_priv, scn->id)); ut_assertok(expo_render(exp)); ut_asserteq(5076, video_compress_fb(uts, dev, false)); @@ -366,7 +366,7 @@ static int cedit_render(struct unit_test_state *uts) return 0; } -BOOTSTD_TEST(cedit_render, UTF_DM | UTF_SCAN_FDT); +BOOTSTD_TEST(cedit_render, UTF_DM | UTF_SCAN_FDT | UTF_NO_SILENT); /* Check the cedit displays textlines correctly */ static int cedit_render_textline(struct unit_test_state *uts) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> This test works as part of the bootstd suite but currently fails if run by itself. The problem is that the console is silenced, so use the new UTF_NO_SILENT flag to fix this. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/cedit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/boot/cedit.c b/test/boot/cedit.c index c4d402343fa..b8c46f2d50c 100644 --- a/test/boot/cedit.c +++ b/test/boot/cedit.c @@ -446,7 +446,7 @@ static int cedit_render_textline(struct unit_test_state *uts) return 0; } -BOOTSTD_TEST(cedit_render_textline, UTF_DM | UTF_SCAN_FDT); +BOOTSTD_TEST(cedit_render_textline, UTF_DM | UTF_SCAN_FDT | UTF_NO_SILENT); /* Check the cedit is arranged correctly */ static int cedit_position(struct unit_test_state *uts) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The highlight_id needs to be considered for non-popup expos as well. As a first step, use the variable 'cur' for the current object, i.e. the one that is highlighted. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index 1392d063c49..1bc4c9c25ac 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -1082,30 +1082,28 @@ static void send_key_obj(struct scene *scn, struct scene_obj *obj, int key, int scene_send_key(struct scene *scn, int key, struct expo_action *event) { - struct scene_obj *obj; + struct scene_obj *cur, *obj; int ret; event->type = EXPOACT_NONE; /* - * In 'popup' mode, arrow keys move betwen objects, unless a menu is - * opened + * In 'popup' mode, arrow keys move betwen objects, unless a menu or + * textline is opened */ + cur = NULL; + if (scn->highlight_id) + cur = scene_obj_find(scn, scn->highlight_id, SCENEOBJT_NONE); if (scn->expo->popup) { - obj = NULL; - if (scn->highlight_id) { - obj = scene_obj_find(scn, scn->highlight_id, - SCENEOBJT_NONE); - } - if (!obj) + if (!cur) return 0; - if (!(obj->flags & SCENEOF_OPEN)) { - send_key_obj(scn, obj, key, event); + if (!(cur->flags & SCENEOF_OPEN)) { + send_key_obj(scn, cur, key, event); return 0; } - switch (obj->type) { + switch (cur->type) { case SCENEOBJT_NONE: case SCENEOBJT_IMAGE: case SCENEOBJT_TEXT: @@ -1114,7 +1112,7 @@ int scene_send_key(struct scene *scn, int key, struct expo_action *event) case SCENEOBJT_MENU: { struct scene_obj_menu *menu; - menu = (struct scene_obj_menu *)obj, + menu = (struct scene_obj_menu *)cur, ret = scene_menu_send_key(scn, menu, key, event); if (ret) return log_msg_ret("key", ret); @@ -1123,7 +1121,7 @@ int scene_send_key(struct scene *scn, int key, struct expo_action *event) case SCENEOBJT_TEXTLINE: { struct scene_obj_textline *tline; - tline = (struct scene_obj_textline *)obj, + tline = (struct scene_obj_textline *)cur, ret = scene_textline_send_key(scn, tline, key, event); if (ret) return log_msg_ret("key", ret); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a function to render a single object by its ID. This provides a convenient way to render individual objects without needing to look up the object pointer first. Assume that text mode is not used. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 18 ++++++++++++++++++ boot/scene_internal.h | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/boot/scene.c b/boot/scene.c index 1bc4c9c25ac..1bcc3c39e33 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -903,6 +903,24 @@ int scene_arrange(struct scene *scn) return 0; } +int scene_render_obj(struct scene *scn, uint id) +{ + struct scene_obj *obj; + int ret; + + obj = scene_obj_find(scn, id, SCENEOBJT_NONE); + if (!obj) + return log_msg_ret("obj", -ENOENT); + + if (!(obj->flags & SCENEOF_HIDE)) { + ret = scene_obj_render(obj, false); + if (ret && ret != -ENOTSUPP) + return log_msg_ret("ren", ret); + } + + return 0; +} + int scene_render_deps(struct scene *scn, uint id) { struct scene_obj *obj; diff --git a/boot/scene_internal.h b/boot/scene_internal.h index 2bfbb5dcf50..5cc81f031a0 100644 --- a/boot/scene_internal.h +++ b/boot/scene_internal.h @@ -314,6 +314,15 @@ bool scene_textline_within(const struct scene *scn, */ int scene_send_click(struct scene *scn, int x, int y, struct expo_action *event); +/** + * scene_render_obj() - Render an object + * + * @scn: Scene containing the object + * @id: Object ID to render + * Returns: 0 if OK, -ENOENT if object not found, -ve on other error + */ +int scene_render_obj(struct scene *scn, uint id); + /** * scene_render_deps() - Render an object and its dependencies * -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When a textline is open, we must render the edit string and show the cursor. The easiest way to do this is to draw the string again, then move the cursor back to the correct place. There is no need to change the font, since the rendering function handles this. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene_textline.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/boot/scene_textline.c b/boot/scene_textline.c index 7e01959c40d..20e2d7f33c4 100644 --- a/boot/scene_textline.c +++ b/boot/scene_textline.c @@ -189,28 +189,21 @@ int scene_textline_render_deps(struct scene *scn, { const bool open = tline->obj.flags & SCENEOF_OPEN; struct udevice *cons = scn->expo->cons; - struct scene_obj_txt *txt; - int ret; - - scene_render_deps(scn, tline->label_id); - scene_render_deps(scn, tline->edit_id); + uint i; - /* show the vidconsole cursor if open */ + /* if open, render the edit text on top of the background */ if (open) { - /* get the position within the field */ - txt = scene_obj_find(scn, tline->edit_id, SCENEOBJT_NONE); - if (!txt) - return log_msg_ret("cur", -ENOENT); - - if (txt->gen.font_name || txt->gen.font_size) { - ret = vidconsole_select_font(cons, - txt->gen.font_name, - txt->gen.font_size); - } else { - ret = vidconsole_select_font(cons, NULL, 0); - } + int ret; ret = vidconsole_entry_restore(cons, &scn->entry_save); + if (ret) + return log_msg_ret("sav", ret); + scene_render_obj(scn, tline->edit_id); + + /* move cursor back to the correct position */ + for (i = scn->cls.num; i < scn->cls.eol_num; i++) + vidconsole_put_char(cons, '\b'); + ret = vidconsole_entry_save(cons, &scn->entry_save); if (ret) return log_msg_ret("sav", ret); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> At present the backspace key (Ctrl-B) has the same value as BKEY_UP, which means the keys are ambiguous. This causes problems when trying to edit a textline. The same problem applies to other keys used in cread_line_process_ch() Start the bootmenu_key enum values at 0x100 to avoid this collision. Keep BKEY_NONE as 0, since it is a sentinel value. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/menu.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/include/menu.h b/include/menu.h index 6cede89b950..9914e680307 100644 --- a/include/menu.h +++ b/include/menu.h @@ -45,10 +45,16 @@ struct bootmenu_data { struct bootmenu_entry *first; /* first menu entry */ }; -/** enum bootmenu_key - keys that can be returned by the bootmenu */ +/** + * enum bootmenu_key - keys that can be returned by the bootmenu + * + * These values start at 0x100 to avoid colliding with ASCII control characters + * (0x01-0x1f) which are used for editing operations in textlines. BKEY_NONE is + * kept at 0 as a sentinel value. + */ enum bootmenu_key { BKEY_NONE = 0, - BKEY_UP, + BKEY_UP = 0x100, BKEY_DOWN, BKEY_SELECT, BKEY_QUIT, -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Move textline key handling outside the loop so keys are always sent to the highlighted textline, regardless of whether it is open. This allows the textline to respond to keyboard input even if not in a popup expo. Otherwise continue to send keys to menus as now. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/scene.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/boot/scene.c b/boot/scene.c index 1bcc3c39e33..5c19bff6011 100644 --- a/boot/scene.c +++ b/boot/scene.c @@ -1152,6 +1152,16 @@ int scene_send_key(struct scene *scn, int key, struct expo_action *event) return 0; } + if (cur && cur->type == SCENEOBJT_TEXTLINE) { + struct scene_obj_textline *tline; + + tline = (struct scene_obj_textline *)cur; + ret = scene_textline_send_key(scn, tline, key, event); + if (ret) + return log_msg_ret("key", ret); + return 0; + } + list_for_each_entry(obj, &scn->obj_head, sibling) { if (obj->type == SCENEOBJT_MENU) { struct scene_obj_menu *menu; @@ -1161,15 +1171,6 @@ int scene_send_key(struct scene *scn, int key, struct expo_action *event) if (ret) return log_msg_ret("key", ret); break; - } else if (!(obj->flags & SCENEOF_OPEN) && - obj->type == SCENEOBJT_TEXTLINE) { - struct scene_obj_textline *tline; - - tline = (struct scene_obj_textline *)obj; - ret = scene_textline_send_key(scn, tline, key, event); - if (ret) - return log_msg_ret("key", ret); - break; } } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When the user types in an unlock password then presses 'enter', select the item containing the password. This avoids needing to click on an item as well. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/bootflow_menu.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/boot/bootflow_menu.c b/boot/bootflow_menu.c index ee3150afef4..01af30c6627 100644 --- a/boot/bootflow_menu.c +++ b/boot/bootflow_menu.c @@ -457,6 +457,13 @@ int bootflow_menu_poll(struct expo *exp, int *seqp) } case EXPOACT_QUIT: return -EPIPE; + case EXPOACT_CLOSE: + /* + * Password textline closed (Enter pressed) - treat as + * selection + */ + *seqp = act.select.id - ITEM_PASS; + break; case EXPOACT_CLICK: if (act.select.id == OBJ_SETTINGS) return -ECOMM; /* layout change request */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a test that creates a textline in a non-popup expo, renders it, opens it, sends keypresses to edit text (including cursor movement with Ctrl+B and character deletion with Ctrl+D), and verifies the text is saved when the textline is closed. Co-developed-by: Claude <claude@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/expo.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/test/boot/expo.c b/test/boot/expo.c index f98a1e46854..03ad3e2167d 100644 --- a/test/boot/expo.c +++ b/test/boot/expo.c @@ -33,6 +33,7 @@ enum { OBJ_MENU_TITLE, OBJ_BOX, OBJ_BOX2, + OBJ_TEXTLINE, OBJ_TEXTED, OBJ_OVERLAP, @@ -1392,3 +1393,118 @@ static int expo_dump_test(struct unit_test_state *uts) return 0; } BOOTSTD_TEST(expo_dump_test, UTF_DM | UTF_SCAN_FDT); + +/* Check behaviour of textlines in a non-popup expo */ +static int expo_render_textline(struct unit_test_state *uts) +{ + struct scene_obj_textline *tline; + struct scene_obj_menu *menu; + struct abuf buf, logo_copy; + struct expo_action act; + struct scene *scn; + struct udevice *dev; + struct expo *exp; + int id; + + ut_assertok(create_test_expo(uts, &exp, &scn, &menu, &buf, &logo_copy)); + dev = exp->display; + + id = scene_textline(scn, "textline", OBJ_TEXTLINE, 20, &tline); + ut_assert(id > 0); + ut_assertok(scene_obj_set_pos(scn, OBJ_TEXTLINE, 500, 500)); + strcpy(abuf_data(&tline->buf), "sample hopwind"); + + /* create the label text object */ + id = scene_txt_str(scn, "tline-label", 0, 0, "Label:", NULL); + ut_assert(id > 0); + tline->label_id = id; + + /* create the edit text object pointing to the textline buffer */ + id = scene_txt_str(scn, "tline-edit", 0, 0, abuf_data(&tline->buf), + NULL); + ut_assert(id > 0); + tline->edit_id = id; + ut_assertok(scene_txt_set_font(scn, tline->edit_id, + "nimbus_sans_l_regular", 40)); + + expo_set_scene_id(exp, SCENE1); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(21007, video_compress_fb(uts, dev, false)); + + /* highlight the textline and re-render */ + scene_set_highlight_id(scn, OBJ_TEXTLINE); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22693, video_compress_fb(uts, dev, false)); + + /* open the textline and re-render */ + ut_assertok(scene_set_open(scn, OBJ_TEXTLINE, true)); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + + /* the cursor should be at the end */ + ut_asserteq(22695, video_compress_fb(uts, dev, false)); + + /* send a keypress to add a character */ + ut_assertok(expo_send_key(exp, 'a')); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22818, video_compress_fb(uts, dev, false)); + + /* move cursor left 3 times */ + ut_assertok(expo_send_key(exp, CTL_CH('b'))); + + /* check cursor moved back one position, before 'a' */ + ut_asserteq(14, scn->cls.num); + ut_asserteq(15, scn->cls.eol_num); + ut_asserteq_str("sample hopwinda", abuf_data(&tline->buf)); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22884, video_compress_fb(uts, dev, false)); + + ut_assertok(expo_send_key(exp, CTL_CH('b'))); + ut_assertok(expo_send_key(exp, CTL_CH('b'))); + ut_assertok(expo_send_key(exp, CTL_CH('b'))); + + /* check cursor moved back three more positions, before 'i' */ + ut_asserteq(11, scn->cls.num); + ut_asserteq(15, scn->cls.eol_num); + ut_asserteq_str("sample hopwinda", abuf_data(&tline->buf)); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22915, video_compress_fb(uts, dev, false)); + + /* delete a character at the cursor */ + ut_assertok(expo_send_key(exp, CTL_CH('d'))); + + /* check character deleted at cursor position */ + ut_asserteq(11, scn->cls.num); + ut_asserteq(14, scn->cls.eol_num); + ut_asserteq_str("sample hopwnda", abuf_data(&tline->buf)); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22856, video_compress_fb(uts, dev, false)); + + /* close the textline with Enter (BKEY_SELECT) */ + ut_assertok(expo_send_key(exp, BKEY_SELECT)); + ut_assertok(expo_action_get(exp, &act)); + ut_asserteq(EXPOACT_CLOSE, act.type); + ut_asserteq(OBJ_TEXTLINE, act.select.id); + ut_assertok(scene_set_open(scn, act.select.id, false)); + + /* check the textline is closed and text was saved */ + ut_asserteq(0, tline->obj.flags & SCENEOF_OPEN); + ut_asserteq_str("sample hopwnda", abuf_data(&tline->buf)); + ut_assertok(scene_arrange(scn)); + ut_assertok(expo_render(exp)); + ut_asserteq(22839, video_compress_fb(uts, dev, false)); + + abuf_uninit(&buf); + abuf_uninit(&logo_copy); + + expo_destroy(exp); + + return 0; +} +BOOTSTD_TEST(expo_render_textline, UTF_DM | UTF_SCAN_FDT | UTF_NO_SILENT); -- 2.43.0
participants (1)
-
Simon Glass