-
Notifications
You must be signed in to change notification settings - Fork 1k
[embassy-usb-dfu] support function level WinUSB GUIDs #4188
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
base: main
Are you sure you want to change the base?
Conversation
This commit makes it possible to provide function level msos GUIDs to usb_dfu. This helps to ensure that composite DFU devices automatically get assigned the WinUSB driver on Windows.
This PR is ready for review now. After more extensive physical testing it turned out the comments I left were not entirely accurate, but that has been corrected now. |
embassy-usb-dfu/src/application.rs
Outdated
winusb_guids: Option<&'d [&str]>, | ||
) { | ||
let mut func = builder.function(0x00, 0x00, 0x00); | ||
if let Some(winusb_guids) = winusb_guids { | ||
// We add MSOS headers so that the device automatically gets assigned the WinUSB driver on Windows. | ||
// Otherwise users need to do this manually using a tool like Zadig. | ||
// | ||
// Adding them here on the function level appears to only be needed for compositive devices. | ||
// In addition to being on the function level, they should also be added to the device level. | ||
// | ||
func.msos_feature(msos::CompatibleIdFeatureDescriptor::new("WINUSB", "")); | ||
func.msos_feature(msos::RegistryPropertyFeatureDescriptor::new( | ||
"DeviceInterfaceGUIDs", | ||
msos::PropertyData::RegMultiSz(winusb_guids), | ||
)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this have been a closure that gets the func passed, instead of an option parameter that seems very specific to winusb?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion, thanks! I can't see use-case for customizing func
further right now, but it's definitely a more future proof approach. I'll give that approach a shot and see if it works out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have now updated this to use a closure instead. Unfortunately, I could not manage to put the closure inside an Option
though since that results in all sorts of lifetime hell...
This provides a more generic interface for users to customise the DFU function instead of restricting customisation to DFU headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This PR makes it possible to provide function level msos GUIDs to usb_dfu. This helps to ensure that composite DFU devices automatically get assigned the WinUSB driver on Windows.