fwupd
Devices with several physical connections
Is your feature request related to a problem? Please describe.
While working on https://github.com/fwupd/fwupd/pull/7488 I faced the unsupported use case:
- the target device could be connected simultaneously via 2.4GHz dongle and directly via USB
- both devices are recognized properly as independent devices
- the only common characteristic is Serial number, which could be used to determine that the both identified device are the same device in reality, connected via different physical methods
- the device does not allow to be updated via 2.4GHz mode, only via direct connection.
Device is detected as
├─ Headset (direct USB):
├─ 2.4GHz USB Receiver:
│ └─ Headset via 2.4GHz USB Receiver:
Where Headset (direct USB) and Headset via 2.4GHz USB Receiver are the same device, connected simultaneously to the host.
PS I've tried to use equivalent ID and with https://github.com/fwupd/fwupd/pull/7740 the connected device replaces the tunneled one on detach. With some additional hacks I'm even able to replace the child and flash the device, but unable to recreate the tunneled device after physical detaching of headset from USB -- the parent device shows the chiled already exists but not shown.
Describe the solution you'd like
- Have some API to mark several detected devices as the same single device, i.e. different connections for the same device
- Set the priority for those connections, allowing to select the suitable/best/fastest way to update
- Those connections might have different protocols for update, for instance plugin qc-s5gen2 is using different bottom transport layer for USB and BLE updates.
- Wait for the new connection to be appeared and substitute the current one, for example ask user to plug the USB cable.
- After unplugging any connection point (i.e. unplud device frm USB) the best available connection should be used to communicate with the device.
PS For me it sounds as several proxies for the single device.
Describe alternatives you've considered
Unable to get proper behavior with the simple devices replacement. It is mostly working if I manage devices on plugin level, however @hughsie do not like this approach (and I agree) since the other plugin could not work with the same device properly. Hense the solution is needed on the engine level.
The child device removal is not working properly for the tunneled device, since it has no physical connection and never triggered for removal from the old devices list. Thus preventing the correct restoring of that kind of the device.
Use cases
Here I try to describe use cases based on my experience with https://github.com/fwupd/fwupd/pull/7488, what I see for now and what I expect to see.
-
Plug Headset USB; dongle is not plugged. Detection and update of the single device works as expected.
-
Plug Headset USB and dongle later. 2.1. Currently 3 devices detected:
├─ Headset (direct USB): ├─ 2.4GHz USB Receiver: │ └─ Headset via 2.4GHz USB Receiver:2.2. Expected: 2 devices detected, USB headset is a child of the dongle:
├─ 2.4GHz USB Receiver: │ └─ Headset (direct USB):2.2.1. That works with equivalent ID set and patch from https://github.com/fwupd/fwupd/pull/7740, but only if the USB headset is detected after 2.4Ghz connection.
-
Plug the dongle and later USB Headset 3.1. Currently 3 devices detected:
├─ Headset (direct USB): ├─ 2.4GHz USB Receiver: │ └─ Headset via 2.4GHz USB Receiver:3.2. Expected: 2 devices detected, USB headset is a child of the dongle:
├─ 2.4GHz USB Receiver: │ └─ Headset (direct USB):3.2.1. That works with equivalent ID set and patch from https://github.com/fwupd/fwupd/pull/7740, but only if the USB headset is detected after 2.4Ghz connection. -
The dongle and USB Headset are plugged, unplug the USB Headset 4.1. Currently not supported, with the patch from https://github.com/fwupd/fwupd/pull/7740 only the dongle is detected:
├─ 2.4GHz USB Receiver:in the same time, parent contains the
Headset via 2.4GHz USB Receiverin a children list returned fromfu_device_get_children()4.2. Expected: 2 devices detected, 2.4GHz headset is returned as a child of the dongle:├─ 2.4GHz USB Receiver: │ └─ Headset via 2.4GHz USB Receiver: -
Update the
Headset via 2.4GHz USB Receiver5.1. Ask user to plug the device via USB cable 5.2. The plugged device should replace the child 5.3. During update, if device is set in bootloader mode it should be recognized via counterparts and/or equivalent ID or other mechanisms. 5.4. After the unplugging from USB, the device must fallback to 2.4GHz connection:├─ 2.4GHz USB Receiver: │ └─ Headset via 2.4GHz USB Receiver:
@d4s alternative proposal. First some caveats:
- I think if we have the device connected with two different protocols (perhaps handled in two different plugins) then we need the device list to have two active devices (but see below). The logic is they're both devices that can come and go independently and may have different children (or no children at all) depending on the protocol used to update them. This also means they can both have different proxies to avoid overloading what a proxy means.
- I think the device list returning the highest priority device like we added in 7cc3b202e42b48050ff4559d03ead826b36dbb82 makes a lot of sense -- this means the plugin can say "hey, I'm the same as this other thing" and let the daemon do the lifecycling and choosing the best device for a given .cab file.
So I think what we have now could actually work pretty well. What the plugin would need to do would be to hook the ->device_registered() vfunc and watch for the better (e.g. wired) device appearing, and if it does, set the equivalent ID on the wired device of the wireless device (e.g. using the serial number to match them). This means that when the user clicks the "update" button on the wireless device the daemon actually chooses the wired device to do the update. Where this might break down a little is connecting up the progress reporting, but helpfully we already do that little dance for proxies so it might just work too.
I think that actually gives us the best of both worlds, where the plugin can device how to match up the "other device" but the actual write_firmware can be handled in whatever plugin with whatever proxy implementation.
I think the bit we're missing is adding a FwupdDeviceProblem on the worse when the better device is active. That will stop the wired device appearing as a new device in the UX as the daemon will push it from UPDATABLE to UPDATABLE_HIDDEN. If all that makes sense to you, say, and I'll start adding the tests to check that FWUPD_DEVICE_PROBLEM_BETTER_DEVICE_ACTIVE is added when we have the equivalent ID set with a higher priority.
There is another device on the market that works similar to this; the Dell WD19TB4. The thunderbolt NVM can be flashed both from the thunderbolt plugin or the intel-usb4 plugin. Also the synaptics MST hub can be flashed from the dell-dock plugin or the synaptics-mst plugin.
I wonder if what you're looking for is FWUPD_PLUGIN_RULE_BETTER_HAN?
This is a little more granular -- i.e. you could have one plugin with two devices, where one is faster than the other. What might make sense is to replace the FWUPD_PLUGIN_RULE_BETTER_THAN with the FWUPD_DEVICE_PROBLEM_LOWER_PRIORITY magic -- IIRC we also have a third way of doing it in the USI dock with the usb-blocked inhibit -- and we can certainly replace that with the new problem if nothing else.
Sure; I think if they can be reported we could tear out that plugin rule while breaking ABI too.
@hughsie I did a lot of experiments with all patches we have for now. Still not working as expected.
Also I set the proper eqivalent_id in plugin_class->device_registered() as you suggested and it is set correctly. But the behavior is still similar to that I've seen before.
I did another try to create a self-test, but fail again since it is not working as I expected in too many places.
So, please check the sketch of one use case with my comments inlined in commit https://github.com/fwupd/fwupd/commit/3f3439924db26d06501dfe8855eb23cdfab96878
btw, what if instead of shadowing devices with equivalent id, I'll try another approach, set virtual parent and update it via any available child (by priority):
dongle (grandpa) -> virtual device (parent) -> device1 (physical connection1)
|-> device2 (physical connection2)
|-> device3 (physical connection3)
just a rough idea without real testing yet.
btw, what if instead of shadowing devices with equivalent id, I'll try another approach, set virtual parent and update it via any available child (by priority):
dongle (grandpa) -> virtual device (parent) -> device1 (physical connection1) |-> device2 (physical connection2) |-> device3 (physical connection3)just a rough idea without real testing yet.
Or another idea instead of relationship you can use a proxy
Or another idea instead of relationship you can use a proxy
I thought about it. But I already using proxies to distinguish devices in https://github.com/fwupd/fwupd/pull/7488. And that might be a problem to get the list of available proxies and set/unset them dynamically.
@d4s aha, so I didn't think of this -- I thought you'd want to do something like fu_device_set_equivalent_id(dongle, fu_device_get_id(direct1)); -- i.e. the direct connection is better than the dongle, rather than better than the child. Is the logic that the dongle could be connected to other {different} devices that are not available on the wired interface?
Aha, I see what's confusing us both. I don't actually think it makes sense to change the parent-child relationship with the direct connection. e.g. you'd have:
add dongle
add child of dongle
add direct -- using fu_device_set_equivalent_id(direct1, fu_device_get_id(child)) -- rather than the other way around
and so we'd have three active devices, and the dongle->child would stay the same regardless of the direct device being added or removed. When we call install on the child-of-dongle we'd find the equivalent_id of the direct connection and use that instead. I'm guessing the direct connection doesn't need child devices as it's a 1:1 mapping? If we do need to copy parameters from child-of-dongle to the direct device we can certainly add that as an explicit vfunc invocation like we already do FuDevice->replace().