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.

Sign up for GitHub

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

Jump to bottom

sys/ztimer: ztimer_set() return the now value #17385

Merged
merged 1 commit into from
Dec 13, 2021
Merged

sys/ztimer: ztimer_set() return the now value #17385

merged 1 commit into from
Dec 13, 2021

Conversation

fjmolinas
Copy link
Contributor

Contribution description

This PR makes ztimer_set return the now value it was set against. This change although an API change should not break anything since the function was returning void, but it should allow estimating the actual value against which the set function was used.

My main motivation for this is that esp timer implementation sets a timer_expire value that cant be directly translated from ztimer to xtimer. So I'm open to any other better suggestion on doing such a thing.

Testing procedure

everything should still build ok!

@github-actions github-actions bot added Area: sys Area: System Area: timers Area: timer subsystems labels Dec 13, 2021
@fjmolinas fjmolinas mentioned this pull request Dec 13, 2021
Merged
kaspar030
kaspar030 reviewed Dec 13, 2021
View reviewed changes
sys/include/ztimer.h Outdated Show resolved Hide resolved
@kaspar030 kaspar030 changed the title sys/ztimer: ztimer_set return the now value sys/ztimer: ztimer_set() return the now value Dec 13, 2021
kaspar030
kaspar030 approved these changes Dec 13, 2021
View reviewed changes
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@kaspar030
Copy link
Contributor

Make mental note: do the same for ztimer64

@fjmolinas fjmolinas added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Dec 13, 2021
@kaspar030
Copy link
Contributor

BTW, does Update sys/include/ztimer.h not trigger the commit check?

@kaspar030
Copy link
Contributor

please squash!

@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 Dec 13, 2021
maribu
maribu reviewed Dec 13, 2021
View reviewed changes
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Sorry for being late to the party. But the current CI queue says there would be still 13 minutes left to squash in this nitpick without wasting CI time :) (I'm not insisting on this, but I do like it when all CI tools are happy.)

sys/ztimer/core.c Show resolved Hide resolved
@kaspar030 kaspar030 merged commit efa24b3 into RIOT-OS:master Dec 13, 2021
@fjmolinas fjmolinas mentioned this pull request Dec 14, 2021
Merged
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Jan 21, 2022
@fjmolinas fjmolinas deleted the pr_ztimer_set_now branch March 24, 2022 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaspar030 kaspar030 kaspar030 approved these changes

@maribu maribu maribu left review comments

@kfessel kfessel Awaiting requested review from kfessel

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

Assignees
No one assigned
Labels
Area: sys Area: System 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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
3 participants
@fjmolinas @kaspar030 @maribu