device: regularize and improve battery status gathering and reporting

lib/logitech_receiver/device.py

@@ -341,6 +341,19 @@ class Device:
             self._persister = _configuration.persister(self)
         return self._persister
 
+    def battery(self):  # None  or  level, next, status, voltage
+        if self.protocol < 2.0:
+            return _hidpp10.get_battery(self)
+        else:
+            battery_feature = self.persister.get('_battery', None) if self.persister else None
+            if battery_feature != 0:
+                result = _hidpp20.get_battery(self, battery_feature)
+                if result:
+                    feature, level, next, status, voltage = result
+                    if self.persister and battery_feature is None:
+                        self.persister['_battery'] = feature
+                    return level, next, status, voltage
+
     def enable_connection_notifications(self, enable=True):
         """Enable or disable device (dis)connection notifications on this
         receiver."""

lib/logitech_receiver/hidpp10.py

@@ -265,8 +265,8 @@ def parse_battery_status(register, reply):
             # some 'charging' notifications may come with no battery level information
             charge = None
 
-        # Return None for next charge level as this is not in HID++ 1.0 spec
+        # Return None for next charge level and voltage as these are not in HID++ 1.0 spec
-        return charge, status_text, None
+        return charge, None, status_text, None
 
 
 def get_firmware(device):

lib/logitech_receiver/hidpp20.py

@@ -1216,27 +1216,32 @@ def get_friendly_name(device):
         return name.decode('utf-8')
 
 
-def get_battery(device):
+def get_battery_status(device):
-    """Reads a device's battery level."""
+    report = feature_request(device, FEATURE.BATTERY_STATUS)
-    battery = feature_request(device, FEATURE.BATTERY_STATUS)
+    if report:
-    if battery:
+        return decipher_battery_status(report)
-        discharge, next, status = _unpack('!BBB', battery[:3])
-        discharge = None if discharge == 0 else discharge
-        status = BATTERY_STATUS[status]
-        if _log.isEnabledFor(_DEBUG):
-            _log.debug('device %d battery %s%% charged, next %s%%, status %s', device.number, discharge, next, status)
-        return discharge, status, next
-    else:
-        battery = feature_request(device, FEATURE.UNIFIED_BATTERY, 0x10)
-        if battery:
-            return decipher_unified_battery(battery)
 
 
-def decipher_unified_battery(report):
+def decipher_battery_status(report):
+    discharge, next, status = _unpack('!BBB', report[:3])
+    discharge = None if discharge == 0 else discharge
+    status = BATTERY_STATUS[status]
+    if _log.isEnabledFor(_DEBUG):
+        _log.debug('battery status %s%% charged, next %s%%, status %s', discharge, next, status)
+    return FEATURE.BATTERY_STATUS, discharge, next, status, None
+
+
+def get_battery_unified(device):
+    report = feature_request(device, FEATURE.UNIFIED_BATTERY, 0x10)
+    if report is not None:
+        return decipher_battery_unified(report)
+
+
+def decipher_battery_unified(report):
     discharge, level, status, _ignore = _unpack('!BBBB', report[:4])
     status = BATTERY_STATUS[status]
     if _log.isEnabledFor(_DEBUG):
-        _log.debug('battery %s%% charged, level %s, charging %s', discharge, level, status)
+        _log.debug('battery unified %s%% charged, level %s, charging %s', discharge, level, status)
     level = (
         _BATTERY_APPROX.full if level == 8  # full
         else _BATTERY_APPROX.good if level == 4  # good
@@ -1244,13 +1249,7 @@ def decipher_unified_battery(report):
         else _BATTERY_APPROX.critical if level == 1  # critical
         else _BATTERY_APPROX.empty
     )
-    return discharge if discharge else level, status, None
+    return FEATURE.UNIFIED_BATTERY, discharge if discharge else level, None, status, None
-
-
-def get_voltage(device):
-    battery_voltage = feature_request(device, FEATURE.BATTERY_VOLTAGE)
-    if battery_voltage:
-        return decipher_voltage(battery_voltage)
 
 
 # voltage to remaining charge from Logitech
@@ -1272,14 +1271,18 @@ battery_voltage_remaining = (
 )
 
 
-# modified to be much closer to battery reports
+def get_battery_voltage(device):
-def decipher_voltage(voltage_report):
+    report = feature_request(device, FEATURE.BATTERY_VOLTAGE)
-    voltage, flags = _unpack('>HB', voltage_report[:3])
+    if report is not None:
+        return decipher_battery_voltage(report)
+
+
+def decipher_battery_voltage(report):
+    voltage, flags = _unpack('>HB', report[:3])
     status = BATTERY_STATUS.discharging
     charge_sts = ERROR.unknown
     charge_lvl = CHARGE_LEVEL.average
     charge_type = CHARGE_TYPE.standard
