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

cpu/msp430/periph_timer: fix timer_query_freqs() [backport 2024.04] #20580

Conversation

maribu
Copy link
Member

@maribu maribu commented Apr 15, 2024

Backport of #20574

Contribution description

The API doc for timer_query_freqs() says it should return 0 Hz as frequency when the prescaler index is out of range. This changes the MSP430 timer driver to live up to the spec.

It also sneaks in an extension to the test app, so that in the future offending code is more easily detected.

Testing procedure

Run the new test. It should pass with this PR, but would fail on master (when the next test is backported).

Issues/PRs references

None

maribu added 2 commits April 15, 2024 19:04
`timer_query_freqs()` should return 0 when index is out of range
according to the doc. This changes the code to live up to the
spec.

(cherry picked from commit 5adf1f1)
Also test that `timer_query_freqs()` for an index out of range
does return 0, as stated by the doc.

(cherry picked from commit 189bb29)
@maribu maribu added Area: cpu Area: CPU/MCU ports Area: tests Area: tests and testing framework labels Apr 15, 2024
@maribu maribu requested a review from kaspar030 as a code owner April 15, 2024 17:04
@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 Apr 15, 2024
@maribu maribu requested a review from gschorcht as a code owner April 15, 2024 17:04
@maribu maribu added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: MSP Platform: This PR/issue effects MSP-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Apr 15, 2024
@maribu maribu closed this Apr 15, 2024
@maribu maribu reopened this Apr 15, 2024
@maribu
Copy link
Member Author

maribu commented Apr 15, 2024

@Teufelchen1 This one includes the increase of the test coverage by adding an expect() that tests that timer_query_freqs() does indeed return 0 on an index out of bounds, which I would rather classify as an enhancement than a bugfix.

Can I keep that commit in the backport anyway, pretty-please?

@riot-ci
Copy link

riot-ci commented Apr 15, 2024

Murdock results

✔️ PASSED

11fe9b8 tests/periph/timer: also test for idx out of range

Success Failures Total Runtime
10044 0 10045 13m:36s

Artifacts

@Teufelchen1 Teufelchen1 enabled auto-merge April 23, 2024 11:49
@Teufelchen1 Teufelchen1 added this pull request to the merge queue Apr 23, 2024
Merged via the queue into RIOT-OS:2024.04-branch with commit a8069ca Apr 23, 2024
57 checks passed
@maribu maribu deleted the backport/2024.04/cpu/msp430/timer-query branch April 23, 2024 18:06
@maribu
Copy link
Member Author

maribu commented Apr 23, 2024

Thx :)

@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
@mguetschow mguetschow added this to the Release 2024.04 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports 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 Platform: MSP Platform: This PR/issue effects MSP-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants