enable configuring the bInterval on hid devices#1087
enable configuring the bInterval on hid devices#1087koltenpearson wants to merge 1 commit intomicropython:masterfrom
Conversation
andrewleech
left a comment
andrewleech
left a comment
There was a problem hiding this comment.
Good find, yes this interval controls the rate of messages sent to the host which can affect the responsiveness of the hid device.
Do you think a comment on the line or brief description elsewhere would help others understand what this does? I know lots of other things here aren't particularly documented either though...
Regardless there's a ci failure noting the commit message needs a little fixup to match standard project guidance.
koltenpearson
force-pushed
the
master
branch
5 times, most recently
from
d67a936 to
c3117ec
Compare
Signed-off-by: kolten <koltenpearson@fastmail.com>
koltenpearson
force-pushed
the
master
branch
from
c3117ec to
36a3ece
Compare
|
Finally got it happy with the commit message. I added a small comment as well |
| set_report_buf=None, | ||
| protocol=_INTERFACE_PROTOCOL_NONE, | ||
| interface_str=None, | ||
| binterval=8, |
There was a problem hiding this comment.
I suggest calling this new parameter just interval. The "b" means it's a byte (ie maximum 255) and I don't think that adds any value here. Using interval matches the style of the other parameters here.
(Other bits of the lower-level code use bInterval with an uppercase I, but this here is a higher-level API and I think interval is best here.)
There was a problem hiding this comment.
I would push back a little and say interval is a very generic name. Googling binterval gives you exactly what this is in the first results. Likewise it is referred to as bInterval by linux at least in all its tooling and logs.
That seems more valuable to me than consistency with the other parameter's style.
There was a problem hiding this comment.
I would push back a little and say interval is a very generic name. Googling binterval gives you exactly what this is in the first results. Likewise it is referred to as bInterval by linux at least in all its tooling and logs.
That seems more valuable to me than consistency with the other parameter's style.
That's fair enough, and I agree that searching for "binterval" gives you a lot of information about this parameter.
But by the same reasoning, protocol that's already used here is also very generic.
Given the context (USB HID device), I feel like it's quite clear what interval means, and it's great you also documented it below, so that it is clear that it's the polling rate. Maybe in the documentation it could refer to it being used as the bInterval parameter?
3 participants
In the process of creating a usb keyboard using the pi pico, I was not able to match the amount of HID reports that the qmk firmware could send, and eventually tracked it down to the bInterval of the qmk firmware being 1, while this one defaults to 8.
So yeah, this will let us configure it, rather than having it hardcoded