Skip to content

enable configuring the bInterval on hid devices #1087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
koltenpearson wants to merge 1 commit into
base: master
Choose a base branch
from
+5 −1
Open

enable configuring the bInterval on hid devices #1087

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion micropython/usb/usb-device-hid/usb/device/hid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def __init__(
set_report_buf=None,
protocol=_INTERFACE_PROTOCOL_NONE,
interface_str=None,
binterval=8,
Copy link
Member

@dpgeorge dpgeorge Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Author

@koltenpearson koltenpearson Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@dpgeorge dpgeorge Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

):
# Construct a new HID interface.
#
Expand All @@ -85,12 +86,15 @@ def __init__(
# - protocol can be set to a specific value as per HID v1.11 section 4.3 Protocols, p9.
#
# - interface_str is an optional string descriptor to associate with the HID USB interface.
#
# - binterval this is the polling rate the device will request the host use in milliseconds.
super().__init__()
self.report_descriptor = report_descriptor
self.extra_descriptors = extra_descriptors
self._set_report_buf = set_report_buf
self.protocol = protocol
self.interface_str = interface_str
self.binterval = binterval

self._int_ep = None # set during enumeration

Expand Down Expand Up @@ -150,7 +154,7 @@ def desc_cfg(self, desc, itf_num, ep_num, strs):
# Add the typical single USB interrupt endpoint descriptor associated
# with a HID interface.
self._int_ep = ep_num | _EP_IN_FLAG
desc.endpoint(self._int_ep, "interrupt", 8, 8)
desc.endpoint(self._int_ep, "interrupt", 8, self.binterval)

self.idle_rate = 0

Expand Down
Loading