Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow redirecting stdio to arbitrary UART #6019

Closed
bmcdonnell-ionx opened this issue Feb 5, 2018 · 4 comments
Closed

allow redirecting stdio to arbitrary UART #6019

bmcdonnell-ionx opened this issue Feb 5, 2018 · 4 comments

Comments

@bmcdonnell-ionx
Copy link
Contributor

Description


Enhancement

Reason to enhance or problem with existing solution

AFAIK, there is no way for a user to redirect stdin/stdout/stderr to an arbitrary UART object:

  • while maintaining line buffering when using Serial
  • when using UARTSerial

(IDK the situation with other *Serial classes.)

The only way I know how to do it now for a Serial object is like this:

char const  SerialStreamPath[] = "/serial";
char const *SerialStreamName   = &(SerialStreamPath[1]);
 
Serial s(P0_0 , P0_1 , SerialStreamName, SERIAL_BAUD_RATE);
 
int main()
{
   // https://os.mbed.com/users/simon/code/stdout/docs/tip/main_8cpp_source.html
   freopen(SerialStreamPath, "w", stdout);
 
   //...
}

But that's suboptimal, because it makes stdout fully buffered instead of line buffered. (Maybe I could make it line buffered again by calling setvbuf(), but then I'd be wasting whatever buffer was already set aside for stdout.)

AFAICT, we can't do that on a UARTSerial object, though, since it doesn't have a ctor with the name parameter.

(I first asked part of this question here.)

Suggested enhancement

Allow user to redirect stdin, stdout, and/or stderr - individually or together - to an arbitrary UART port - probably when using an object of any class that inherits from SerialBase. Maintain line buffering by default if the new source/destination is a UART.

I don't have a strong opinion yet on what the interface should be. e.g. If you do the freopen() POSIX way, or if you add say "retarget" functions to SerialBase, or some other way. In any case, please give examples in the official docs.

@kjbracey
Copy link
Contributor

kjbracey commented Feb 6, 2018

Unless I'm missing something, what you're asking for is one of the 4 things provided by #5571, and the most-discussed:

  • The ability for target or application to change stdin/stdout/stderr to be a custom FileHandle by overriding weak functions..

You just implement mbed_override_console():

FileHandle *mbed_override_console(int fd) {
      status UARTSerial s(P0_0, P0_1, ...);
      return &s;
}

The existing FileLike "name" mechanism for finding devices by a name passed to fopen is problematic because it doesn't allow linker exclusion. If you had multiple devices like that, they'd all be put in the image, and opened at start time. The fopen call doesn't actually open them - just looks them up - and the device driver will be in the image and opened whether the application "opens" them or not.

The pattern above better reflects the reality of "opened on construction" for those device-like objects and allows linker exclusion if the "give me the filehandle" function is never called.

Serial and Stream further compound the confusion because they actually fopen themselves as well as open. If you fopen a Serial object by name like your example you've actually opened it twice in the C library! They also have an isatty that returns false making them buffered by default - their self-opening overrides with a "set unbuffered".

As such, there is no existing class you can use to neatly override non-buffered with mbed_override_console. This is kind of intentional - an unbuffered serial port does not work well as a normal application I/O device suitable for connecting to the C library. But as some people may want it for synchronous output-only logging use, extending RawSerial to be a working FileHandle like UARTSerial would make sense, as noted in #5571 comments.

Mind you, rather than using the fully-generic "override to an arbitrary device" mechanism, it would possibly be easier for the simplest serial case to just add two more JSON option for mbed_retarget. If you look at the default serial behaviour in the absence of mbed_override_console, it now has 4 fundamental parameters - TX pin, RX pin, baud rate, buffered or not

static FileHandle* default_console()
{
#if DEVICE_SERIAL
#  if MBED_CONF_PLATFORM_STDIO_BUFFERED_SERIAL
    static UARTSerial console(STDIO_UART_TX, STDIO_UART_RX,  MBED_CONF_PLATFORM_STDIO_BAUD_RATE);
#  else
static DirectSerial console(STDIO_UART_TX, STDIO_UART_RX, MBED_CONF_PLATFORM_STDIO_BAUD_RATE);
#  endif
#else // DEVICE_SERIAL
    static Sink console;
#endif
    return &console;
}

