devanlai / webdfu

WebUSB Device Firmware Upgrade example
https://devanlai.github.io/webdfu/dfu-util/
ISC License
277 stars 95 forks source link

Issue SET_INTERFACE even if alt=0 #14

Closed smunaut closed 3 years ago

smunaut commented 3 years ago

Some devices with several alt settings for the DFU interface expect a call to SET_INTERFACE even if the alt settings you want to use is the default 0 one.

dfu-util issues SET_INTERFACE all the time to deal with that, but seems the web impl doesn.

devanlai commented 3 years ago

Hi @smunaut, thanks for bringing this up. At a glance, it looks like dfu-util uses the following rules:

  1. If there are multiple alt settings on the DFU interface, always select the alt setting for DFU
  2. If the device is in run-time mode and the DFU interface is not on interface 0, select the alt setting for DFU even if there is only alt setting.

That should be pretty straightforward to implement here, though I'll have to do some testing to make sure it doesn't break DFU for any of the devices I have lying around. Looking through the dfu-util bug tracker, it seems like the libusb SET_INTERFACE support sometimes breaks on some versions on some platforms.

smunaut commented 3 years ago

So a SET_INTERFACE on a interface with a single alt setting is allowed to fail, so if it does, you should ignore it. It's perfectly valid for a device to STALL on that.

devanlai commented 3 years ago

It makes sense that a device might STALL the request, but from the dfu-util comments, the implication seems to be that some devices might just crash in response to an unexpected SET_INTERFACE request. I'm not sure whether that's something that the dfu-util authors have actually encountered in the wild or if it's a just-in-case thing.

devanlai commented 3 years ago

Hi @smunaut, I've started #15 to try to address this.

From testing on Windows and Linux, it seems that on Windows the OS automagically sends the SET_INTERFACE request to select alternate 0 and subsequently the USB backend suppresses any redundant SET_INTERFACE requests. On Linux it sends SET_INTERFACE requests only when requested.

smunaut commented 3 years ago

I tested on the test website linked in the PR and this worked for me here (on linux).

smunaut commented 3 years ago

As a side note, it's also be nice to be able to skip the reset() at the end of download phase. Devices with multiple interfaces can be because you have multiple part the firmware or multiple MCU etc ... that need to be updated together / stay in sync and so you download fw #0, then fw #1 and then only you reset.

devanlai commented 3 years ago

Hmm, I see, I guess I could refactor do_download() to only reset the device in certain states instead of always resetting it. I'll add it to my list of things to do if I ever get around to making improvements again.