Only explicitly clear already enqueued HID reports

Commit 2050d0b ("Traverse all hid/hidraw queued reports and return the
last one") aggressively started to clear all enqueued reports whenever a
read was done on a HID using hidapi.

While that seemed to work ok, it could cause important reports to be
lost, which might eventually break some drivers.  As no contributor has
enough devices to test all drivers, it is better to be conservative and
only explicitly clear enqueued reports.

Notably, it is wiser to clear the queue before any writes that precede
one or more reads.  This will clear stale data that has already been
queued, but will not risk loosing expected responses.  And drivers that
expect reads to match specific writes should already be prepared to
handle what is left.

The objective is not definitively clear all previous reports, but simply
to avoid acting on (or forwarding to the user) stale data.  The subtle
difference is that _stale_ presumes a delay of more than infinitesimal
length.

Related: #87 ("1.3.2 regression: kraken_two.get_status() returns wrong
fan RPM value")
This commit is contained in:
Jonas Malaco 2020-02-17 00:40:56 -03:00
parent 5de9973cb7
commit fd4ccd70d0
5 changed files with 37 additions and 11 deletions

View File

@ -111,6 +111,7 @@ class CorsairHidPsuDriver(UsbHidDriver):
Note: replies before calling this function appear to follow the
pattern <address> <cte 0xfe> <zero> <zero> <padding...>.
"""
self.device.clear_enqueued_reports()
self._write([0xfe, 0x03]) # not well understood
self._read()
mode = OCPMode.SINGLE_RAIL if single_12v_ocp else OCPMode.MULTI_RAIL
@ -128,6 +129,7 @@ class CorsairHidPsuDriver(UsbHidDriver):
Returns a list of `(property, value, unit)` tuples.
"""
self.device.clear_enqueued_reports()
ret = self._exec(WriteBit.WRITE, CMD.PAGE, [0])
if ret[1] == 0xfe:
LOGGER.warning('possibly uninitialized device')

View File

@ -245,13 +245,15 @@ class KrakenTwoDriver(UsbHidDriver):
def supports_cooling_profiles(self):
if self._supports_cooling_profiles is None:
if self.supports_cooling:
self._read()
self._read(clear_first=False)
self._supports_cooling_profiles = self._firmware_version >= (3, 0, 0)
else:
self._supports_cooling_profiles = False
return self._supports_cooling_profiles
def _read(self):
def _read(self, clear_first=True):
if clear_first:
self.device.clear_enqueued_reports()
msg = self.device.read(_READ_LENGTH)
self.device.release()
LOGGER.debug('received %s', ' '.join(format(i, '02x') for i in msg))

View File

@ -264,6 +264,7 @@ class SmartDeviceDriver(CommonSmartDeviceDriver):
"""
status = []
noise = []
self.device.clear_enqueued_reports()
for i, _ in enumerate(self._speed_channels):
msg = self.device.read(self._READ_LENGTH)
LOGGER.debug('received %s', ' '.join(format(i, '02x') for i in msg))
@ -402,6 +403,7 @@ class SmartDeviceV2Driver(CommonSmartDeviceDriver):
Returns a list of (key, value, unit) tuples.
"""
self.device.clear_enqueued_reports()
# initialize
self._write([0x60, 0x02, 0x01, 0xE8, 0x03, 0x01, 0xE8, 0x03])
self._write([0x60, 0x03])
@ -450,6 +452,7 @@ class SmartDeviceV2Driver(CommonSmartDeviceDriver):
rpm_offset += 2
status.append(('Noise level', msg[noise_offset], 'dB'))
self.device.clear_enqueued_reports()
self._read_until({b'\x67\x02': parse_fan_info})
self.device.release()
return sorted(status)

View File

@ -74,6 +74,7 @@ class SeasonicEDriver(UsbHidDriver):
Returns a list of `(property, value, unit)` tuples.
"""
self.device.clear_enqueued_reports()
fw_human, fw_cam = self._get_fw_versions()
status = [
('Temperature', self._get_float(CMD.READ_TEMPERATURE_2), '°C'),

View File

@ -352,12 +352,24 @@ class PyUsbHid(PyUsbDevice):
This change (while unorthodox) unifies the behavior of read() and write()
between PyUsbHid and HidapiDevice.
Additionally, clear_enqueued_reports() is provided for compatibility with
HidapiDevice.
"""
def __init__(self, usbdev):
super().__init__(usbdev)
self.hidin = 0x81
self.hidout = 0x1 # FIXME apart from NZXT HIDs, usually ctrl (0x0)
def clear_enqueued_reports(self):
"""Clear already enqueued incoming reports.
This methodis available for compatibitily with HidapiDevice, but here
it is as a no-op since we always directly read from the device, and
thus avoid any queuing of reports at the OS level.
"""
pass
def read(self, length):
"""Read raw report from HID."""
return self.usbdev.read(self.hidin, length, timeout=0)
@ -411,18 +423,24 @@ class HidapiDevice:
"""NOOP."""
pass
def clear_enqueued_reports(self):
"""Clear already enqueued incoming reports.
The OS generally enqueues incomming reports for open HIDs, and hidapi
emulates this when running on top of libusb. On Linux, up to 64
reports can be enqueued.
This method quickly reads and discards any already enqueued reports,
and is useful when later reads are not expected to return stale data.
"""
self.hiddev.set_nonblocking(True)
while self.hiddev.read(1):
pass
def read(self, length):
"""Read raw report from HID."""
self.hiddev.set_nonblocking(False)
ret = self.hiddev.read(length)
# the OS queues incoming reports, but we rather just have the last one
self.hiddev.set_nonblocking(True)
while True:
tmp = self.hiddev.read(length)
if not tmp:
break
ret = tmp
return ret
return self.hiddev.read(length)
def write(self, data):
"""Write raw report to HID."""