Mbed-os: There is no tests to verify that UART TX is complete

[LMESTM]

Doing some basic tests like (Corrected after comment from Martin):

Serial *pc = new Serial(USBTX, USBRX);
pc->printf("blabla");
deepsleep();
OR
pc.printf("blabla");
delete pc;

it looks like several targets may not send the complete serial Tx before entering sleep mode.
Proposal is to add a test that verifies this is done well.

[0xc0170]

How would you verify that? Shouldn't an application do flushing prior going to sleep?

[LMESTM]

As I read serial_api.h it says it's a blocking call, which means to me it's a synchronous API vs. the Async API that is being currently added.

So when serial_putc returns I expect this means transfer is complete. Any serial HW IP should let you know when the transmission is complete.

This avoids unpredictable behaviors and allows developers to get rid of here and there flush calls and/or wait() that were added after trial & errors debug sessions.

Note that I also haven't found a flush API in serial_api.h ...

[0xc0170]

Your example uses printf, without a newline. This is not connected to a serial API we provide. It's connected to the stdlib. stdout is buffered, therefore you need to flush ot, or use a newline (I dont think it's guaranteed there anyway - implementation defined).

You can disable buffering (setbuf(stdout, NULL);), or flush the buffer (fflush(stdout);)

[LMESTM]

Sorry - if you look at the example code in the commit,
I am actually using the Serial (and Stream.h) API, not the one from the stdlib:

The issue description / test case should rather be:

Serial *pc = new Serial(USBTX, USBRX);
pc->printf("blablarn"); // with or withou new line
deepsleep();
OR
pc->printf("blabla");
delete pc;

[0xc0170]

Yes, sorry, Serial anyway uses stream which implements it as:

int Stream::printf(const char* format, ...) {
    std::va_list arg;
    va_start(arg, format);
    fflush(_file);
    int r = vfprintf(_file, format, arg);
    va_end(arg);
    return r;
}

which is the same as I described. I assume you would propose after vfprintf do fflush() or another synch mechanism?

[LMESTM]

There may be several ways to solve the issue - the point is that the test does not pass if we're not changing anything now and I think that the test is valid and worth being added .

Now from implementation point of view:

• on STM32 I prefer to ensure that the transmission is complete when returning from serial_putc as the HW offers this information (and HAL reference implementation also checks this information, I feel it's better to be 100% reliable - performance will come with asynch and buffers transfers instead of single char transfers). => this I've done in separate PR.
• Adding a flush in the stream class is maybe a good idea as well, but I can't tell - better get feedback from stream developers.

[Sissors]

Changing overall putc to be completely blocking until done will seriously slow down some targets though.

[LMESTM]

This thread is just an issue, possible fixes is another matter ...
IMHO about performances so far: default baudrate is 9600 😕 and using byte per byte api is not meant for performance but for reliability and portability. Higher performance shall come with Dma based async api and/or moving to 115200kbps or higher .... in any case I only propose to update stm targets (no visible slowness so far) ... I would let other targets owners decide what they want to do

[adamgreen]

If the user is logging data which is small enough to fit in a FIFO and the FIFO would drain before the next chunk is logged then it is better to utilize the FIFO to minimize the impact of this logging on the rest of the code which is doing the actual work.

[Sissors]

In general since UART is an asynchronous protocol with independent reading and writing, the current putc command is even at its maximum speed just as fast as any DMA based solution.

Personally I see more in a function like serial.idle() next to readable/writable, or maybe a txDone interrupt.

[LMESTM]

All, thank you for your feedback. in order to converge towards a common conclusion, I'd like to separate the discussion into 2 separate threads.
1) this current thread we're discussing in is about adding a new "serial complete" test as proposed https://github.com/LMESTM/mbed/commit/7de8c210d44adc52d5b926605b3f9126a68c7100
So question in this thread is : do we think such a test should PASS and is needed ... or not ?
2) If we think the test is needed, another discussion is how to make it reliably work while maintaining performances. This can be discussed in #1801.

[ciarmcom]

ARM Internal Ref: IOTMORF-170

[bridadan]

@LMESTM The concern for being able to check if the serial hardware has finished sending all data has been raised again in #3132 and #2966.

This was the initial issue that brought it up, however there is quite a bit of discussion in here that is slightly unrelated.

The solution we proposed is to add a C HAL function that allows you to check if the UART hardware is finished sending all data (aka check if the FIFO is empty). In order to track this better, I'd like to create a new issue detailing the proposed fix and summarizing the discussions we've had across these PRs and issues. Do you think we can safely close this issue if this new issue is created?

[LMESTM]

@bridadan do you have the PR / Issue reference with request to add the C HAL function to check if UART HW is finished ?

[bridadan]

Hi @LMESTM it looks like this issue was never created. Would you like to add it?

[LMESTM]

@bridadan as the proposed solution comes from you or your team I'd prefer the issue or PR to come from you guys with your own description.

[bridadan]

I'll go ahead and close this since we're tracking the issue in #4408