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/periph_timer: also test for spurious IRQs #18963

Merged
merged 2 commits into from
Dec 8, 2022
Merged

tests/periph_timer: also test for spurious IRQs #18963

merged 2 commits into from
Dec 8, 2022

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 24, 2022

Contribution description

Previously the test only checked if IRQs fired when expected. This extends the test to also check that IRQs do not fire when not expected.

In addition, some long overdue style / misuse of volatile fixes are added as separate commit.

Testing procedure

The test app should pass on correct timer drivers (e.g. samr21-xpro), and fail on broken ones (e.g. nucleo-f767zi).

Issues/PRs references

None

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Nov 24, 2022
@maribu maribu 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 Nov 24, 2022
@riot-ci
Copy link

riot-ci commented Nov 24, 2022 •
edited
Loading

Murdock results

✔️ PASSED

496abf0 tests/periph_timer: also test for spurious IRQs

Success Failures Total Runtime
674 0 674 02m:23s

Artifacts

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Nov 24, 2022
@maribu maribu force-pushed the tests/periph_timer branch from 4787cc2 to e1d2d1b Compare November 24, 2022 14:06
@maribu maribu requested a review from gschorcht as a code owner November 24, 2022 19:35
@github-actions github-actions bot added the Platform: ESP Platform: This PR/issue effects ESP-based platforms label Nov 24, 2022
@kaspar030 kaspar030 mentioned this pull request Nov 24, 2022
Closed
@kaspar030
Copy link
Contributor

(IMO the periph fixes should not in a tests/periph_timer PR but split out)
After an hour or so debugging and reading the reference manual, I came to the same fix for nrf5x independently.

This was referenced Nov 24, 2022
Merged
Merged
Merged
@maribu
Copy link
Member Author

maribu commented Nov 24, 2022

Agreed. It does make testing easier though, if the CI runs the tests along with the fixes. When this is rebased on master once the split out fixes are merged, this will contain only the test.

@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 Nov 24, 2022
This was referenced Nov 25, 2022
Merged
Merged
Merged
tests/periph_timer: fix style and use of volatile
e7ee53e 
To synchronize communication via shared memory between ISR context and
thread context it is a common misconception that `volatile` is
sufficient. This is however is not the cause and the cause of many
subtle data race bugs. This fixes the issue.
tests/periph_timer: also test for spurious IRQs
496abf0 
Previously the test only checked if IRQs fired when expected. This
extends the test to also check that IRQs do not fire when not expected.
@maribu maribu force-pushed the tests/periph_timer branch from 26558c8 to 496abf0 Compare November 25, 2022 11:34
@github-actions github-actions bot removed Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Nov 25, 2022
This was referenced Nov 25, 2022
Open
Merged
@riot-hil-bot
Copy link

HiL Test Results

PASS FAIL SKIP
21 2 0
  ❌ slstk3400a (1 fail test)
PASS FAIL SKIP
0 1 0
TEST RESULT
tests/periph_timer ❌ test fail
  😬 frdm-k64f (1 fail flash)
PASS FAIL SKIP
0 1 0
TEST RESULT
tests/periph_timer 😬 flash fail
  ✅ frdm-k22f
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-f091rc
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-f303re
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-f767zi
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-l152re
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ arduino-mega2560
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-l432kc
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ saml10-xpro
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ saml11-xpro
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ samr21-xpro
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nrf52dk
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ remote-revb
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-l073rz
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ slstk3401a
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-f411re
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ nucleo-f103rb
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ frdm-kw41z
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ esp8266-esp-12x
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ esp32-wroom-32
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ z1
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass
  ✅ arduino-due
PASS FAIL SKIP
1 0 0
TEST RESULT
tests/periph_timer ✅ pass

This was referenced Nov 26, 2022
Merged
Merged
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 CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Dec 8, 2022
@maribu maribu enabled auto-merge December 8, 2022 07:35
@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 Dec 8, 2022
@maribu maribu merged commit 88d840f into RIOT-OS:master Dec 8, 2022
@maribu maribu deleted the tests/periph_timer branch December 8, 2022 10:12
@maribu
Copy link
Member Author

maribu commented Dec 8, 2022

Thx :)

@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@smlng smlng Awaiting requested review from smlng

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

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

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

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@DipSwitch DipSwitch Awaiting requested review from DipSwitch

@vincent-d vincent-d Awaiting requested review from vincent-d

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@gschorcht gschorcht Awaiting requested review from gschorcht

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: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2023.01
Development

Successfully merging this pull request may close these issues.
5 participants
@maribu @riot-ci @kaspar030 @riot-hil-bot @benpicco