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.

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

Document (and maybe revisit) assumptions on comparability between default instances #1

Open
chrysn opened this issue Sep 21, 2022 · 0 comments

Comments

@chrysn
Copy link

chrysn commented Sep 21, 2022

The way this is used in choreographer (timers are not carried around ever but only created briefly and polled) implies that it's an assumption in groundhog that tick values can be compared between different (created-through-Default) instances of a RollingTimer, and that the timer keeps rolling even when no instances are around.

For a concrete backend I'm having in mind (RIOT's ztimer) this is not given, and I was one merged upstream PR short of implementing RollingTimer on it. I see two ways forward from this:

  • Document that it is so.

    It's a valid way, but makes it likely unusable on RIOT -- or the implementation would need to work around it by having a global static timer that creates a hidden ZTimer lock in a hidden global mutex at first use.

  • Document that it is not so. (Or state that this is the intention).

    This would necessitate changes in choreographer. I'm not sure if the actions all have good access to a single stored RollingTimer. If so, it's easy. If not, they might .clone() one around. Note that implementations that comply with the current model would probably all be zero-sized, and can have zero-cost .clone() methods.

If you lean toward the second way but don't want to do the changes, that's something I can work with too -- in that case, I (if and when I use choreographer on RIOT ... or whoever next stumbles on this) could see that changes along these lines would be welcome to choreographer, and once PR'd the semantics could be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant