[PATCH] winebus.sys: Do not report HID report read errors unconditionally
aric at codeweavers.com
Wed Aug 15 12:49:03 CDT 2018
On 8/15/18 1:47 AM, Kai Krakow wrote:
> 2018-08-15 8:10 GMT+02:00 Alexandre Julliard <julliard at winehq.org>:
>> Kai Krakow <kai at kaishome.de> writes:
>>> Device reports may come in faster than our consumers could possibly read
>>> them, this is especially true for multi-axis events: When you move a
>>> stick across its range, it will always generate at least two events, one
>>> for the x axis, and one for the y axis. This is not really an error
>>> situation, so let's report this situation only once at most. If we
>>> already know the multi-event situation, let's skip logging this
>>> completely: We won't loose any information anyway because the report
>>> contains a complete device state and only axis positions were updated.
>>> Thus, this commit adds a parameter to process_hid_report() to let it
>>> know if we are currently processing reports that are known to be sent
>>> multiple times in sequence (with updated reports).
>>> Also, if our consumers are slow to respond, then report the issue only
>>> once and not per each occurrence during the duration of one consumer
>>> read cycle.
>>> Finally, this is not really an error, it's a warning at most, thus
>>> degrade the error message to a warning to not pollute peoples consoles
>>> and logs with unimportant stuff.
>> Is it really necessary to add all that complexity then? Why not simply
>> remove the message?
> I was also thinking about it but I'm not completely sure if there are
> valid cases for the warning to appear. As lined out, it may still
> loose button events if a reader in the queues doesn't read fast
> enough. And I don't know the complete driver stack in detail.
> Discarding the message only for axis events is safe, tho. This is why
> the boolean parameter was added. The other change just reduces the log
> impact in case the warning would be displayed.
> The problem here is that the intermediate layer needs to translate
> between HID events from Linux to full device reports for Windows, so
> it just coalesces events into the report state. But it's not a queue.
> Thus, having the warning may help debugging any problems left.
> But if someone with more insight into the whole driver stack sees no
> more problems, I'm okay with removing the message completely.
> @Aric: What do you think?
I have no strong opinion in that regard. It does seem like a lot of complexity to simply silence a message. I see no more problems with the driver stack and have no problem removing the message if Kai feels like it really is as harmless as it seems.
More information about the wine-devel