Hi On Thu, 8 Dec 2022 at 21:59, Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote: > > 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 Yes, thanks, I completely missed that. Lets continue discussion on the patches. Thanks! David
