Skip to content

tests: add xtimer_usleep test#6394

Merged
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
kYc0o:test_xtimer_usleep
Jan 19, 2017
Merged

tests: add xtimer_usleep test#6394
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
kYc0o:test_xtimer_usleep

Conversation

@kYc0o
Copy link
Copy Markdown
Contributor

@kYc0o kYc0o commented Jan 17, 2017

While checking some xtimer functionalities on several boards, I realised that xtimer_usleep was failing, and we didn't have a test. So, here there is.

@kYc0o kYc0o added Area: tests Area: tests and testing framework Area: timers Area: timer subsystems labels Jan 17, 2017
@kYc0o kYc0o added this to the Release 2017.01 milestone Jan 17, 2017
OlegHahm
OlegHahm approved these changes Jan 17, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think a (p)expect script would be nice here. You could check if the drift lies inside certain boundaries, e.g. +/- 100ppm.

tests/xtimer_usleep/main.c Outdated

#define ONE_SEC_SLEEP (1 * 1000U * 1000U)
#define FIVE_SEC_SLEEP (5 * 1000U * 1000U)
#define TEN_SEC_SLEEP (10 * 1000U * 1000U)
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 17, 2017

Choose a reason for hiding this comment

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

Any particular reason not to use SEC_IN_USEC from timex.h?

Copy link
Copy Markdown
Contributor Author

@kYc0o kYc0o Jan 17, 2017

Choose a reason for hiding this comment

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

Not really, I didn't think about it, will change...

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 17, 2017

I think a (p)expect script would be nice here. You could check if the drift lies inside certain boundaries, e.g. +/- 100ppm.

Any hints on how to do it? should I use something like xtimer_now() ?

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 17, 2017

I don't think you will need xtimer_now(). Take the local timestamp at first occurence of
<======== If using pyterm, this is the time when started.
and then compute the intervals and compare it to the expected ones.

OlegHahm
OlegHahm requested changes Jan 17, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm left a comment

Choose a reason for hiding this comment

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

expect script missing

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 17, 2017

For the record: test seems to work fine on Z1 with MSP430. I don't have any AVR here to test.

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 17, 2017

Should be ok.

aabadie
aabadie reviewed Jan 17, 2017
View reviewed changes
tests/xtimer_usleep/main.c Outdated
* @file
* @brief xtimer_usleep test application
*
* @author Francisco Acosta <[email protected]>
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jan 17, 2017

Choose a reason for hiding this comment

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

typo ! inria

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 17, 2017

Should be ok.

What should be okay?

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 17, 2017

The test. Just corrected some typos

aabadie
aabadie reviewed Jan 17, 2017
View reviewed changes
tests/xtimer_usleep/main.c Outdated
printf("\n<======== If using pyterm, this is the time when started.\n\n");

xtimer_usleep(ONE_SEC_SLEEP);
printf("Slept for 1 sec...\n");
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jan 17, 2017

Choose a reason for hiding this comment

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

Why not display the message before xtimer_usleep. With the message "Sleeping during x s" ?

Copy link
Copy Markdown
Contributor Author

@kYc0o kYc0o Jan 17, 2017

Choose a reason for hiding this comment

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

Just to make the test happy...

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 17, 2017

Oh wait, will conflict with #6399

@aabadie aabadie mentioned this pull request Jan 17, 2017
Merged
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 18, 2017

I would say, what got merged first wins. The other one has to be rebased.

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 18, 2017

Will you be my ACK @OlegHahm ?

@kYc0o kYc0o force-pushed the test_xtimer_usleep branch from 04143ee to 62728a4 Compare January 18, 2017 10:55
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 18, 2017

Rebased.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jan 18, 2017

need squashing

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 18, 2017

After I got my ACK.

@OlegHahm OlegHahm added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties labels Jan 18, 2017
OlegHahm
OlegHahm requested changes Jan 18, 2017
View reviewed changes
tests/xtimer_usleep/tests/01-run.py Outdated
child.expect('\r\n')
child.expect('<======== If using pyterm, this is the time when started.')
child.expect('\r\n')
n = 3
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 18, 2017

Choose a reason for hiding this comment

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

I would propose to use a bigger n.

Copy link
Copy Markdown
Contributor Author

@kYc0o kYc0o Jan 18, 2017

Choose a reason for hiding this comment

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

an "N" you mean?

Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 18, 2017

Choose a reason for hiding this comment

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

No, I meant something like n = 30.

tests/xtimer_usleep/tests/01-run.py Outdated
child.expect('\r\n')
n = 3
while (n):
if n == 3:
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 18, 2017

Choose a reason for hiding this comment

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

...and then use modulo here.

tests/xtimer_usleep/tests/01-run.py Outdated
exp_diff1 = 1000000
exp_diff5 = 5000000
exp_diff10 = 10000000
child.expect('This test will print "Slept for X sec..." every 1, 5 and 10 seconds.\r\n')
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 18, 2017

Choose a reason for hiding this comment

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

If you make these strings unicode (prefixing with U) the script should be Python 2 and Python 3 compatible.

@kYc0o kYc0o force-pushed the test_xtimer_usleep branch from 62728a4 to 28f0b3d Compare January 18, 2017 20:02
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jan 18, 2017

@OlegHahm do you like the changes?

OlegHahm
OlegHahm reviewed Jan 18, 2017
View reviewed changes
tests/xtimer_usleep/tests/01-run.py Outdated
@@ -0,0 +1,55 @@
#!/usr/bin/env python3
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm Jan 18, 2017

Choose a reason for hiding this comment

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

s/python3/python/

OlegHahm
OlegHahm approved these changes Jan 18, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@OlegHahm OlegHahm left a comment

Choose a reason for hiding this comment

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

Apart from minor change: ACK

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 18, 2017

Why is CI green? Needs squashing label is set.

@OlegHahm OlegHahm added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 18, 2017
@kYc0o kYc0o force-pushed the test_xtimer_usleep branch from 2502f19 to ddaa28f Compare January 18, 2017 22:55
@kYc0o kYc0o added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 18, 2017
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 19, 2017

Merging in favour of Olegs ACK

@PeterKietzmann PeterKietzmann merged commit f7043c5 into RIOT-OS:master Jan 19, 2017
@kYc0o kYc0o mentioned this pull request Jan 20, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie left review comments

@OlegHahm OlegHahm OlegHahm approved these changes

Assignees

@OlegHahm OlegHahm

Labels

Area: tests Area: tests and testing framework Area: timers Area: timer subsystems CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kYc0o @OlegHahm @aabadie @PeterKietzmann