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

tests/periph_rtt: Fix for tick conversion test #19522

Merged
merged 1 commit into from
May 4, 2023

Conversation

chudov
Copy link
Contributor

@chudov chudov commented Apr 28, 2023

Contribution description

Type casting and printf formatting for the RTT_MAX_VALUE and RTT_FREQUENCY are fixed so 32-bit value is properly handled by 'avr-libc'.
The original tick conversion test assumes that RTT_FREQUENCY is power of 2 so forward and backward ticks to seconds conversion results in the original ticks value. To fix it the result of the forward-backward conversion is compared with ticktest / RTT_FREQUENCY * RTT_FREQUENCY that considers rounding errors.

Changes were tested on deRFmega256 and nrf52840dongle.

Testing procedure

tests/periph_rtt on a board with ATmega256RFR2 shall:

  • show correct RTT_MAX_VALUE
  • conversion check shall report no error.

Issues/PRs references

Fixes #15940

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Apr 28, 2023
@benpicco benpicco added 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 May 2, 2023
Copy link
Contributor

@benpicco benpicco 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!
Just one thing: please change the commit message to match the PR title (prefix with the affected module)

@riot-ci
Copy link

riot-ci commented May 2, 2023

Murdock results

✔️ PASSED

8803944 tests/periph_rtt: Fix for tick conversion test #15940

Success Failures Total Runtime
20 0 20 01m:56s

Artifacts

@chudov chudov force-pushed the tests/periph_rtt_avr_rfr2_fix branch from 78f3fb7 to 6c94491 Compare May 2, 2023 13:48
@chudov
Copy link
Contributor Author

chudov commented May 2, 2023

@benpicco is it better now?

@benpicco
Copy link
Contributor

benpicco commented May 2, 2023

Yes, thank you!
But CI found that the build now fails for ESP32

tests/periph_rtt/main.c Outdated Show resolved Hide resolved
tests/periph_rtt/main.c Outdated Show resolved Hide resolved
@chudov chudov force-pushed the tests/periph_rtt_avr_rfr2_fix branch from 6c94491 to 7c3186d Compare May 2, 2023 14:01
@benpicco benpicco requested a review from maribu May 2, 2023 14:04
@chudov chudov force-pushed the tests/periph_rtt_avr_rfr2_fix branch from 7c3186d to a6ad25c Compare May 2, 2023 14:07
@chudov chudov force-pushed the tests/periph_rtt_avr_rfr2_fix branch from a6ad25c to 8803944 Compare May 2, 2023 14:13
Copy link
Contributor

@benpicco benpicco 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 May 4, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 34c094d into RIOT-OS:master May 4, 2023
@chudov chudov deleted the tests/periph_rtt_avr_rfr2_fix branch May 5, 2023 13:25
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 CI: run tests If set, CI server will run tests on hardware for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic test for periph/rtt introduced in #15431 is incorrect
3 participants