Deprecate Stream and Serial

[kjbracey]

Serial is replaced by UARTSerial - this provides the necessary buffers
to use serial reliably for input from non-interrupt contexts, and avoid
excess spinning waiting for TX buffer space.

(Historically, anyone using Serial but needing input has done so by
bypassing its stdio-based calls, and instead making calls
to the underlying SerialBase from interrupt, as the stdio input calls
do not work reliably from threads due to lack of buffering, and can't
be used from interrupts. This reliance on the underlying implementation
means it wasn't possible to add buffering in a backwards-compatible
fashion, hence the distinct UARTSerial).

Because it no longer uses Stream, use of UARTSerial no longer
necessarily forces pulling in the C library stdio system.

Stream is replaced by drivers implementing FileHandle directly, and
applications using fdopen(FileHandle *) to get a FILE * to use full
C/C++ stream features. This avoids being locked into an odd design
pattern whereby the class works at 2 layers:

• Application - uses Stream-based driver
• Stream - uses <stdio.h> FILE
• C library - Implements FILE, uses FileHandle
• Stream - implements FileHandle, uses virtual _getc/_putc
• Derived-from-Stream (eg Serial) - implements _getc/_putc

Stream's implementation of FileHandle on a simple blocking _getc/_putc
cannot be usefully extended to the fuller FileHandle API required for
real-life use like PPP or ATCmdParser.

The revised setup now looks like:

• Application - uses FILE
• C library - Implements FILE, uses FileHandle
• Driver - Implements FileHandle

What we lose is the syntactical sugar that allowed you to do

Serial serial(RX, TX, 115200);
serial.printf("Hello");

Instead, you now must do

UARTSerial serial(RX, TX, 115200);
FILE *out = fdopen(&serial, "w");
fprintf(out, "Hello");

[kjbracey]

Attempt to complete tidy-up of the serial classes, by deprecating the classes that UARTSerial was designed to replace.

[kjbracey]

Related PRs: #5571 (mbed_retarget), #5446 (trying to use Serial from interrupt).

[sg-]

On hold pending further investigation

[bmcdonnell-ionx]

I referenced this PR in issue #6019, but the pingback didn't show here (for whatever reason), so I'm leaving this comment.

[bmcdonnell-ionx]

#5571 has been merged into Mbed OS 5.8.0 RC1.

If the plan is still to deprecate these, I think it would be nice if you can get it in to Mbed OS 5.8.0, to help users discover that they should use UARTSerial instead of Serial, etc. (There are a lot of examples around using Serial...)

[kjbracey]

Still subject to discussions - the fact so many people are using Serial is why this is a bone of contention (how can something so popular and widely-used be bad?). Also, if you're going to deprecate, got to be confident that you're 100% happy with the proposed replacement.

Latest thoughts were

• making RawSerial also be a FileHandle - removes the need for DirectSerial inside the retarget code, improving system symmetry
• restore UARTSerial to its original name of BufferedSerial to improve clarity
• make both safe(r) to use from trickier IRQ/exception-like contexts (eg for mbed_error crashes)
• investigate whether there should be a Stream replacement for the "C++-flavoured" XXX.printf() and what it would look like.
• RawSerial::printf is current special-cased to be safe(ish) to use from IRQ - fprintf(fdopen(RawSerial)) wouldn't be due to C library file mutexes. Need an alternative.

