Skip to content

event/timeout: remove forced ZTIMER_USEC dependency#16892

Closed
haukepetersen wants to merge 2 commits intoRIOT-OS:masterfrom
haukepetersen:fix_eventtimeout_ztimertimerdeps
Closed

event/timeout: remove forced ZTIMER_USEC dependency#16892
haukepetersen wants to merge 2 commits intoRIOT-OS:masterfrom
haukepetersen:fix_eventtimeout_ztimertimerdeps

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen commented Sep 24, 2021

Contribution description

This PR is (for now) simply a rough draft on how #16891 could be addressed. The goal is to remove the forced ZTIMER_USEC dependency from event/timeout, as this disables low-power modes on many platforms, even if the USEC timer is actually never used in the build...

The idea is to change the style of back-end timer selection a little bit for this module:

  1. ztimer is always preferred -> so if ztimer is included in the build, it is automatically selected as backend. In the majority of RIOT builds, this is most likely the case by now...
  2. per default, event_timeout_init() is not available. If modules (still) use this, it must be actively enabled by declaring a dependency on event_timeout_legacy.

TODO:

  • the event_timeout_ztimer module can be removed
  • existing dependencies to event_timeout must be changed to depend on event_timeout_legacy
  • step-by-step, modules like e.g. gcoap or dhcp_client shoudl be migrated to use the event_timeout_ztimer_x() functions, possibly while migrating the complete modules to ztimer :-)

So what do you think?

Testing procedure

n/a as of now

Issues/PRs references

fixes #16891

@haukepetersen haukepetersen added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Sep 24, 2021
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Sep 24, 2021
@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Sep 27, 2021

From a quick look, I like the approach, looks as a good solution IMO.

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Sep 27, 2021

From a quick look, I like the approach, looks as a good solution IMO.

Although maybe we should go the same way as the netdev2 migration, and first call it event_timeout_ztimer (with he same handling as you have) and the migrate the naming). This was external code with the dependency still would work.

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Oct 6, 2021

From a quick look, I like the approach, looks as a good solution IMO.

Although maybe we should go the same way as the netdev2 migration, and first call it event_timeout_ztimer (with he same handling as you have) and the migrate the naming). This was external code with the dependency still would work.

I have an alternative allowing for the same result but making with the inverse approach making drop-in replacement easy. See here a POC, I can PR.

Basically, instead of adding a new module for the legacy API, make event_timeout_ztimer a module not depending on event_timeout and if only event_timeout_ztimer is selected only the new api is implemented (no forced backend). If event_timeout and ztimer are selected then ztimer_usec is pulled in for a drop in replacement. This IMO makes incrementally changing things easier, while not breaking external code.

What do you think @haukepetersen? In the long term marking the old api as legacy is best, with the current speed at wich we change things this seems like maybe the more sensible path.

@fjmolinas fjmolinas mentioned this pull request Oct 6, 2021
Merged
@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Oct 6, 2021

closing in favor of #16958

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgundogan cgundogan Awaiting requested review from cgundogan

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

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

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

Labels

Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

event/timeout: fixed dependency on ZTIMER_USEC preventing low-power operation

2 participants

@haukepetersen @fjmolinas