8000 tests/periph/timer: fix timeout computation and overflow handling by maribu · Pull Request #20161 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

tests/periph/timer: fix timeout computation and overflow handling #20161

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
Merged
merged 2 commits into from
Dec 7, 2023
Merged

tests/periph/timer: fix timeout computation and overflow handling #20161

merged 2 commits into from
Dec 7, 2023

Conversation

maribu
Copy link
Member
@maribu maribu commented Dec 7, 2023 •
edited
Loading

Contribution description

  • the timeout computation for the spurious IRQ test confused numerator and denominator in a fraction
  • the timeout offset between timer channels was hardcoded to 5000 from when the timer was only tested with 1 MHz as frequency
    • This resulted in slooooow test runs when running at slow frequencies
  • fix overflow handling in the spinning wait
    • likely this would never overflow anyway assuming that timer_init() resets the counter value, but let's not rely on this and just fix the bug for good

Testing procedure

The test now passes with #20160 on the 16 MHz timer

Issues/PRs references

Found during #20160

@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) 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 labels Dec 7, 2023
@maribu maribu mentioned this pull request Dec 7, 2023
Merged
1 task
@maribu maribu requested a review from benpicco December 7, 2023 13:46
@maribu maribu added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Dec 7, 2023
@riot-ci
Copy link
riot-ci commented Dec 7, 2023 •
edited
Loading

Murdock results

✔️ PASSED

aa045d5 tests/periph/timer: fix timeout computation and overflow handling

Success Failures Total Runtime
42 0 42 01m:14s

Artifacts

@maribu maribu force-pushed the tests/periph/timer/bugfix branch 2 times, most recently from cc20def to cd1fb15 Compare December 7, 2023 14:00
@maribu maribu requested a review from jia200x as a code owner December 7, 2023 14:00
@github-actions github-actions bot added the Area: doc Area: Documentation label Dec 7, 2023
@maribu maribu force-pushed the tests/periph/timer/bugfix branch from cd1fb15 to deb2e52 Compare December 7, 2023 14:00
maribu
maribu commented 8000 Dec 7, 2023
View reviewed changes
@maribu maribu force-pushed the tests/periph/timer/bugfix branch from deb2e52 to 197f4dc Compare December 7, 2023 14:02
@github-actions github-actions bot removed the Area: doc Area: Documentation label Dec 7, 2023
@maribu maribu force-pushed the tests/periph/timer/bugfix branch 2 times, most recently from 75d62b4 to d2db78a Compare December 7, 2023 14:26
@maribu maribu requested a review from kfessel December 7, 2023 14:28
@maribu
Copy link
Member Author
maribu commented Dec 7, 2023

All green, even the spell checks

kfessel
kfessel reviewed Dec 7, 2023
View reviewed changes
@maribu maribu mentioned this pull request Dec 7, 2023
Merged
2 tasks
kfessel
kfessel reviewed Dec 7, 2023
View reviewed changes
kfessel
kfessel approved these changes Dec 7, 2023
View reviewed changes
Copy link
Contributor
@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

looks good, heard @maribu testing this

@maribu
tests/periph/timer: fix timeout computation and overflow handling
aa045d5 
- the timeout computation for the spurious IRQ test confused numerator
  and denominator in a fraction
- the timeout offset between timer channels was hardcoded to 5000 from
  when the timer was only tested wi
8000
th 1 MHz as frequency
    - This resulted in slooooow test runs when running at slow
      frequencies
- fix overflow handling in the spinning wait
    - likely this would never overflow anyway assuming that
      `timer_init()` resets the counter value, but let's not rely on
      this and just fix the bug for good
@maribu maribu force-pushed the tests/periph/timer/bugfix branch from 2da66fd to aa045d5 Compare December 7, 2023 15:07
@maribu maribu enabled auto-merge December 7, 2023 15:08
@maribu maribu added this pull request to the merge queue Dec 7, 2023
Merged via the queue into RIOT-OS:master with commit 5dc3d9c Dec 7, 2023
@maribu maribu deleted the tests/periph/timer/bugfix branch December 7, 2023 15:14
@maribu
Copy link
Member Author
maribu commented Dec 7, 2023

Thx :-)

@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfessel kfessel kfessel 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

@benpicco benpicco Awaiting requested review from benpicco

@jia200x jia200x Awaiting requested review from jia200x

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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2024.01
Development

Successfully merging this pull request may close these issues.
4 participants
@maribu @riot-ci @kfessel @MrKevinWeiss
0