Remove NamedInts: Convert NotificationFlag to flag

Related #2273
pwr-Solaar · Nov 19, 2024 · 8eee66e · 8eee66e
1 parent c728e1d
commit 8eee66e
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 46 deletions.
diff --git a/lib/logitech_receiver/device.py b/lib/logitech_receiver/device.py
@@ -39,6 +39,7 @@
 from . import settings_templates
 from .common import Alert
 from .common import Battery
+from .hidpp10_constants import NotificationFlag
 from .hidpp20_constants import SupportedFeature
 
 if typing.TYPE_CHECKING:
@@ -468,10 +469,8 @@ def enable_connection_notifications(self, enable=True):
 
         if enable:
             set_flag_bits = (
-                hidpp10_constants.NOTIFICATION_FLAG.battery_status
-                | hidpp10_constants.NOTIFICATION_FLAG.ui
-                | hidpp10_constants.NOTIFICATION_FLAG.configuration_complete
-            )
+                NotificationFlag.BATTERY_STATUS | NotificationFlag.UI | NotificationFlag.CONFIGURATION_COMPLETE
+            ).value
         else:
             set_flag_bits = 0
         ok = _hidpp10.set_notification_flags(self, set_flag_bits)
@@ -480,8 +479,12 @@ def enable_connection_notifications(self, enable=True):
 
         flag_bits = _hidpp10.get_notification_flags(self)
         if logger.isEnabledFor(logging.INFO):
-            flag_names = None if flag_bits is None else tuple(hidpp10_constants.NOTIFICATION_FLAG.flag_names(flag_bits))
-            logger.info("%s: device notifications %s %s", self, "enabled" if enable else "disabled", flag_names)
+            if flag_bits is None:
+                flag_names = None
+            else:
+                flag_names = hidpp10_constants.NotificationFlag.flag_names(flag_bits)
+            is_enabled = "enabled" if enable else "disabled"
+            logger.info(f"{self}: device notifications {is_enabled} {flag_names}")
         return flag_bits if ok else None
 
     def add_notification_handler(self, id: str, fn):

diff --git a/lib/logitech_receiver/hidpp10.py b/lib/logitech_receiver/hidpp10.py
@@ -26,6 +26,7 @@
 from .common import BatteryLevelApproximation
 from .common import BatteryStatus
 from .common import FirmwareKind
+from .hidpp10_constants import NotificationFlag
 from .hidpp10_constants import Registers
 
 logger = logging.getLogger(__name__)
