sys/evtimer: introduce ZTIMER_MSEC as timer backend

[jue89]

Contribution description

Inspired by the implementation of xtimer_on_ztimer, I tried the same thing for evtimer.

According to evtimer's description, it has been built to allow for long running timers. Thus, I made an adaption to the ZTIMER_MSEC clock which should perfectly meet this requirement.

This PR is still WIP ... not all tests are green.

Testing procedure

Run tests/evtimer_msg with the following command line:
USEMODULE="ztimer_usec evtimer_on_ztimer" make all test

(tests/evtimer_underflow doesn't work on native.)

Issues/PRs references

Requires #13666

[jue89]

Since #13666 has been merged, I get back to this PR.

I rebased this PR on master and included a ztimer implementation for evtimer_now_*() .

Furthermore, I've tested tests/evtimer_underflow on the samr30-xpro board and it turns out to be working properly. I guess ztimer isn't configured for native properly, yet. I'll further investigate this issue.

[maribu]

Some comments inline. I was about to implement this myself - luckily I checked for open PRs first :-)

[maribu]

Btw: It is now called ztimer_xtimer_compat, not xtimer_on_ztimer. Maybe it would make sense to rename this for consistency?

[kaspar030]

Btw: It is now called ztimer_xtimer_compat, not xtimer_on_ztimer. Maybe it would make sense to rename this for consistency?

they both exist. "xtimer_on_ztimer" includes full xtimer, but makes xtimer use ztimer as backend. "ztimer_xtimer_compat" is a wrapper (xtimer calls get wrapped to ztimer calls). The latter doesn't support the complete xtimer api.

[jue89]

I think everything that should be in this PR has been implemented, @maribu.

This PR:

• Implements the evtimer_on_ztimer
• Handels the evtimer_on_ztimer dependency management in sys/ztimer/Makefile.dep
• Fixes tests/evtimer_msg when evtimer

Follow-up PR 1:

• Move xtimer_on_ztimer pseudo module definition to makefiles/pseudomodules.mk
• Implement a check that no Makefile.dep touches PSEUDOMODULES
• Edit: ztimer_xtimer_compat redefines xtimer to be a pseudo module during dependency resolution ... we need to find a fix for this problem.

Follow-up PR 2:

• Move xtimer and xtimer_on_ztimer dependency management to sys/Makefile.dep
• Move evtimer and evtimer_on_ztimer dependency management to sys/Makefile.dep
• Fix the indentation quirk

Does this sound like a plan? If so I would rebase and clean-up this PR if you don't mind. It became kinda messy ;)

[kaspar030]

* Implement a check that no `Makefile.dep` touches `PSEUDOMODULES`

We could also go the other way around. There's no harm in Makefile.dep adding to pseudomodules. We could move them from pseudomodules.mk to the Makefile.deps, so they're close to the other module dependencies.

[jue89]

Rebased and ready for review ;)

[jue89]

We could also go the other way around. There's no harm in Makefile.dep adding to pseudomodules. We could move them from pseudomodules.mk to the Makefile.deps, so they're close to the other module dependencies.

I think @maribu's point is the recursive dependency resolution by including all Makefile.dep over and over again until the result hasn't altered during a run. So in this case PSEUDOMODULES ends up with containing xtimer_on_ztimer several times.

This does work but takes some time. But in case of the conditional PSEUDOMODULES += xtimer I don't see any other solution to pull PSEUDOMODULES into the dependency resolution process and deduplicate und sort it, as well.

Or have I got something wrong?

[fjmolinas]

This does work but takes some time. But in case of the conditional PSEUDOMODULES += xtimer I don't see any other solution to pull PSEUDOMODULES into the dependency resolution process and deduplicate und sort it, as well.

Could it work if you declare a pseudomodule xtimer_on_xtimer and then in xtimer/Makefile?

NO_AUTO_SRC = 1

ifneq (,$(filter xtimer_on_xtimer,$(USEMODULE)))
  SRC += xtimer_core.c 
  SRC += xtimer.c 
