Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boards: common: stdio_cdc_acm: let tests wait a bit for serial port #19128

Merged
merged 2 commits into from
Jan 12, 2023

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 11, 2023

Contribution description

Some tests may fail spuriously with stdio_cdc_acm, since the tty might not exist yet. This configures a small delay after resetting for testutils and also piggybacks the honoring of that delay for the riotctrl-based tests.

Testing procedure

CI should pass all tests. Maybe we should also run tests-on-iotlab for this PR.

Issues/PRs references

None

@miri64 miri64 requested a review from maribu January 11, 2023 12:00
@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: tests Area: tests and testing framework labels Jan 11, 2023
@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jan 11, 2023
@miri64
Copy link
Member Author

miri64 commented Jan 11, 2023

Maybe we should also run tests-on-iotlab for this PR.

See https://github.com/RIOT-OS/RIOT/actions/runs/3892598987. The one to look for is nrf52840dk.

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 11, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@riot-ci
Copy link

riot-ci commented Jan 11, 2023

Murdock results

✔️ PASSED

64bea31 tests/congure_*: wait for serial port after reset if configured

Success Failures Total Runtime
6770 0 6770 13m:17s

Artifacts

@miri64
Copy link
Member Author

miri64 commented Jan 12, 2023

The ESP issues seem unrelated but they show up :-/.

@miri64
Copy link
Member Author

miri64 commented Jan 12, 2023

@miri64 miri64 force-pushed the stdio_cdc_acm/enh/tests-wait branch from cc83f44 to 64bea31 Compare January 12, 2023 09:14
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 12, 2023
@miri64
Copy link
Member Author

miri64 commented Jan 12, 2023

Deactivated tests for now. The tests this touches worked and the failing tests fail in master as well.

@aabadie
Copy link
Contributor

aabadie commented Jan 12, 2023

bors merge

bors bot added a commit that referenced this pull request Jan 12, 2023
19128: boards: common: stdio_cdc_acm: let tests wait a bit for serial port r=aabadie a=miri64



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build failed:

@miri64
Copy link
Member Author

miri64 commented Jan 12, 2023

bors retry

@miri64
Copy link
Member Author

miri64 commented Jan 12, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Already running a review

bors bot added a commit that referenced this pull request Jan 12, 2023
19128: boards: common: stdio_cdc_acm: let tests wait a bit for serial port r=aabadie a=miri64



19133: makefiles/utils/strings.mk: Fix version_is_greater_or_equal r=maribu a=maribu

### Contribution description

The Makefile function `version_is_greater_or_equal` is used to check if a version of GNU Make is at least the required one. However, it has the built-in assumption the version numbers have to format x.y.z, but Alpine Linux currently ships GNU Make 4.4. This results in `$(call _pad_number,3,)` which runs `printf '$03d' ''` in the shell, which is not valid.

This fixes the issue by making `_pad_number` more robust by fall back to printing `0` with the given padding, if the number given to print is empty.

### Testing procedure

Append

```Makefile

$(info A=$(call version_is_greater_or_equal,4.2.0,4.2.0))
$(info B=$(call version_is_greater_or_equal,4.2,4.2.0))
$(info C=$(call version_is_greater_or_equal,4.1,4.2.0))
$(info D=$(call version_is_greater_or_equal,4.1.9,4.2.0))
$(info E=$(call version_is_greater_or_equal,5.1.9,4.2.0))
$(info F=$(call version_is_greater_or_equal,5.0.0,4.2.0))
$(info G=$(call version_is_greater_or_equal,4.2.1,4.2.0))
$(info H=$(call version_is_greater_or_equal,4.3.1,4.2.0))
```

e.g. to `makefiles/utils/strings.mk`, build something and observe the info output.

This yields

```
A=1
B=1
C=
D=
E=1
F=1
G=1
H=1
```

for me and does not complain about invalid `printf` invocations.

### Issues/PRs references

None

Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Jan 12, 2023
19128: boards: common: stdio_cdc_acm: let tests wait a bit for serial port r=aabadie a=miri64



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build failed:

@maribu
Copy link
Member

maribu commented Jan 12, 2023

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build succeeded:

@bors bors bot merged commit 1dcccdc into RIOT-OS:master Jan 12, 2023
@miri64 miri64 deleted the stdio_cdc_acm/enh/tests-wait branch January 13, 2023 07:35
@kaspar030 kaspar030 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 13, 2023
bors bot added a commit that referenced this pull request Jan 13, 2023
19143: boards: common: stdio_cdc_acm: let tests wait a bit for serial port [backport 2023.01] r=kaspar030 a=kaspar030

# Backport of #19128



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
bors bot added a commit that referenced this pull request Jan 13, 2023
19143: boards: common: stdio_cdc_acm: let tests wait a bit for serial port [backport 2023.01] r=kaspar030 a=kaspar030

# Backport of #19128



19144: shell/rtc: Fix out of bounds access; document error behavior [backport 2023.01] r=kaspar030 a=kaspar030

# Backport of #19141

### Contribution description



### Testing procedure

Should be trivial enough, especially as the difference is hard to spot interactively.

On native, run the default example (and wait for the traffic to settle).

Then, run

```
> rtc poweron
> rtc settime 2022-01-01 00:00:00
> rtc settime 2022-99-01 00:00:00
```

Both still work, but the latter doesn't access unassigned memory any more

### Issues/PRs references

This fixes an issue that was submitted anonymously.

Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
Co-authored-by: chrysn <chrysn@fsfe.org>
bors bot added a commit that referenced this pull request Jan 13, 2023
19143: boards: common: stdio_cdc_acm: let tests wait a bit for serial port [backport 2023.01] r=kaspar030 a=kaspar030

# Backport of #19128



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
bors bot added a commit that referenced this pull request Jan 14, 2023
19143: boards: common: stdio_cdc_acm: let tests wait a bit for serial port [backport 2023.01] r=kaspar030 a=kaspar030

# Backport of #19128



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Feb 4, 2023
After a reset, it can take several seconds before the CDC ACM interface becomes available as serial device `ttyACMx`. This was a change that was already made for `stdio_cdc_acm` in RIOT-OS#19128.
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Feb 4, 2023
After a reset, it can take several seconds before the CDC ACM interface becomes available as serial device `ttyACMx`. This was a change that was already made for `stdio_cdc_acm` in RIOT-OS#19128.
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Feb 6, 2023
After a reset, it can take several seconds before the CDC ACM interface becomes available as serial device `ttyACMx`. This was a change that was already made for `stdio_cdc_acm` in RIOT-OS#19128.
@jia200x jia200x added this to the Release 2023.04 milestone Apr 25, 2023
zhaolanhuang pushed a commit to zhaolanhuang/RIOT that referenced this pull request Dec 6, 2023
After a reset, it can take several seconds before the CDC ACM interface becomes available as serial device `ttyACMx`. This was a change that was already made for `stdio_cdc_acm` in RIOT-OS#19128.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants