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

Change Clock trait to require interior mutable state? #32

Closed
PTaylor-us opened this issue Jul 1, 2020 · 1 comment · Fixed by #33
Closed

Change Clock trait to require interior mutable state? #32

PTaylor-us opened this issue Jul 1, 2020 · 1 comment · Fixed by #33
Labels
discussion Something that needs more discussion

Comments

@PTaylor-us
Copy link
Member

PTaylor-us commented Jul 1, 2020

I believe this this change must be made (now(&mut self) --> now(&self)) or every Timer function that needs to access the clock must be passed a &mut of the clock. I don't like the latter for the following reasons:

  • Makes usage much more verbose
  • Allows usage of different clocks with the same Timer

I think the last item is the greatest concern. What do you think @eldruin? How much more difficult would this make implementation? In my example implementation (using peripheral [not external]) timers, it doesn't change anything as the peripherals are all interior mutable already.

@PTaylor-us PTaylor-us added the discussion Something that needs more discussion label Jul 1, 2020
@PTaylor-us PTaylor-us linked a pull request Jul 2, 2020 that will close this issue
@eldruin
Copy link
Contributor

eldruin commented Jul 2, 2020

I believe this this change must be made (now(&mut self) --> now(&self)) or every Timer function that needs to access the clock must be passed a &mut of the clock. I don't like the latter for the following reasons:

  • Makes usage much more verbose
  • Allows usage of different clocks with the same Timer

I understand. The same thing happens on drivers with the mutable I2C devices. The solution there is to consume the resource and return it upon destruction. See here. The return of the valuable resources can make code messy, though, if doing type-level transformations as highlighted at 2. here (sorry for citing myself again :) I just want to be clear).

Interior mutability would be ok for me. It can be worked around in the driver. It just requires the implementation to contain a RefCell to a second mutable layer. I used this to implement split() for the pcf857x I/O expander driver, among others. It is a bit cumbersome but it works. It only adds a CouldNotAcquireDevice error in case the acquisition failed, which cannot be statically reasoned about.

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

Successfully merging a pull request may close this issue.

2 participants