Add PartialEq, Eq and Hash to DeviceInfo

[alvaro-cuesta]

Also added transitively to BusType and WcharString. As far as I can tell this is safe to do since their equality relation is total.

A simple change for consumer QoL. I am trying to poll the list of devices and need this to track changes on the list of devices (libusb hotplug is not yet available in Windows, so I have to resort to polling, and I have to use HID anyways).

I didn't particularly need hash (yet) but it felt right to add it semantically, and I can foresee me using the DeviceInfo in HashSet or HashMap at some point.

[ruabmbua]

I am not sure if its a good idea to implement Eq operator directly for this struct.
Out of interest, what is you usecase for it?

[alvaro-cuesta]

You mean Eq in particular or both PartialEq + Eq?

For Eq in particular I think the equality relation is total so you're supposed to implement Eq, right? So just semantics, no particular use case.

For rationale on PartialEq see comment in PR description. I'm already using it (and Hash through HashSet) to track devices over time.

Is there any other way to uniquely identify a device? Am I supposed to just compare all relevant fields individually?

[alvaro-cuesta]

Actually I think I'm using strict Eq too through HashSet::difference (bound on its impl Iterator).

[ruabmbua]

My concern with implementing Eq (as an operator overload) is, that it is unclear what device equality means. If we implement it for the DeviceInfo struct, it just means that one instance is equal in terms of data representation to the

I fear people might misinterpret that as a way to check if two devices are the same, but due to differences in the platforms it might not quite reflect reality. Even if you check all the platform implementations now and see no problem, you have to take newer versions into account. Can you guarantee that all new fields we may want to add in the future will not break this assumption?

A much better way to do this would be an explicit method with documentation, that can be used to identify if two devices are the same. But even that is kind of hard to pull off, since two different applications might have different expectations about device equality.

USB is a bit of a pain in that regard in general, with devices having cloned serial numbers, certain hubs screwing up usb descriptors ....

[ruabmbua]

Maybe we want multiple methods. One of them for the more general case of trying to providing equality between actual physical devices. (e.g. detecting that the same keyboard was plugged into a different port, but you can still tell its the same).

Another that just looks at the topology / ports etc...

[alvaro-cuesta]

Great insights!

I didn't consider the "physical device" perspective -- I was thinking in hidapi hid_device_info terms, in particular because I've e.g. learnt not to trust S/N (it's "" for my particular device)...

The problem with having explicit methods instead of implementing traits is the consumer would still have to newtype if they want to use the struct for e.g. HashSet, but I see your point.

Just thinking out loud here: a solution that won't force consumer to newtype around the desired method might be to split some of the fields into a inner physical-device-related struct, so that consumer could use that one for Partial/Eq/Hash, or the whole DeviceInfo, as desired. That'd be a breaking change though, and I'm not sure it addresses your future-proofing concerns.

---

Would you still accept if I rework this PR only for WcharString and BusType, so I can easily newtype DeviceInfo for now? I think those should be safe, right?

[alvaro-cuesta]

For reference: libusb/hidapi#291 implemented stable paths in hidapi libusb backend but signal11/hidapi#405 doesn't seem like it's been addressed in libusb/hidapi (and I guess just depends on whatever the underlying hidraw driver does). No idea about other backends' guarantees (I can only confirm Windows keep the path stable for device+USB port across reconnections).

This means equality in DeviceInfo would only make semantic sense if it is meant to have strictly the same meaning as hid_device_info in hidapi, i.e. identify a particular HID path+device (as in "connected device" this is what DeviceInfo is, am I right?), but cannot tell anything about physical devices or even reconnections of the same device on same USB port (would be nice to document this non-guarantee regardless of outcome).

DeviceInfo equality could still be useful for e.g. tracking this "connected device" across time. I think this would be needed for Windows hotplug which only reports that there are new devices, but not which (at least with WM_DEVICECHANGE, unsure about CM_Register_Notification), also might be useful for 1+2 here so you can rule out the duplicate. Whether that warrants implementing the traits is debatable.