-
     if flags & (1 << 7):
         status = BATTERY_STATUS.recharging
         charge_sts = CHARGE_STATUS[flags & 0x03]
@@ -1295,19 +1298,39 @@ def decipher_voltage(voltage_report):
         status = BATTERY_STATUS.slow_recharge
     elif (flags & (1 << 5)):
         charge_lvl = CHARGE_LEVEL.critical
-
     for level in battery_voltage_remaining:
         if level[0] < voltage:
             charge_lvl = level[1]
             break
-
     if _log.isEnabledFor(_DEBUG):
         _log.debug(
-            'device ???, battery voltage %d mV, charging = %s, charge status %d = %s, charge level %s, charge type %s',
+            'battery voltage %d mV, charging %s, status %d = %s, level %s, type %s', voltage, status, (flags & 0x03),
-            voltage, status, (flags & 0x03), charge_sts, charge_lvl, charge_type
+            charge_sts, charge_lvl, charge_type
         )
+    return FEATURE.BATTERY_VOLTAGE, charge_lvl, None, status, voltage
 
-    return charge_lvl, status, voltage, charge_sts, charge_type
+
+battery_functions = {
+    FEATURE.BATTERY_STATUS: get_battery_status,
+    FEATURE.BATTERY_VOLTAGE: get_battery_unified,
+    FEATURE.UNIFIED_BATTERY: get_battery_voltage,
+}
+
+
+def get_battery(device, feature):
+    """Return battery information - feature, approximate level, next, charging, voltage"""
+    if feature is not None:
+        battery_function = battery_functions.get(feature, None)
+        if battery_function:
+            result = battery_function(device)
+            if result:
+                return result
+    else:
+        for battery_function in battery_functions.values():
+            result = battery_function(device)
+            if result:
+                return result
+    return 0, None, None, None, None
 
 
 def get_keys(device):

lib/logitech_receiver/notifications.py

@@ -235,7 +235,7 @@ def _process_hidpp10_custom_notification(device, status, n):
         assert n.data[-1:] == b'\x00'
         data = chr(n.address).encode() + n.data
         charge, status_text, next_charge = _hidpp10.parse_battery_status(n.sub_id, data)
-        status.set_battery_info(charge, status_text, next_charge)
+        status.set_battery_info(charge, next_charge, status_text, None)
         return True
 
     if n.sub_id == _R.keyboard_illumination:
@@ -315,11 +315,8 @@ def _process_feature_notification(device, status, n, feature):
 
     if feature == _F.BATTERY_STATUS:
         if n.address == 0x00:
-            discharge_level = ord(n.data[:1])
+            _ignore, discharge_level, discharge_next_level, battery_status, voltage = _hidpp20.decipher_battery_status(n.data)
-            discharge_level = None if discharge_level == 0 else discharge_level
+            status.set_battery_info(discharge_level, discharge_next_level, battery_status, voltage)
-            discharge_next_level = ord(n.data[1:2])
-            battery_status = ord(n.data[2:3])
-            status.set_battery_info(discharge_level, _hidpp20.BATTERY_STATUS[battery_status], discharge_next_level)
         elif n.address == 0x10:
             if _log.isEnabledFor(_INFO):
                 _log.info('%s: spurious BATTERY status %s', device, n)
@@ -328,15 +325,15 @@ def _process_feature_notification(device, status, n, feature):
 
     elif feature == _F.BATTERY_VOLTAGE:
         if n.address == 0x00:
-            battery_level, battery_status, battery_voltage, _ignore, _ignore = _hidpp20.decipher_voltage(n.data)
+            _ignore, level, next, status, voltage = _hidpp20.decipher_battery_voltage(n.data)
-            status.set_battery_info(battery_level, battery_status, None, battery_voltage)
+            status.set_battery_info(level, next, status, voltage)
         else:
             _log.warn('%s: unknown VOLTAGE %s', device, n)
 
     elif feature == _F.UNIFIED_BATTERY:
         if n.address == 0x00:
-            battery_level, battery_status, battery_voltage = _hidpp20.decipher_unified_battery(n.data)
+            _ignore, level, next, status, voltage = _hidpp20.decipher_battery_unified(n.data)
-            status.set_battery_info(battery_level, battery_status, None, battery_voltage)
+            status.set_battery_info(level, next, status, voltage)
         else:
             _log.warn('%s: unknown UNIFIED BATTERY %s', device, n)
 
