Hi Heinrich, On Thu, 27 Oct 2022 at 10:41, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > > > On 10/27/22 17:22, Simon Glass wrote: > > Hi Heinrich, > > > > On Wed, 26 Oct 2022 at 00:13, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> > >> > >> On 10/26/22 01:35, Simon Glass wrote: > >>> Hi Heinrich, > >>> > >>> On Sat, 22 Oct 2022 at 03:21, Heinrich Schuchardt > >>> <heinrich.schucha...@canonical.com> wrote: > >>>> > >>>> We may enter the command line interface in a state where on the remote > >>>> console the cursor is not shown. Send an escape sequence to enable it. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >>>> --- > >>>> common/main.c | 4 ++++ > >>>> 1 file changed, 4 insertions(+) > >>>> > >>>> diff --git a/common/main.c b/common/main.c > >>>> index 682f3359ea..4e7e7b17d6 100644 > >>>> --- a/common/main.c > >>>> +++ b/common/main.c > >>>> @@ -7,6 +7,7 @@ > >>>> /* #define DEBUG */ > >>>> > >>>> #include <common.h> > >>>> +#include <ansi.h> > >>>> #include <autoboot.h> > >>>> #include <bootstage.h> > >>>> #include <cli.h> > >>>> @@ -66,6 +67,9 @@ void main_loop(void) > >>>> > >>>> autoboot_command(s); > >>>> > >>>> + if (!CONFIG_IS_ENABLED(DM_VIDEO) || > >>>> CONFIG_IS_ENABLED(VIDEO_ANSI)) > >>>> + printf(ANSI_CURSOR_SHOW "\n"); > >>> > >>> Could we create a library which emits these codes? Like > >>> ansi_cursor_show() ? > >> > >> The question is not if we could but if we should. > >> > >> Up to now we tried to call printf() only once where ANSI_* is only one > >> part of the output string. > >> > >> E.g. cmd/eficonfig.c:415: > >> printf(ANSI_CLEAR_CONSOLE > >> ANSI_CURSOR_POSITION > >> ANSI_CURSOR_SHOW, 1, 1); > >> > >> This minimizes the number of call library calls but may not be very > >> elegant. > >> > >> Creating a library function for ANSI_CURSOR_SHOW alone does not make > >> much sense to me. Should you want to convert our code base from using > >> ANSI_* to library functions this should cover the whole code base. > >> > >> So I think we should let this patch pass as it solves a current problem > >> and leave creating a ANSI_* function library to a separate exercise. > > > > Then we should add this to the serial API...what we have here is quite > > bizarre in a way, since it outputs escape characters as the default > > for U-Boot. Mostly we don't need it. > > > > So add a set_cursor_visible() method to the serial API and allow it to > > be controlled via platform data in the serial driver. > > I have no clue which platform data you might be thinking of. > > It is also not my intention to eject the escape sequence whenever a > serial console is probed. > > Using serial_puts() in this patch instead of printf() would help to > avoid the CONFIG dependency.
Here's what should happen: 1. ANSI codes should not appear in logs or on CI (this is currently a problem with EFI tests) 2. We cannot show an ANSI code on boot by default, since that puts ctrl characters into every start-up of U-Boot So I think my suggestion makes sense. See drivers/serial/sandbox.c and have a way of calling isatty() to check the terminal type (with a cmdline flag to override it, see -t for example). Then we can get the correct behaviour. Regards, Simon
