Skip to content

tests/periph_timer_short_relative_set: fix diff for non 32 bit timers#12856

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
MichelRottleuthner:pr_fix_test_short_relative_set
Dec 2, 2019
Merged

tests/periph_timer_short_relative_set: fix diff for non 32 bit timers#12856
cgundogan merged 1 commit intoRIOT-OS:masterfrom
MichelRottleuthner:pr_fix_test_short_relative_set

Conversation

@MichelRottleuthner
Copy link
Contributor

@MichelRottleuthner MichelRottleuthner commented Dec 2, 2019

Contribution description

Fixes the problem described in #12610 (comment): the diff calculation between two timestamps is not working properly for timers that are not 32 bit.

Testing procedure

Look at the code, run /tests/periph_timer_short_relative_set on a platform with 16 bit timer (e.g. iotlab-m3).

Issues/PRs references

#10493, #12610

@MichelRottleuthner MichelRottleuthner added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: tests Area: tests and testing framework labels Dec 2, 2019
@kaspar030 kaspar030 added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 2, 2019
@kaspar030
Copy link
Contributor

run /tests/periph_timer_short_relative_set on a platform with 16 bit timer (e.g. iotlab-m3).

Don't have such a board at hand, could someone else give this a quick run?

@aabadie
Copy link
Contributor

aabadie commented Dec 2, 2019

platform with 16 bit timer (e.g. iotlab-m3).

IoT-LAB provides quite a few of them. Is it required to have direct access to hardware to test this ?

@MichelRottleuthner
Copy link
Contributor Author

Physical access is not required so iot lab should do. Keep in mind that the expected result is that the tast will fail on that platform (refer to #12610 (comment)).

Copy link
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

Tested on an iotlab-m3 node with a 16-bit timer. I adjusted the interval variable in main.c to compensate for the timer width and assure that a wrap-around happens. With interval=1000 the counter goes down to 951, while this patch let's the test run down to iteration 2. ACK

@cgundogan cgundogan added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Dec 2, 2019
@cgundogan cgundogan merged commit 3c99481 into RIOT-OS:master Dec 2, 2019
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines 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