The newest 2 options - baud and buffered are JSON; the original pin options are fixed by the target. If the application could just override STDIO_UART_TX/RX easily... (Or can they just define them in the JSON anyway?)

Looking at the example linked - https://os.mbed.com/users/simon/code/stdout/docs/tip/main_8cpp_source.html - that pattern wouldn't quite mesh with #5571. There's no fdreopen allowing you to after-startup reattach stdout. Probably should be added.

Still, it's potentially inefficient, because you're still including all the code and initialisation of the original stdout - that's part of the point of the mbed_override_console mechanism. If you override that, the default code is left out.

@bmcdonnell-ionx
Copy link
Contributor Author

  • The ability for target or application to change stdin/stdout/stderr to be a custom FileHandle by overriding weak functions.

There's a lot of things going on over at #5571. Yes, my request here does appear to be covered by that. Thanks for pointing it out.

You just implement mbed_override_console():

How will a user maintain line buffering on a redirected stdio with this method?

(BTW, typo in your first code block - static, not status - right?)

Looking at the example linked - https://os.mbed.com/users/simon/code/stdout/docs/tip/main_8cpp_source.html - that pattern wouldn't quite mesh with #5571. There's no fdreopen allowing you to after-startup reattach stdout. Probably should be added.

As you suggest, it may be best to maintain existing functionality, even if that's not going to be the recommended way to do it (for good reason).

Mind you, rather than using [1] the fully-generic "override to an arbitrary device" mechanism, it would possibly be easier for the simplest serial case to just [2] add two more JSON option for mbed_retarget. If you look at the default serial behaviour in the absence of mbed_override_console, it now has 4 fundamental parameters - TX pin, RX pin, baud rate, buffered or not

Are you talking about making both of these methods work, or just one? If both, fine by me.

If just one, I'd be concerned if you choose [2], since IME using JSON files can limit mbed export behavior. (e.g. With mbed export -i mcuxpresso, the default generates both Debug and Release build configs in the project, but with a JSON file you can only have one. Though maybe it wouldn't in this case, IDK.) Also, method [2] wouldn't allow runtime redirection, AFAICT.

@kjbracey
Copy link
Contributor

kjbracey commented Feb 7, 2018

How will a user maintain line buffering on a redirected stdio with this method?

The FileHandle's isatty method should return 1, which should stop the C library from fully buffering. If you redirect to an actual disc file, that would be buffered because isatty is 0 for a file.

However, newlib-nano fails to call _isatty to check buffering, but instead hard-codes so that stdin/stdout/stderr are non-buffered, all other are buffered. There's a workaround in mbed_retarget.cpp so that fdopen() checks buffering, just as there was in Stream. But if you used plain fopen with a filename, you'd hit that issue.

Method [1] is definitely staying, I'm just saying we could extend the existing JSON for the built-in serial too. On the other hand I am wanting to avoid a pattern where every single potential device and its parameters (serial, SWO, Segger RTT) ends up in mbed_retarget.cpp.

Another possibility is something like a JSON option "retarget.console-device" = "XXX", and then XXX.cpp provides the "get_default_console" routine whenever MBED_CONF_RETARGET_CONSOLE_DEVICE == XXX. That could be extended to new devices without touching retarget.cpp.

Not familiar with that particular export behaviour - raise it as an issue?

@bmcdonnell-ionx
Copy link
Contributor Author

How will a user maintain line buffering on a redirected stdio with this method?

The FileHandle's isatty method should return 1, which should stop the C library from fully buffering. If you redirect to an actual disc file, that would be buffered because isatty is 0 for a file.

I see that UARTSerial (which inherits from FileHandle) returns 1 from its isatty function, so this should resolve my issue after #5571 is merged, since that includes mbed_override_console().

Not familiar with that particular export behaviour - raise it as an issue?

ARMmbed/mbed-cli#588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants