sys/usb_cdc_acm_stdio: only submit and flush for non-empty buffer [backport 2024.10]

[benpicco]

Backport of #20986

Contribution description

There seems to be some problem the implementation of stdio over usbus_cdc_acm where printing an empty string sometimes leads to the next character being missed in the printout. I've managed to pin the problem down to the call to usbus_cdc_acm_flush(&cdcacm); in sys/usb/usbus/cdc/acm/cdc_acm_stdio.c and, more specifically, to _ep_xmit in cpu/nrf5x_common/periph/usbdev.c, but since I have no non-nRF hardware at hand, I cannot tell whether it is a nRF specific issue.

In any case, skipping the flushing (and submitting) in the first place seems to fix the issue for now. If anyone has an idea how to investigate this further and maybe fix the root cause somewhere deeper in the usb driver, please tell me, I'm happy to test more.

Testing procedure

1. apply the following diff:

diff --git a/tests/sys/usbus_cdc_acm_stdio/main.c b/tests/sys/usbus_cdc_acm_stdio/main.c
index c603882121..50c0b9a9d8 100644
--- a/tests/sys/usbus_cdc_acm_stdio/main.c
+++ b/tests/sys/usbus_cdc_acm_stdio/main.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 
 #include "shell.h"
+#include "fmt.h"
 
 static int cmd_text(int argc, char **argv)
 {
@@ -30,7 +31,7 @@ static int cmd_text(int argc, char **argv)
         return -1;
     }
     int n = atoi(argv[1]);
-    if (n <= 0) {
+    if (n < 0) {
         puts(usage);
         return -1;
     }
@@ -40,7 +41,9 @@ static int cmd_text(int argc, char **argv)
         }
         putc('0' + (i % 10), stdout);
     }
-    puts("");
+    print_str("");
+    print_str("x");
+    print_str("\n");
     return 0;
 }

2. pick any (nrf-based?) board using stdio_cdc_acm such as feather-nrf52840-sense
3. make -C tests/sys/usbus_cdc_acm_stdio BOARD=feather-nrf52840-sense flash term
4. repeatedly enter text 0

On master, this sometimes skips over the x in the printout. With this PR, I never managed to reproduce the issue.

Issues/PRs references

Found while investigating failing tests for #20980

[mguetschow]

Thanks for the backport!

[riot-ci]

Murdock results

✔️ PASSED

ed00724 sys/usb_cdc_acm_stdio: only submit and flush for non-empty buffer

Success	Failures	Total	Runtime
10215	0	10215	19m:58s

Artifacts

• Documentation preview

[dylad]

Looks like CI has some troublesome with this one. That's weird.

[benpicco]

When the CI queue gets too long, the GitHub merge queue times out.
I wonder if this is configurable somewhere

[dylad]

This PR is cursed :/ What I don't understand is why this one is failing (taking so long) but not the one which landed in master ?

[maribu]

The timeout is 360 minutes (maximum timeout supported by GitHub). One PR takes about 3h 30 min to be merged. Hence, the moment another PR is in front of the queue, it will not get merged.

But since the last Murdock run succeeded, we could rerun with compilation tests, as lomg as nothing else gets merged in between.

[maribu]

It is now at the head of the merge queue, so the recent passing CI run will still be valid when this gets in.

For reference, here is the output of the last full Murdock run: https://ci.riot-os.org/details/dded58edea91403bb7a189410e01d4fe

[maribu]

So much for skipping compile tests. But this time - assuming now flaky tests fail - it should pass Murdock before the 360 mins have passed.

[mguetschow]

Hallelujah 🎉