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

boards/arduino-due: allow changing frequency #16257

Merged
merged 2 commits into from
Mar 30, 2021

Conversation

fjmolinas
Copy link
Contributor

@fjmolinas fjmolinas commented Mar 30, 2021

Contribution description

This PR allows overriding the default value for the rtt frequency, it's one I missed in #14303.

Testing procedure

Sadly I do not have a BOARD to test, but this should be the testing procedure:

  • tests/periph_rtt should still pass on sam3 boards with different frequencies

  • BOARD=arduino-due make -C tests/periph_rtt flash test --no-print-directory -j3

  • CFLAGS+=-DRTT_FREQUENCY=1024 BOARD=arduino-due make -C tests/periph_rtt clean flash test --no-print-directory -j3

Although IMO a green Murdock should be enough.

Issues/PRs references

Realized I missed this sam* family while looking at #16254

@fjmolinas fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: cpu Area: CPU/MCU ports labels Mar 30, 2021
@fjmolinas fjmolinas requested a review from maribu March 30, 2021 12:31
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 30, 2021
@maribu
Copy link
Member

maribu commented Mar 30, 2021

The test fails (also in master) with Error: rtt_get_alarm() not working. Looks like the increase test coverage of that test found another bug I didn't notice before.

@maribu
Copy link
Member

maribu commented Mar 30, 2021

After rebasing on top of #16258 I get:

2021-03-30 14:58:17,181 # Help: Press s to start test, r to print it is ready
s
2021-03-30 14:58:18,502 # START
2021-03-30 14:58:18,511 # main(): This is RIOT! (Version: 2021.04-devel-1176-gc70d9e-fjmolinas)
2021-03-30 14:58:18,511 # 
2021-03-30 14:58:18,514 # RIOT RTT low-level driver test
2021-03-30 14:58:18,516 # RTT configuration:
2021-03-30 14:58:18,517 # RTT_MAX_VALUE: 0xffffffff
2021-03-30 14:58:18,519 # RTT_FREQUENCY: 1024
2021-03-30 14:58:18,520 # 
2021-03-30 14:58:18,521 # Testing the tick conversion
2021-03-30 14:58:18,524 # Trying to convert 1 to seconds and back
2021-03-30 14:58:18,528 # Trying to convert 256 to seconds and back
2021-03-30 14:58:18,532 # Trying to convert 65536 to seconds and back
2021-03-30 14:58:18,536 # Trying to convert 16777216 to seconds and back
2021-03-30 14:58:18,540 # Trying to convert 2147483648 to seconds and back
2021-03-30 14:58:18,541 # All ok
2021-03-30 14:58:18,541 # 
2021-03-30 14:58:18,544 # Initializing the RTT driver
2021-03-30 14:58:18,548 # This test will now display 'Hello' every 5 seconds
2021-03-30 14:58:18,549 # 
2021-03-30 14:58:18,550 # RTT now: 3
2021-03-30 14:58:18,552 # Setting initial alarm to now + 5 s (5123)
2021-03-30 14:58:18,555 # rtt_get_alarm() PASSED
2021-03-30 14:58:18,559 # Done setting up the RTT, wait for many Hellos
2021-03-30 14:58:24,372 # Hello
2021-03-30 14:58:30,197 # Hello
2021-03-30 14:58:36,024 # Hello
2021-03-30 14:58:41,847 # Hello
2021-03-30 14:58:47,671 # Hello

So timing is off quite a bit, but so it is on master. Hence, IMO the PR delivers what is promised. The bugs in the RTT are preexisting and unrelated to this PR.

@maribu maribu merged commit 8919b2c into RIOT-OS:master Mar 30, 2021
@kaspar030 kaspar030 added this to the Release 2021.04 milestone Apr 23, 2021
@fjmolinas fjmolinas deleted the pr_arduino_due_config_freq branch July 30, 2021 11:28
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 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
Development

Successfully merging this pull request may close these issues.

3 participants