tests/stdin: fix automatic test on slow platforms

[aabadie]

Contribution description

This PR fixes the stdin tests on slow plaform. It could have been fixed with #12461 but as @fjmolinas told me IRL, it's a bit weird to use the feature tested by the test to sync the test.

The solution is proposed by this PR is then to add a small delay in the Python test script before it send the character that is testing stdin.

Testing procedure

The following command should succeed on supported platforms, including AVR based boards:

make BOARD=<board of your choice> -C tests/stdin flash test

Issues/PRs references

Could have been added to #12651

[aabadie]

The issue reported by Murdock on nrf52dk is unrelated.

[kaspar030]

Wouldn't it be more stable to wait for a trigger?

It is clear that the python script should not try to send O before uart has been initialized. Maybe print in main before "getchar()" and try synchronizing on that?

Waiting one second might fail depending on test machine load. On platforms that can be reset while the terminal is open (e.g., all CI boards), all output after reset is captured.

[aabadie]

Maybe print in main before "getchar()" and try synchronizing on that?

This was indeed the first option I tried (and it worked). But after discussing this IRL with @fjmolinas, we thought the small delay was simpler (I don't remember the exact reasons in fact).

[kaspar030]

This was indeed the first option I tired (and it worked).

Ok. Let's see if this solves it on esp32 (all other CI boards are fast enough).

[aabadie]

Let's see if this solves it on esp32

It seems that it's the case: #11449 (comment)

[fjmolinas]

This was indeed the first option I tried (and it worked). But after discussing this IRL with @fjmolinas, we thought the small delay was simpler (I don't remember the exact reasons in fact).

We had talked about not using test_utils_interactive_sync for this since it meant using stdin in a test for syncing in a test that is testing stdin.

The issue with syncing on a print is that if the application prints before the python script is capturing it will never sync correctly. A better fix might be to try multiple times to send the character, same as in test_utils_interactive_sync, which makes me think maybe calling tests_utils_interactive_sync can be the test? Including it explicitly so it doesn't seem like it has an empty main :).

[fjmolinas]

@aabadie thoughts on my latest comment? (since you re-requested the review)

[aabadie]

thoughts on my latest comment?

Yes, 2:

1. We can try to send the character multiple times like it's done in tests_utils_interactive_sync
2. Do we really need this test now ?

Maybe we could go for 1. since it will really test the feature, with a very minimal setup.

[fjmolinas]

Do we really need this test now ?

Instead of the current status? I just think its a better fix

[aabadie]

Instead of the current status?

I meant do we really need tests/stdin, since tests_utils_interactive_sync is already testing the same feature in almost all the other tests.

[fjmolinas]

I meant do we really need tests/stdin, since tests_utils_interactive_sync is already testing the same feature in almost all the other tests.

I think you do, there is not test for tests_utils_interactive_sync. Taking it to a extreme it would be like saying why test xtimer when so many application use xtimer so it is already test. IMO tests should as much as possible tests functionalities in isolation. If those functionalities are proven you can build on top of these.

[aabadie]

Ok let's do as you suggested, e.g. point 1.

[kaspar030]

Whats the state here? #11449 is waiting for this.

[kaspar030]

Whats the state here? #11449 is waiting for this.

We could merge as is and improve later.

[fjmolinas]

We could merge as is and improve later.

I only commented on the PR, no changes request, so it is @aabadie call.

[aabadie]

During my last tests on arduino-uno, I wasn't able to reproduce the issue, even on master. But maybe that was something weird with the fuses. I need to investigate this more but won't have the hardware with me before next week.

[fjmolinas]

@kaspar030 is this fixing the issue for esp32?

[fjmolinas]

During my last tests on arduino-uno, I wasn't able to reproduce the issue, even on master. But maybe that was something weird with the fuses. I need to investigate this more but won't have the hardware with me before next week.

@kaspar030 is this fixing the issue for esp32?

If it does we could just go as is just fix the commit message. I would still like the test to be improved, but it can be done later.

[kaspar030]

@kaspar030 is this fixing the issue for esp32?

yes!

[aabadie]

I pushed an update that makes the test even more robust. It should work on AVR (couldn't test yet) and works well on esp32-wroom-32 and hifive1b (also broken on master).

If @fjmolinas is fine with the current state, I think this PR can be merged.

[fjmolinas]

I haven't tested, but changes look.

[fjmolinas]

Tested on arduino-uno:

Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-12-05 13:09:05,376 # Connect to serial port /dev/riot/tty-arduino-uno
Welcome to pyterm!
Type '/exit' to exit.
2019-12-05 13:09:06,381 # T! (Version: 2020.01-devel-1157-ga4c3d-pr-12816)
2019-12-05 13:09:06,962 # 

2019-12-05 13:09:07,031 # main(): This is RIOT! (Version: 2020.01-devel-1157-ga4c3d-pr-12816)
2019-12-05 13:09:08,375 # You entered 'O'

make: Leaving directory '/home/francisco/workspace/RIOT-test/tests/stdin'

[fjmolinas]

@kaspar030 can you test on esp32?

[kaspar030]

@kaspar030 can you test on esp32?

works fine on esp32-wroom-32!

[fjmolinas]

ACK!