[PATCH v3 03/25] bootmenu: Add a few comments

[Simon Glass]

Hi Heinrich,

On Sat, 7 Jan 2023 at 15:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 1/7/23 01:13, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 6 Jan 2023 at 09:53, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 1/6/23 15:52, Simon Glass wrote:
> >>> The behaviour of these two functions is completely undocumented. Add some
> >>> notes so the poor, suffering dev can figure out what is going on.
> >>>
> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>    include/menu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> >>>    1 file changed, 42 insertions(+)
> >>>
> >>> diff --git a/include/menu.h b/include/menu.h
> >>> index 702aacb170c..0b4d9734149 100644
> >>> --- a/include/menu.h
> >>> +++ b/include/menu.h
> >>> @@ -42,6 +42,7 @@ struct bootmenu_data {
> >>>        struct bootmenu_entry *first;   /* first menu entry */
> >>>    };
> >>>
> >>> +/** enum bootmenu_key - keys that can be returned by the bootmenu */
> >>>    enum bootmenu_key {
> >>>        KEY_NONE = 0,
> >>>        KEY_UP,
> >>> @@ -53,8 +54,49 @@ enum bootmenu_key {
> >>>        KEY_SPACE,
> >>>    };
> >>>
> >>> +/**
> >>> + * bootmenu_autoboot_loop() - handle autobooting if no key is pressed
> >>> + *
> >>> + * This shows a prompt to allow the user to press a key to interrupt auto boot
> >>> + * of the first menu option.
> >>> + *
> >>> + * It then waits for the required time (menu->delay in seconds) for a key to be
> >>> + * pressed. If nothing is pressed in that time, @key returns KEY_SELECT
> >>> + * indicating that the current option should be chosen.
> >>> + *
> >>> + * @menu: Menu being processed
> >>> + * @key: Returns the code for the key the user pressed:
> >>> + *   enter: KEY_SELECT
> >>> + *   Ctrl-C: KEY_QUIT
> >>> + *   anything else: KEY_NONE
> >>> + * @esc: Set to 1 if the escape key is pressed, otherwise not updated
> >>> + */
> >>>    void bootmenu_autoboot_loop(struct bootmenu_data *menu,
> >>>                            enum bootmenu_key *key, int *esc);
> >>> +
> >>> +/**
> >>> + * bootmenu_loop() - handle waiting for a keypress when autoboot is disabled
> >>> + *
> >>> + * This is used when the menu delay is negative, indicating that the delay has
> >>> + * elapsed, or there was no delay to begin with.
> >>> + *
> >>> + * It reads a character and processes it, returning a menu-key code if a
> >>> + * character is recognised
> >>> + *
> >>> + * @menu: Menu being processed
> >>> + * @key: Returns the code for the key the user pressed:
> >>> + *   enter: KEY_SELECT
> >>> + *   Ctrl-C: KEY_QUIT
> >>> + *   Up arrow: KEY_UP
> >>> + *   Down arrow: KEY_DOWN
> >>> + *   Escape (by itself): KEY_QUIT
> >>> + *   Plus: KEY_PLUS
> >>> + *   Minus: KEY_MINUS
> >>> + *   Space: KEY_SPACE
> >>> + * @esc: On input, a non-zero value indicates that an escape sequence has
> >>> + *   resulted in that many characters so far. On exit this is updated to the
> >>> + *   new number of characters
> >>> + */
> >>>    void bootmenu_loop(struct bootmenu_data *menu,
> >>>                   enum bootmenu_key *key, int *esc);
> >>
> >> This should not be an exported function. The program part that wants to
> >> display a menu should not have to deal with navigation keys.
> >
> > This is a patch to add comments to code which is not otherwise
> > addressed in this series. We need comments so that we know what things
> > do. We can always adjust the logic later.
> >
> > My intent with this series is to add a new menu which is more flexible.
>
> We should take care that the new menu is not using this design.
>

Yes, the new menu does not use any of the code here.

This patch is purely to add comments.

I wonder whether the expo menu might eventually replace the current
menu, since the expo one does support text mode too.

>
> This series should be of interest for you too:
>
> https://patchwork.ozlabs.org/project/uboot/list/?series=335054
> eficonfig: add vertical scroll support and refactoring

OK ta. Scrolling is another thing not supported by expo.

Regards,
Simon