-
Notifications
You must be signed in to change notification settings - Fork 234
Generic Mutex Trait #119
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
Comments
Oh sorry, the name should probably be just |
@Rahix seems like a good idea. Is the idea of putting |
Well, first of all I would just suggest to add a As soon as that is the case, using shared-bus and the like would be much easier as you no longer have to specify which mutex you want to use via a feature flag. And as @eldruin mentioned, this is also of benefit for other drivers that need to synchronize peripheral accesses.
No, you will still need shared-bus for the manager and proxy types. We could think about moving them into the hal as well, but I am not sure if that is a good idea ... In my opinion it doesn't hurt to have a different crate for that. The reason it hurts right now is because the mutex type is not in hal. |
This is now in https://github.com/rust-embedded/mutex-trait |
While writing
shared-bus
I was hit with the lack of a standardized Mutex type.std
solves this by implementing the mutex type differently for all architectures it supports and this is what I currently do in shared-bus as well. However, this is not feasible in the long run, because we want to support all architectures that implementembedded-hal
. In my opinion,embedded-hal
needs a standardized mutex trait and I would propose it to look similar to the one I have in shared-bus right now:As explanation:
create
method is needed to allow drivers to transparently create mutex objects. I specifically chose not to name itnew
to avoid name conflicts.lock
returns a lock-guard but this design is not feasible here. The reason is that the mutex type defined inbare-metal
is to be used with a closure and aCriticalSection
.cc @jamesmunns, @therealprof
The text was updated successfully, but these errors were encountered: