shell and putchar

Hi all,

I’m working on improving IRQ support on PIC32 and I’m testing UART Rx using the shell.

The shell uses putchar, but putchar is buffered and needs a flush, thus when typing at the console I get no feedback until I press enter (the line feed gets echo’d and then we get the flush).

I’m surprised other newlib implementations don’t see this ?

A flush is probably needed here:

#if !defined(SHELL_NO_ECHO) || !defined(SHELL_NO_PROMPT) #ifdef MODULE_NEWLIB /* use local copy of putchar, as it seems to be inlined,

  • enlarging code by 50% */ static void _putchar(int c) { putchar©;

fflush(STDOUT) <------- HERE

} #else #define _putchar putchar #endif #endif

I’m happy to do a PR for this, but this is very core functionality, I don’t want to break anything ??

Cheers,

Neil

Hi Neil,

digging through old unread emails

Answering inline.

Hi all,

I'm working on improving IRQ support on PIC32 and I'm testing UART Rx using the shell.

The shell uses putchar, but putchar is buffered and needs a flush, thus when typing at the console I get no feedback until I press enter (the line feed gets echo'd and then we get the flush).

I'm surprised other newlib implementations don't see this ?

I think most developers have their local program doing the 'echo' for them and only sending lines by lines anyway. So I personally do not really pay attention to this. I did not even knew we were doing 'echo' by default…

A flush is probably needed here:

#if !defined(SHELL_NO_ECHO) || !defined(SHELL_NO_PROMPT) #ifdef MODULE_NEWLIB /* use local copy of putchar, as it seems to be inlined,   * enlarging code by 50% */ static void _putchar(int c) {      putchar(c);      fflush(STDOUT) <------- HERE

} #else #define _putchar putchar #endif

I agree it could make sense to add it when in `echo` mode only.

     putchar(c) #ifndef SHELL_NO_ECHO      fflush(stdout); #endif

As for the shell prompt it is already handled here RIOT/sys/shell/shell.c at master · RIOT-OS/RIOT · GitHub

I'm happy to do a PR for this, but this is very core functionality, I don't want to break anything ??

But maybe other developers have more knowledge about this.

Cheers, Gaëtan