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.

Sign up for GitHub

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

Jump to bottom

tests: Add test for irq_disable() / irq_restore() #14011

Merged
merged 1 commit into from
May 4, 2020
Merged

tests: Add test for irq_disable() / irq_restore() #14011

merged 1 commit into from
May 4, 2020

Conversation

maribu
Copy link
Member

@maribu maribu commented May 4, 2020

Contribution description

As the title suggests: Adds a stupidly simple test to check if irq_disable() and irq_restore() work as intended.

Testing procedure

BOARD=<SOME_BOARD> make -C tests/irq_disable_restore flash term

Issues/PRs references

Useful to test #13999

@maribu maribu added Area: tests Area: tests and testing framework Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 4, 2020
@maribu maribu requested a review from fjmolinas May 4, 2020 08:15
@maribu
Copy link
Member Author

maribu commented May 4, 2020

Unless I overlooked something, we had no test direct coverage for those two functions before. (Still, chances are good that every second tests fails if irq_disable() and irq_restore() won't work. But having a stupidly simple test that directly checks those two functions might be useful.)

@fjmolinas fjmolinas added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label May 4, 2020
@fjmolinas
Copy link
Contributor

Will test on a bunch of boards ASAP.

@fjmolinas
Copy link
Contributor

I have an issue with fputs on native:


Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2020.07-devel-378-g90618-pr-14011)
Test for irq_disable() / irq_restore()
======================================

[SUCCESS]
[SUCCESS]
Timeout in expect script at "child.expect_exact("Verifying test works: [SUCCESS]")" (tests/irq_disable_restore/tests/01-run.py:14)

@fjmolinas
Copy link
Contributor

I have a compilation error on msp430 boards, I don't really understand it..

BUILD_IN_DOCKER=1 BOARD=z1 make -C tests/irq_disable_restore/

/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:212:9: error: expected specifier-qualifier-list before ‘int_least8_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:213:9: error: expected specifier-qualifier-list before ‘uint_least8_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:214:9: error: expected specifier-qualifier-list before ‘int_least16_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:215:9: error: expected specifier-qualifier-list before ‘uint_least16_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:216:9: error: expected specifier-qualifier-list before ‘int_least32_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:217:9: error: expected specifier-qualifier-list before ‘uint_least32_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:218:9: error: expected specifier-qualifier-list before ‘int_least64_t’
/data/riotbuild/riotbase/cpu/msp430_common/include/stdatomic.h:219:9: error: expected specifier-qualifier-list before ‘uint_least64_t’
...

@maribu maribu mentioned this pull request May 4, 2020
Merged
@maribu
Copy link
Member Author

maribu commented May 4, 2020

I have an issue with fputs on native:

I worked around this by using fmt for output. Using full blown printf() would increase ROM size quite a bit, so fmt seems to be the best option.

@fjmolinas
Copy link
Contributor

Tested on these boards, only z1 failed, will be fixed with #14012.

nrf52840-mdk nrf52840dk slstk3402a nucleo-f091rc hifive1b remote-revb b-l072z-lrwan1 z1 nucleo-l152re nucleo-l433rc ek-lm4f120xl nucleo-f103rb i-nucleo-lrwan1 arduino-uno nucleo-f410rb arduino-zero frdm-k64f nucleo-f303re b-l475e-iot01a openmote-b stm32l476g-disco iotlab-m3 pba-d-01-kw2x nucleo-f207zg nrf52dk arduino-mega2560 nucleo-l073rz nucleo-f767zi

@fjmolinas
Copy link
Contributor

@maribu once #14012 is in please rebase and squash (and trigger murdock)!

@fjmolinas
Copy link
Contributor

@maribu please rebase!

@maribu maribu force-pushed the tests_irq_disable_restore branch from f59c583 to d3c3bac Compare May 4, 2020 10:03
@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 4, 2020
@maribu
Copy link
Member Author

maribu commented May 4, 2020

Looks like with llvm somehow stdint.h is not included implicitly with stdatomic.h. I'd say that newlib's stdatomic.h missing an #include <stdint.h> - but that would have to be fixed upstream. I work around this by just adding the include in main.c. I'll squash right away.

@maribu maribu force-pushed the tests_irq_disable_restore branch from d3c3bac to 51c19d5 Compare May 4, 2020 10:52
@maribu
Copy link
Member Author

maribu commented May 4, 2020

OK, in the log of test application that could be compiled a hint is given:

tests/libc_newlib: blacklist esp32 boards temporarily

ESP32 boards must be blacklisted for tests/libc_newlib to avoid CI compilation errors until the toolchain is updated on all nodes.

So I guess a toolchain update is still pending on

-- running on worker breeze thread 11, build number 283085.

@fjmolinas
Copy link
Contributor

So I guess a toolchain update is still pending on

-- running on worker breeze thread 11, build number 283085.

@kaspar030 @cgundogan @aabadie @riot-ci can you take a look?

@fjmolinas
Copy link
Contributor

@maribu if its only one worker we could just re-schedule and hope for the best :D

@fjmolinas fjmolinas mentioned this pull request May 4, 2020
Merged
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 4, 2020
@maribu
Copy link
Member Author

maribu commented May 4, 2020

The single failed test looks serious, but completely unrelated.

@fjmolinas
Copy link
Contributor

fjmolinas commented May 4, 2020 •
edited
Loading

The single failed test looks serious, but completely unrelated.

FYI @gschorcht (don't have any boards to try and replicate :( )

@fjmolinas fjmolinas 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 May 4, 2020
@fjmolinas
Copy link
Contributor

Ajj we got the faulty worker again.

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 4, 2020
@maribu
Copy link
Member Author

maribu commented May 4, 2020

Ajj we got the faulty worker again.

Let's roll the dice again :-D

fjmolinas
fjmolinas approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@fjmolinas fjmolinas merged commit dbab3a6 into RIOT-OS:master May 4, 2020
@maribu
Copy link
Member Author

maribu commented May 4, 2020

Thanks for the review :-)

@maribu maribu deleted the tests_irq_disable_restore branch May 4, 2020 14:11
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@smlng smlng Awaiting requested review from smlng

Assignees
No one assigned
Labels
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 Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2020.07
Development

Successfully merging this pull request may close these issues.
3 participants
@maribu @fjmolinas @miri64