Remove wait in MBED_11 test (mbed 2)

[bcostm]

Description

Remove the wait loop present in the main function for the MBED_11 test for mbed 2

• This test was failed/unstabled on our test bench since mbed revision 153 / mbed-os-5.6.2
• Removing this wait loop makes the MBED_11 test to be PASS again.
• The MBED_34 and MBED_23 OS2 tests don't have this wait loop.

Status

READY

Migrations

NO

ST_INTERNAL_REF 39289

[0xc0170]

This test was failed/unstabled on our test bench since mbed revision 153 / mbed-os-5.6.2

Why did it become unstable? How wait(1) affects the rest of the code?

[bcostm]

How wait(1) affects the rest of the code?
This is exactly the question I'm asking to myself...
cc @ticker-experts-team

[bcostm]

I checked different values passed to the wait function:
0.01 FAIL
0.1 OK
0.2 FAIL
0.5 FAIL
1.0 FAIL
0.1 + 1.0 FAIL
5.0 OK
10.0 OK
10.1 FAIL...

[0xc0170]

cc @ticker-experts-team

@mprse Can you check this test, and confirm that wait should be fine to have (in mbed 2 context, this is a blocking call)

[bcostm]

MBED_11 test log I have with wait(1.0):

IAR

* in 0.99 sec (0.01) [OK]
* in 1.00 sec (0.00) [OK]
* in 1.01 sec (0.01) [OK]
* in 1.00 sec (0.00) [OK]
* in 0.97 sec (0.03) [OK]
* in 1.03 sec (0.03) [OK]
* in 1.00 sec (0.00) [OK]
* in 0.98 sec (0.02) [OK]
* in 1.01 sec (0.01) [OK]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.02 sec (0.98) [FAIL]
* in 0.00 sec (1.00) [FAIL]

GCC_ARM

* in 1.00 sec (0.00) [OK]
* in 1.00 sec (0.00) [OK]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.01 sec (0.99) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]
* in 0.00 sec (1.00) [FAIL]

ARM

* in 0.98 sec (0.02) [OK]
* in 1.02 sec (0.02) [OK]
* in 1.01 sec (0.01) [OK]
* in 1.00 sec (0.00) [OK]
* in 1.00 sec (0.00) [OK]
* in 1.00 sec (0.00) [OK]
* in 0.98 sec (0.02) [OK]
* in 1.00 sec (0.00) [OK]
* in 1.00 sec (0.00) [OK]
* in 1.02 sec (0.02) [OK]

[0xc0170]

This test is for mbed 2, no rtos involved. Printf in ISR is bad practice, therefore if we can rewrite print_char to rather putc instead? How would that change things?

Leaving wait() there, we read time almost constantly from the ticker. There is an interrupt happening regularly. I suspect reading is causing a problem here. The test without wait() would be fine, but there should be also wait (or at least constantly reading time, to test how ticker is handling reading/interrupt handling) ?
This has been long there and tested on plenty of devices, @bcostm have you experienced this bug with other targets? I do not see what targets have been tested

What are the failures?

[bcostm]

to rather putc instead? How would that change things?

As suggested I have replaced the print_char with pc.putc('*'); fflush(stdout); (and add Serial pc(USBTX, USBRX);) and I kept wait(1.0) in the main and the test is OK now for IAR and GCC.

I use a NUCLEO_L476RG board but this issue has been seen on all of our boards.

So, do you want that I put back the wait and replace the print_char in this PR ? Or do I close it and you (ARM) modify the test ?

Edit: I tested again with wait(0.5) and still FAIL...

[jeromecoutant]

I agree that, in no RTOS (OS2), wait function should not be called

[0xc0170]

I agree that, in no RTOS (OS2), wait function should not be called

@jeromecoutant Wait should not matter and should be functional though, thus I suspect removing it we are hiding a potential bug in a target's ticker code (read the time and execute a ticker interrupt)

[jeromecoutant]

For information, issue is not related to STM32 only:

| FAIL | K64F | ARM | MBED_11 | Ticker Int | 3.38 | 15 | 0/1 |
| FAIL | K64F | GCC_ARM | MBED_11 | Ticker Int | 5.41 | 15 | 0/1 |
| OK | K64F | IAR | MBED_11 | Ticker Int | 11.42 | 15 | 1/1 |

With patch:

| OK | K64F | ARM | MBED_11 | Ticker Int | 11.38 | 15 | 1/1 |
| OK | K64F | GCC_ARM | MBED_11 | Ticker Int | 11.36 | 15 | 1/1 |
| OK | K64F | IAR | MBED_11 | Ticker Int | 11.34 | 15 | 1/1 |

[0xc0170]

I noticed the above exchange Serial still does not fix the problem. I reproduced this issue locally. Fix it, as above mentioned, use RawSerial from interrupt handler callback.

diff --git a/features/unsupported/tests/mbed/ticker/main.cpp b/features/unsupported/tests/mbed/ticker/main.cpp
index fc1ed9ca3..a710ae4fa 100644
--- a/features/unsupported/tests/mbed/ticker/main.cpp
+++ b/features/unsupported/tests/mbed/ticker/main.cpp
@@ -1,10 +1,11 @@
 #include "mbed.h"
 #include "test_env.h"

+RawSerial pc(USBTX, USBRX);
+
 void print_char(char c = '*')
 {
-    printf("%c", c);
-    fflush(stdout);
+    pc.putc(c);
 }

 Ticker flipper_1;

Tested with K64F and GCC - OK

Can you confirm?

[0xc0170]

@bcostm Can you test my proposal?

[bcostm]

Can you test my proposal?

Yes I'm going to. I jumped on the UART IRQ issue and didn't have time to test your proposal yet.

[bcostm]

I confirm that the use of RawSerial fixes the problem. I have tested it with ARM, IAR and GCC several times. I have also changed the delay in the while loop and it is pass all the time. 👍

How do we proceed now ? I think I should close this PR and I (or someone else) should create another one to change the RawSerial.

[0xc0170]

How do we proceed now ? I think I should close this PR and I (or someone else) should create another one to change the RawSerial.

thanks !
You can amend this PR to change it to RawSerial or if is easier to create a new one. Will be good t o have this fixed 👍

[bcostm]

I close it and I'll open a new one.