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

Ergonomics of Interface::new device parameter #929

Open
TomCrypto opened this issue May 13, 2024 · 4 comments
Open

Ergonomics of Interface::new device parameter #929

TomCrypto opened this issue May 13, 2024 · 4 comments

Comments

@TomCrypto
Copy link

Currently the Interface struct needs a Device to construct itself via Interface::new, but all it does with it is call capabilities on it. In some cases (e.g. embedded code) we would like to instantiate the Interface before the device object can be constructed, even though we typically know the device capabilities upfront.

Should Interface::new directly take a DeviceCapabilities parameter instead of a Device?

@chenzhuoyu
Copy link

@TomCrypto I worked around this problem by creating a "dummy" device with the only purpose of providing the DeviceCapabilities, and it works fine. But I am certainly wish there be a better solution.

@thvdveld
Copy link
Contributor

We indeed don't use the device when creating the interface, only the capabilities. We could indeed modify the function to take the capabilities instead of the device. However, I'm not sure about the implications. Maybe the reason why it was taking in a device was to have ownership of it. But I'm not sure if that is really necessary. Pinging @whitequark and @Dirbaio for this.

@Dirbaio
Copy link
Member

Dirbaio commented May 24, 2024

Originally Interface would permanently own the Device. Later I changed it to taking &mut Device in method calls. It's true creation only needs the DeviceCapabilities, but I thought taking the DeviceCapabilities directly could be footgunny, so I kept the entire Device. A footgun example is if new takes DeviceCapabilities it "looks like" you can specify/set the capabilities yourself to anything you want, while in reality you're obligated to pass the device's, and you'll get weird incorrect behavior if you don't.

I'm not opposed to the change if others think it's a good idea, it just makes me a bit uncomfortable.

@whitequark
Copy link
Contributor

I'm not opposed to the change if others think it's a good idea, it just makes me a bit uncomfortable.

I agree. I think originally I had it own the Device because then correctness is guaranteed by construction. But also, isn't it true that if you pass a &mut Device it could be a different one each time?

One possibility is adding a debug_assert!() that the device capabilities match each time you poll.

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

No branches or pull requests

5 participants