Hi Heinrich, On Sat, 29 Oct 2022 at 20:56, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > > > On 10/30/22 02:43, Simon Glass wrote: > > 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) > > A program outputting different things in continuous integration then in > real live does not provide realistic testing. > > It is the CI that has to adapt not the program under test. > > Which logs are you talking about? > > > 2. We cannot show an ANSI code on boot by default, since that puts > > ctrl characters into every start-up of U-Boot > > Why would that be a problem? > > > > > 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. > > isatty() is not a U-Boot function. So it is irrelevant in this context. > > You cannot detect if a serial console understands ANSI sequences without > sending any.
NAK Please try a little harder to understand my POV above. Regards, Simon