endif

include $(RIOTBASE)/Makefile.base

(did not test this, just brainstorming)

[maribu]

But in case of the conditional PSEUDOMODULES += xtimer I don't see any other solution to pull PSEUDOMODULES into the dependency resolution process and deduplicate und sort it, as well.

Oh yes, a module this is only conditionally a pseudo-module is indeed a special case. The approach @fjmolinas suggested would certainly work. Maybe it could be simplified to this:

ifneq (,$(filter xtimer_on_xtimer,$(USEMODULE)))
  NO_AUTO_SRC = 1
endif

Alternatively, this special case IMO justifies an exception. A small comment above the PSEUDOMODULES += outside of pseudomodules.mk to state the reasons for this would be greatly appreciated. This would prevent it for becoming a role model for touching PSEUDOMODULES during dependency resolution.

I don't think that de-duplication of PSEUDOMODULES would actually be needed. If I recall correctly, we only access it using $(filter <FOO>,$(PSEUDOMODULES)); and for that it does not matter if it contains duplicates. (Deduplication might improve performance if the array would contain many duplicates, though. But that is not the case here.)

[jue89]

Really good idea! But I would put this in a follow-up PR.
(And the name of the trouble maker is ztimer_xtimer_compat ;))

[kaspar030]

I think @maribu's point is the recursive dependency resolution by including all Makefile.dep over and over again until the result hasn't altered during a run. So in this case PSEUDOMODULES ends up with containing xtimer_on_ztimer several times.

This does work but takes some time.

Can we quantify this? The recursion stops when there are no changes (additions) to the checked variables. Adding to PSEUDOMODULES doesn't even cause deeper recursion. And if having a module in there multiple times probably won't have any measurable effect on performance. Neither would a PSEUDOMODULES := $(sort $ÜSEUDOMODULES)), which would deduplicate it.

You can certainly "fix" this, but IMO there's no issue.

[fjmolinas]

@kaspar030 only squashing is missing here, are you ok with the change?

[maribu]

Note: This won't change generated machine code, unless someone explicitly choose to use ztimer. IMO, this is, hence, low impact and should be considered for merging even during soft feature freeze.

[bergzand]

Note: This won't change generated machine code, unless someone explicitly choose to use ztimer. IMO, this is, hence, low impact and should be considered for merging even during soft feature freeze.

Fine with me 👍

[jue89]

The testbed is still alive :)

Does anyone mind if I squash the fixups? (@kaspar030?)

[kaspar030]

yes, please squash!

[benpicco]

Looks like this uncovered a missing #include <assert.h in gnrc/mac/timeout.h

[jue89]

Looks like this uncovered a missing #include <assert.h in gnrc/mac/timeout.h

Yip. I pushed a commit fixing that. I think this one-liner doesn't need its own PR?

[kaspar030]

Should we update the documentation in this PR or postpone to a followup? It is currently pretty specific with explaining that xtimer is used as backend...

[jue89]

Hmm, I'll push some lines the evtimer's documentation pointing out that it may be based on xtimer or ztimer_msec depending on configuration. Currently the documentation states "uses xtimer as backend" which isn't true for this case ;)

Postponing documentation to a later point in time is likely to never happen :D

[jue89]

I added a quick note to refer to evtimer_on_ztimer. Now, nothing wrong is stated in the documentation. Should I add more docu or is this already enough?

[maribu]

I added a quick note to refer to evtimer_on_ztimer. Now, nothing wrong is stated in the documentation. Should I add more docu or is this already enough?

IMO, this is good as it as. Let's rerun Murdock. (One build failed due to connectivity issues of the worker.)

[maribu]

@kaspar030: All green ;-)

[kaspar030]

ACK.

[jue89]

Am I allowed to press that green shiny Merge button? :)

[jue89]

Thank you all for reviewing!

[kfessel]

why was this line added?

[kfessel]

speaking about that ztimer_now64