From 8eee66e192103d1d17cdd4d80e488528f8c0be3b Mon Sep 17 00:00:00 2001 From: MattHag <16444067+MattHag@users.noreply.github.com> Date: Sat, 16 Nov 2024 14:46:45 +0100 Subject: [PATCH] Remove NamedInts: Convert NotificationFlag to flag Related #2273 --- lib/logitech_receiver/device.py | 15 ++-- lib/logitech_receiver/hidpp10.py | 5 +- lib/logitech_receiver/hidpp10_constants.py | 92 ++++++++++++++-------- lib/logitech_receiver/receiver.py | 8 +- lib/solaar/listener.py | 2 +- tests/logitech_receiver/test_hidpp10.py | 4 +- 6 files changed, 80 insertions(+), 46 deletions(-) diff --git a/lib/logitech_receiver/device.py b/lib/logitech_receiver/device.py index 7ee9b85efb..caa9080c85 100644 --- 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 index 6d4ee95702..29399c8caf 100644 --- 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 index 92b788638f..d6306cc901 100644 --- 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 index 29f2abb2bc..a7ef1ede3f 100644 --- 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 index 76a4b1afde..1011a7406d 100644 --- 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 index 6280ae278d..1c39b0c057 100644 --- 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