@@ -190,7 +191,7 @@ def set_3leds(self, device: Device, battery_level=None, charging=None, warning=N
     def get_notification_flags(self, device: Device):
         return self._get_register(device, Registers.NOTIFICATIONS)
 
-    def set_notification_flags(self, device: Device, *flag_bits):
+    def set_notification_flags(self, device: Device, *flag_bits: NotificationFlag):
         assert device is not None
 
         # Avoid a call if the device is not online,
@@ -200,7 +201,7 @@ def set_notification_flags(self, device: Device, *flag_bits):
             if device.protocol and device.protocol >= 2.0:
                 return
 
-        flag_bits = sum(int(b) for b in flag_bits)
+        flag_bits = sum(int(b.value) for b in flag_bits)
         assert flag_bits & 0x00FFFFFF == flag_bits
         result = write_register(device, Registers.NOTIFICATIONS, common.int2bytes(flag_bits, 3))
         return result is not None

diff --git a/lib/logitech_receiver/hidpp10_constants.py b/lib/logitech_receiver/hidpp10_constants.py
@@ -16,7 +16,9 @@
 ## 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 from __future__ import annotations
 
+from enum import Flag
 from enum import IntEnum
+from typing import List
 
 from .common import NamedInts
 
@@ -58,37 +60,61 @@ class PowerSwitchLocation(IntEnum):
     BOTTOM_EDGE = 0x0C
 
 
-# Some flags are used both by devices and receivers. The Logitech documentation
-# mentions that the first and last (third) byte are used for devices while the
-# second is used for the receiver. In practise, the second byte is also used for
-# some device-specific notifications (keyboard illumination level). Do not
-# simply set all notification bits if the software does not support it. For
-# example, enabling keyboard_sleep_raw makes the Sleep key a no-operation unless
-# the software is updated to handle that event.
-# Observations:
-# - wireless and software present were seen on receivers, reserved_r1b4 as well
-# - the rest work only on devices as far as we can tell right now
-# In the future would be useful to have separate enums for receiver and device notification flags,
-# but right now we don't know enough.
-# additional flags taken from https://drive.google.com/file/d/0BxbRzx7vEV7eNDBheWY0UHM5dEU/view?usp=sharing
-NOTIFICATION_FLAG = NamedInts(
-    numpad_numerical_keys=0x800000,
-    f_lock_status=0x400000,
-    roller_H=0x200000,
-    battery_status=0x100000,  # send battery charge notifications (0x07 or 0x0D)
-    mouse_extra_buttons=0x080000,
-    roller_V=0x040000,
-    power_keys=0x020000,  # system control keys such as Sleep
-    keyboard_multimedia_raw=0x010000,  # consumer controls such as Mute and Calculator
-    multi_touch=0x001000,  # notify on multi-touch changes
-    software_present=0x000800,  # software is controlling part of device behaviour
-    link_quality=0x000400,  # notify on link quality changes
-    ui=0x000200,  # notify on UI changes
-    wireless=0x000100,  # notify when the device wireless goes on/off-line
-    configuration_complete=0x000004,
-    voip_telephony=0x000002,
-    threed_gesture=0x000001,
-)
+class NotificationFlag(Flag):
+    """Some flags are used both by devices and receivers.
+
+    The Logitech documentation mentions that the first and last (third)
+    byte are used for devices while the second is used for the receiver.
+    In practise, the second byte is also used for some device-specific
+    notifications (keyboard illumination level). Do not simply set all
+    notification bits if the software does not support it. For example,
+    enabling keyboard_sleep_raw makes the Sleep key a no-operation
+    unless the software is updated to handle that event.
+
+    Observations:
+    - wireless and software present seen on receivers,
+    reserved_r1b4 as well
+    - the rest work only on devices as far as we can tell right now
+    In the future would be useful to have separate enums for receiver
+    and device notification flags, but right now we don't know enough.
+    Additional flags taken from https://drive.google.com/file/d/0BxbRzx7vEV7eNDBheWY0UHM5dEU/view?usp=sharing
+    """
+
+    @classmethod
+    def flag_names(cls, flag_bits: int) -> List[str]:
+        """Extract the names of the flags from the integer."""
+        indexed = {item.value: item.name for item in cls}
+
+        flag_names = []
+        unknown_bits = flag_bits
+        for k in indexed:
+            # Ensure that the key (flag value) is a power of 2 (a single bit flag)
+            assert bin(k).count("1") == 1
+            if k & flag_bits == k:
+                unknown_bits &= ~k
+                flag_names.append(indexed[k].replace("_", " ").lower())
+
+        # Yield any remaining unknown bits
+        if unknown_bits != 0:
+            flag_names.append(f"unknown:{unknown_bits:06X}")
+        return flag_names
+
+    NUMPAD_NUMERICAL_KEYS = 0x800000
+    F_LOCK_STATUS = 0x400000
+    ROLLER_H = 0x200000
+    BATTERY_STATUS = 0x100000  # send battery charge notifications (0x07 or 0x0D)
+    MOUSE_EXTRA_BUTTONS = 0x080000
+    ROLLER_V = 0x040000
+    POWER_KEYS = 0x020000  # system control keys such as Sleep
+    KEYBOARD_MULTIMEDIA_RAW = 0x010000  # consumer controls such as Mute and Calculator
+    MULTI_TOUCH = 0x001000  # notify on multi-touch changes
+    SOFTWARE_PRESENT = 0x000800  # software is controlling part of device behaviour
+    LINK_QUALITY = 0x000400  # notify on link quality changes
+    UI = 0x000200  # notify on UI changes
+    WIRELESS = 0x000100  # notify when the device wireless goes on/off-line
+    CONFIGURATION_COMPLETE = 0x000004
+    VOIP_TELEPHONY = 0x000002
+    THREED_GESTURE = 0x000001
 
 
 def flags_to_str(flag_bits: int | None, fallback: str) -> str:
@@ -97,8 +123,8 @@ def flags_to_str(flag_bits: int | None, fallback: str) -> str:
         if flag_bits == 0:
             flag_names = (fallback,)
         else:
-            flag_names = list(NOTIFICATION_FLAG.flag_names(flag_bits))
-    return f"\n{' ':15}".join(flag_names)
+            flag_names = NotificationFlag.flag_names(flag_bits)
+    return f"\n{' ':15}".join(sorted(flag_names))
 
 
 class ErrorCode(IntEnum):

diff --git a/lib/logitech_receiver/receiver.py b/lib/logitech_receiver/receiver.py
@@ -36,6 +36,7 @@
 from .common import Alert
 from .common import Notification
 from .device import Device
+from .hidpp10_constants import NotificationFlag
 from .hidpp10_constants import Registers
 
 if typing.TYPE_CHECKING:
@@ -172,7 +173,7 @@ def enable_connection_notifications(self, enable=True):
             return False
 
         if enable:
-            set_flag_bits = hidpp10_constants.NOTIFICATION_FLAG.wireless | hidpp10_constants.NOTIFICATION_FLAG.software_present
+            set_flag_bits = NotificationFlag.WIRELESS | NotificationFlag.SOFTWARE_PRESENT
         else:
             set_flag_bits = 0
         ok = _hidpp10.set_notification_flags(self, set_flag_bits)
@@ -181,7 +182,10 @@ def enable_connection_notifications(self, enable=True):
             return None
 
         flag_bits = _hidpp10.get_notification_flags(self)
-        flag_names = None if flag_bits is None else tuple(hidpp10_constants.NOTIFICATION_FLAG.flag_names(flag_bits))
+        if flag_bits is None:
+            flag_names = None
+        else:
+            flag_names = hidpp10_constants.NotificationFlag.flag_names(flag_bits)
         if logger.isEnabledFor(logging.INFO):
             logger.info("%s: receiver notifications %s => %s", self, "enabled" if enable else "disabled", flag_names)
         return flag_bits

diff --git a/lib/solaar/listener.py b/lib/solaar/listener.py
@@ -72,7 +72,7 @@ def has_started(self):
             logger.info("%s: notifications listener has started (%s)", self.receiver, self.receiver.handle)
         nfs = self.receiver.enable_connection_notifications()
         if logger.isEnabledFor(logging.WARNING):
-            if not self.receiver.isDevice and not ((nfs if nfs else 0) & hidpp10_constants.NOTIFICATION_FLAG.wireless):
+            if not self.receiver.isDevice and not ((nfs if nfs else 0) & hidpp10_constants.NotificationFlag.WIRELESS.value):
                 logger.warning(
                     "Receiver on %s might not support connection notifications, GUI might not show its devices",
                     self.receiver.path,

diff --git a/tests/logitech_receiver/test_hidpp10.py b/tests/logitech_receiver/test_hidpp10.py
@@ -255,7 +255,7 @@ def test_set_notification_flags(mocker):
     spy_request = mocker.spy(device, "request")
 
     result = _hidpp10.set_notification_flags(
-        device, hidpp10_constants.NOTIFICATION_FLAG.battery_status, hidpp10_constants.NOTIFICATION_FLAG.wireless
+        device, hidpp10_constants.NotificationFlag.BATTERY_STATUS, hidpp10_constants.NotificationFlag.WIRELESS
     )
 
     spy_request.assert_called_once_with(0x8000 | Registers.NOTIFICATIONS, b"\x10\x01\x00")
@@ -267,7 +267,7 @@ def test_set_notification_flags_bad(mocker):
     spy_request = mocker.spy(device, "request")
 
     result = _hidpp10.set_notification_flags(
-        device, hidpp10_constants.NOTIFICATION_FLAG.battery_status, hidpp10_constants.NOTIFICATION_FLAG.wireless
+        device, hidpp10_constants.NotificationFlag.BATTERY_STATUS, hidpp10_constants.NotificationFlag.WIRELESS
     )
 
     assert spy_request.call_count == 0