@@ -348,12 +345,12 @@ def _process_feature_notification(device, status, n, feature):
             status_text = _hidpp20.BATTERY_STATUS.discharging
             if n.address == 0x00:
                 status[_K.LIGHT_LEVEL] = None
-                status.set_battery_info(charge, status_text, None)
+                status.set_battery_info(charge, None, status_text, None)
             elif n.address == 0x10:
                 status[_K.LIGHT_LEVEL] = lux
                 if lux > 200:
                     status_text = _hidpp20.BATTERY_STATUS.recharging
-                status.set_battery_info(charge, status_text, None)
+                status.set_battery_info(charge, None, status_text, None)
             elif n.address == 0x20:
                 if _log.isEnabledFor(_DEBUG):
                     _log.debug('%s: Light Check button pressed', device)

lib/logitech_receiver/status.py

@@ -190,7 +190,7 @@ class DeviceStatus(dict):
 
     __nonzero__ = __bool__
 
-    def set_battery_info(self, level, status, nextLevel=None, voltage=None, timestamp=None):
+    def set_battery_info(self, level, nextLevel, status, voltage, timestamp=None):
         if _log.isEnabledFor(_DEBUG):
             _log.debug('%s: battery %s, %s', self._device, level, status)
 
@@ -245,35 +245,14 @@ class DeviceStatus(dict):
     # Retrieve and regularize battery status
     def read_battery(self, timestamp=None):
         if self._active:
-            d = self._device
+            assert self._device
-            assert d
+            battery = self._device.battery()
-
-            if d.protocol < 2.0:
-                battery = _hidpp10.get_battery(d)
-                self.set_battery_keys(battery)
-                return
-
-            battery = _hidpp20.get_battery(d)
-            if battery is None:
-                v = _hidpp20.get_voltage(d)
-                if v is not None:
-                    level, status, voltage, _ignore, _ignore = v
-                    self.set_battery_keys((level, status, None), voltage)
-                    return
-
-            # Really unnecessary, if the device has SOLAR_DASHBOARD it should be
-            # broadcasting it's battery status anyway, it will just take a little while.
-            # However, when the device has just been detected, it will not show
-            # any battery status for a while (broadcasts happen every 90 seconds).
-            if battery is None and d.features and _hidpp20.FEATURE.SOLAR_DASHBOARD in d.features:
-                d.feature_request(_hidpp20.FEATURE.SOLAR_DASHBOARD, 0x00, 1, 1)
-                return
             self.set_battery_keys(battery)
 
-    def set_battery_keys(self, battery, voltage=None):
+    def set_battery_keys(self, battery):
         if battery is not None:
-            level, status, nextLevel = battery
+            level, nextLevel, status, voltage = battery
-            self.set_battery_info(level, status, nextLevel, voltage)
+            self.set_battery_info(level, nextLevel, status, voltage)
         elif self.get(KEYS.BATTERY_STATUS, None) is not None:
             self[KEYS.BATTERY_STATUS] = None
             self[KEYS.BATTERY_CHARGING] = None

lib/solaar/cli/show.py

@@ -67,21 +67,16 @@ def _battery_text(level):
 
 
 def _battery_line(dev):
-    battery = _hidpp20.get_battery(dev)
+    battery = dev.battery()
-    if battery is None:
-        battery = _hidpp10.get_battery(dev)
     if battery is not None:
-        level, status, nextLevel = battery
+        level, nextLevel, status, voltage = battery
         text = _battery_text(level)
+        if voltage is not None:
+            text = text + (' %smV ' % voltage)
         nextText = '' if nextLevel is None else ', next level ' + _battery_text(nextLevel)
         print('     Battery: %s, %s%s.' % (text, status, nextText))
     else:
-        battery_voltage = _hidpp20.get_voltage(dev)
+        print('     Battery status unavailable.')
-        if battery_voltage:
-            (level, status, voltage, charge_sts, charge_type) = battery_voltage
-            print('     Battery: %smV, %s, %s.' % (voltage, status, level))
-        else:
-            print('     Battery status unavailable.')
 
 
 def _print_device(dev, num=None):
@@ -225,7 +220,7 @@ def _print_device(dev, num=None):
                 else:
                     mode = 'On-Board'
                 print('            Device Mode: %s' % mode)
-            elif feature in (_F.BATTERY_STATUS, _F.BATTERY_VOLTAGE, _F.BATTERY_VOLTAGE):
+            elif _hidpp20.battery_functions.get(feature, None):
                 print('', end='       ')
                 _battery_line(dev)
             for setting in dev_settings: