On Thu, Dec 8, 2022 at 7:46 AM David Rheinsberg <david.rheinsberg@xxxxxxxxx> wrote: > > Hi > > On Mon, 5 Dec 2022 at 22:04, Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote: > > I'm working on a firmware of a device that exposes a HID interface via > > USB and/or BLE and uses, among other things, non-numbered feature > > reports. Included in this series are two paches I had to create in > > order for hidraw devices created for aforementioned subsystems to > > behave in the same way when exerciesd by the same test tool. > > > > I don't know if the patches are acceptable as-is WRT to not breaking > > existing userspace, hence the RFC tag. > > Can you elaborate why you remove the special handling from USBHID but > add it to UHID? They both operate logically on the same level, so > shouldn't we simply adjust uhid to include the report-id in buf[0]? > > Also, you override buf[0] in UHID, so I wonder what UHID currently > returns there? > > IOW, can you elaborate a bit what the current behavior of each of the > involved modules is, and what behavior you would expect? This would > allow to better understand what you are trying to achieve. The more > context you can give, the easier it is to understand what happens > there. > Sorry it's not very clear, so the difference between the cases is that in the case of UHID the report ID ends up being included as a part of "SET_FEATURE", so BlueZ checks UHID_DEV_NUMBERED_FEATURE_REPORTS, which is not set (correctly) and tries to send the whole payload. This ends up as a maxlen + 1 (extra byte) write to a property that is maxlen long, which gets rejected by device's BLE stack. In the case of USBHID the problem happens in "GET_FEATURE" path. When userspace reads the expected data back it gets an extra 0 prepended to the payload, so all of the actual payload has an offset of 1. This doesn't happen with UHID, which I think is the correct behavior here. Hopefully that explains the difference, let me know if something is unclear