(Although another discussion was suggesting a "Serial"->"UART" conversion generally including the HAL, so it might be RawUART and BufferedUART. Whatever, the Raw/Buffered distinction is what's important).

Won't be happening for 5.8, and still not on the roadmap for 5.9 - only a very small proportion of developers seem to notice the Serial performance issues, as they only really use it for logging, so this is not getting much attention. Over here #5965 is busy working on logging, and has been actually relying on Serial's non-yielding behaviour.

[bmcdonnell-ionx]

Still subject to discussions - the fact so many people are using Serial is why this is a bone of contention (how can something so popular and widely-used be bad?).

Deprecation may be appropriate in a situation when there is a new and improved version, even when the original isn't bad. Right?

Maybe deprecation isn't the right thing to do now. If not, hopefully you have a plan to determine if/when it will be...

OTOH, why do both UARTSerial and Serial exist? Why not just improve Serial? (I suspect you have good reason; I just don't know it. Apologies if you covered that before; feel free to link.)

Also, if you're going to deprecate, got to be confident that you're 100% happy with the proposed replacement.

Good point here.

Related: #6295.

Won't be happening for 5.8, and still not on the roadmap for 5.9 - only a very small proportion of developers seem to notice the Serial performance issues, as they only really use it for logging, so this is not getting much attention.

I can't recall performance issues with it, either. My issue was wanting proper stdio redirection without changing the buffering. I'm happy to say that seems to be working fine now with UARTSerial and mbed_override_console() in 5.8.0 RC1. 😄

[kjbracey]

OTOH, why do both UARTSerial and Serial exist? Why not just improve Serial? (I suspect you have good reason; I just don't know it. Apologies if you covered that before; feel free to link.)

Original description on this PR goes through some of it - main blocker is that anyone who's managed to do reliable input on Serial must have gone deep on accessing its SerialBase internals - I've seen a lot of such code - we couldn't change its implementation enough to make it even optionally buffered without breaking too much stuff.

And the Stream it's based on is conceptually broken, not least because it uses 1 simplex FILE for a duplex FileHandle, so has to put a fflush on every op in case of direction changes, and its FileHandle::read() works fundamentally wrongly. (read(buffer, len) won't return until you've received len bytes).

Therefore, we made the buffered serial a complete new implementation. The proposed future symmetry backfilling is to also make RawSerial a working FileHandle - it will retain the inherent performance problems of Serial, but could otherwise work correctly enough to be useful with care, including from IRQ and exceptions.

Serial's FileHandle was only ever intended to be private (but it isn't) to implement Stream, and what it does is good enough for C library stdout, but not for more advanced stuff like AT parsers and PPP data pumps.

Basically anyone currently using Serial is either doing simple stuff from thread, in which case they should switch to UARTSerial - or even better just stdin and stdout so they're console-independent - or they're doing complex fiddly stuff from interrupts, in which case they should be using RawSerial, or find they no longer need to do fiddly stuff and use UARTSerial. Sometimes they're doing a hybrid - thread for output, IRQ for input, so using Serial for output and its underlying SerialBase for input. No way we can transparently switch all those people.

Original commit adding UARTSerial (under the name BufferedSerial): 2790d44

[cmonr]

@kjbracey-arm @ARMmbed/mbed-os-maintainers Is there any value in keeping this open considering the conversation has become stale?

[bmcdonnell-ionx]

Am I interpreting the following correctly?

In the following cases, deprecating Stream and Serial should be...

Basically anyone currently using Serial is either doing simple stuff from thread, in which case they should switch to UARTSerial - or even better just stdin and stdout so they're console-independent

...fine - a helpful prompt to switch.

or they're doing complex fiddly stuff from interrupts, in which case they should be using RawSerial

...OK? Can they just drop RawSerial in place of Serial?

or find they no longer need to do fiddly stuff and use UARTSerial.

...fine.

Sometimes they're doing a hybrid - thread for output, IRQ for input, so using Serial for output and its underlying SerialBase for input.

...I guess this is the tough one. Could there be a recommended migration plan here?

No way we can transparently switch all those people.

[kjbracey]

@cmonr Had some discussions with @sg- and other folks - we're going to do something - but I think we can park this PR for now.

@bmcdonnell-ionx - re the fiddly case.

Could there be a recommended migration plan here?

Depends what they're doing. Maybe the straight buffering of UARTSerial is sufficient, so they can switch to that. Or maybe they want to do some sort of deframing from the interrupt context, in which case they need to keep using RawSerial and interrupts.

A new issue has recently come to my attention - serial interrupt handlers (and hence UARTSerial) are incompatible with deep sleep - the assumption is that if a serial interrupt handler is installed, deep sleep needs to be locked.

We need to address that in 2 ways:

1. platforms that can wake for serial in deep sleep need a way of flagging that,
2. and we need a way of specifying "unidirectional open" for UARTSerial so we can get the benefit of buffered output without also installing the interrupt handler for buffered input and potentially unnecessarily killing deep sleep.

[kjbracey]

Oh, and my point was that if we do do unidirectional open primarily for the sleep issue, then conceivably you could legally do RawSerial interrupt input and UARTSerial thread-buffered output without restructuring a weird app.

[kjbracey]

Quick update on two of the things-to-do mentioned above, that have been done:

• UARTSerial::write is now safe(ish) to call from exception/interrupt - it falls back to a synchronous output, making it okay for fault handlers, at least.
• We now have enable_input and enable_output APIs, which effectively gives us unidirectional open for power saving purposes.