[PATCH] menu: Re-enable the ANSI codes

[Pali Rohár]

On Thursday 15 June 2023 13:34:09 Simon Glass wrote:
> Hi Pali,
> 
> On Mon, 12 Jun 2023 at 22:33, Pali Rohár <pali at kernel.org> wrote:
> >
> > On Monday 12 June 2023 22:17:48 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Mon, 12 Jun 2023 at 21:22, Pali Rohár <pali at kernel.org> wrote:
> > > >
> > > > On Monday 12 June 2023 21:14:32 Simon Glass wrote:
> > > > > The intent here was to allow ANSI codes to be disabled, since it was
> > > > > proving impoosible to test operation of the menu code when it kept moving
> > > > > the cursor. Unfortunately this ended up in the patch.
> > > > >
> > > > > Correct this by enabling ANSI again.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > Reported-by: Pali Rohár <pali at kernel.org>
> > > > > Reported-by: Mark Kettenis <mark.kettenis at xs4all.nl>
> > > > > Reported-by: Frank Wunderlich <frank-w at public-files.de>
> > > > > Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
> > > > > ---
> > > > >
> > > > >  common/menu.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/common/menu.c b/common/menu.c
> > > > > index 94514177e4e9..b55cf7b99967 100644
> > > > > --- a/common/menu.c
> > > > > +++ b/common/menu.c
> > > > > @@ -15,7 +15,7 @@
> > > > >
> > > > >  #include "menu.h"
> > > > >
> > > > > -#define ansi 0
> > > > > +#define ansi 1
> > > > >
> > > > >  /*
> > > > >   * Internally, each item in a menu is represented by a struct menu_item.
> > > > > --
> > > > > 2.41.0.162.gfafddb0af9-goog
> > > > >
> > > >
> > > > Hello, I have tested this change but bootmenu still does not work. There
> > > > is still same issue which I reported month ago. When I press DOWN key
> > > > then bootmenu immediately quit instead of moving into the next entry.
> > >
> > > Thanks for testing this.
> > >
> > > Is there a way for me to test this with sandbox? Or does your Nokia
> > > test check it?
> >
> > I guess that bootmenu command could work in sandbox. But I have not
> > tried it.
> >
> > Nokia CI test does not try any terminal, keyboard or VGA interaction, so
> > broken rendering or broken keyboard input is not caught by CI.
> >
> > But it is possible to test it manually. See U-Boot documentation how to
> > run Nokia u-boot image in qemu. Bootmenu is automatically started.
> > https://u-boot.readthedocs.io/en/latest/board/nokia/rx51.html#run-in-qemu
> 
> I tried to follow this but got stuck here:
> 
> ./configure --enable-system --target-list=arm-softmmu --disable-werror
> 
> ERROR: Cannot use 'python', Python 2.4 or later is required.
>        Note that Python 3 or later is not yet supported.
>        Use --python=/path/to/python to specify a supported Python.
> 
> Python 2 has been deprecated for years and I think it was removed recently.

Ach :-( Is not there some configure option to disable python?

In U-Boot CI is qemu compiling without issues (but variant without GUI).

> >
> > Bootmenu is available on both VGA display and terminal output. Problem
> > with DOWN key is on the terminal.
> >
> > >
> > > I'm sure you have this commit:?
> > >
> > > 17b45e684af9 ("cli: Correct several bugs in cli_getch()")
> >
> > Yes, as it is in the master branch already.
> 
> I tried the following with sandbox:
> 
> setenv bootmenu_0 "my item=echo item 0"
> setenv bootmenu_1 "second item=echo item 1"
> setenv bootmenu_delay 10
> bootmenu
> 
> I see that with the ANSI patch the 'press key' prompt now appears in
> the right place.
> 
> When I press down key repeatedly, it goes down until the last item and
> then refuses to go any further.
> 
> So I can't repeat this problem.

Hm... so it looks like some ARM or UART specific thing... or something
which is not triggered by sandbox.

Just some suggestions, could you try to add some more entries? Or trying
to wait e.g. 30s before pressing another key (qemu is slow so there can
be some delay issue?)? Or could you try to run bootmenu on some real
hardware via UART?

> I tried looking at the Nokia code. The keyboard driver should be in
> drivers/input, but I eventually found it in the board directory.
> 
> Are you sure that rx51_kp_tstc() behaves correctly? Can you add
> debugging to see what codes it is emitting?
> 
> Also you don't need to generate escape sequences in rx51_kp_fill().
> You can just use Ctrl-N, Ctrl-P, Ctrl-B and Ctrl-F (see cli_ch_esc()).
> That should simplify things a bit.
> 
> If you can find a way to repeat this problem and have verified that
> the Nokia code is correct, I can take another look.

Problem with DOWN key is on the terminal, not on the screen or keyboard.
So all those rx51_kp_tstc() and rx51_kp_fill() should not be the cause
(unless some input/output multiplexing in u-boot takes a role).

Terminal to qemu is connected via qemu's UART emulation, so omap2 uart
driver in this case is the one which connects u-boot to the qemu's UART
emulation.

> Regards,
> Simon
> 
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20230612201434.861700-1-sjg@chromium.org/