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.

Sign up for GitHub

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

Jump to bottom

Allow naming lock type of shared resources #612

Closed
wants to merge 6 commits into from
Closed

Allow naming lock type of shared resources #612

wants to merge 6 commits into from

Conversation

luiswirth
Copy link

@luiswirth luiswirth commented Feb 15, 2022 •
edited

This PR allows users to name the lock types of shared resources. This is useful to pass the locks to other functions.
It's not always possible to just specify the argument type as impl rtic::Mutex<Resource>, because the function then is no longer trait object safe. The type can now be named as app::shared_resources::{}_lock.

Example:
This trait is now object safe!

trait TraitObject {
  fn func(
    &self,
    lock: &mut app::shared_resources::resource_lock,
  );
}

changes:

  • change lock type ident: {}_that_needs_to_be_locked => {}_lock
  • change visibility of shared_resources module to pub

closes #611

@luiswirth luiswirth mentioned this pull request Feb 15, 2022
Open
@luiswirth
Copy link
Author

luiswirth commented Mar 8, 2022 •
edited

What's the state of this PR? Are you interested in this addition?

I provided a MWE as mentioned in this comment, showing what this PR solves.
I think this is a nice improvement, especially since it involves little modifications of RTIC.

@perlindgren
Copy link
Collaborator

I think the main concern here is that it exposes the implementation details. I don't think it would be unsound to expose though. The underlying problem seems to be related to the creation vtables in Rust (so it cannot be made into a trait object).

I've been experimenting with another internal representation of locks that might solve this particular problem. It won't solve the underlying problem with vtables totally though. There might be other issues with the new lock implementation that comes up along the way, thus as of today its unclear if it will be implemented or not. I think there was also some other potential fix discussed, need to get back on that.

/Per

@korken89
Copy link
Collaborator

Hi,

After today's' meeting we decided that we don't want to give this guarantee of an implementation detail.
As long as you find this useful you are welcome to roll your own fork of RTIC.

Thank you for the effort!

@korken89 korken89 closed this Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Naming lock types
3 participants
@luiswirth @perlindgren @korken89