Refactor: hidpp20 to use enum (#2647)

* Remove duplicated Param definition

Use constants from hidpp20 constants

Related #2273

* hidpp20/Param: Refactor to use IntEnum

Related #2273

* hidpp20_constants: Refactor to use IntEnum

Related #2273

lib/logitech_receiver/base.py

@@ -595,7 +595,7 @@ def request(
                             devnumber,
                             request_id,
                             error,
-                            hidpp20_constants.ERROR[error],
+                            hidpp20_constants.ErrorCode(error),
                         )
                         raise exceptions.FeatureCallError(
                             number=devnumber,

lib/logitech_receiver/hidpp20.py

@@ -42,12 +42,13 @@ from .common import BatteryLevelApproximation
 from .common import BatteryStatus
 from .common import FirmwareKind
 from .common import NamedInt
-from .hidpp20_constants import CHARGE_LEVEL
 from .hidpp20_constants import CHARGE_STATUS
-from .hidpp20_constants import CHARGE_TYPE
 from .hidpp20_constants import DEVICE_KIND
-from .hidpp20_constants import ERROR
 from .hidpp20_constants import GESTURE
+from .hidpp20_constants import ChargeLevel
+from .hidpp20_constants import ChargeType
+from .hidpp20_constants import ErrorCode
+from .hidpp20_constants import ParamId
 from .hidpp20_constants import SupportedFeature
 
 logger = logging.getLogger(__name__)
@@ -604,16 +605,6 @@ class KeysArrayPersistent(KeysArray):
             logger.warning(f"Key with index {index} was expected to exist but device doesn't report it.")
 
 
-# Param Ids for feature GESTURE_2
-PARAM = common.NamedInts(
-    ExtraCapabilities=1,  # not suitable for use
-    PixelZone=2,  # 4 2-byte integers, left, bottom, width, height; pixels
-    RatioZone=3,  # 4 bytes, left, bottom, width, height; unit 1/240 pad size
-    ScaleFactor=4,  # 2-byte integer, with 256 as normal scale
-)
-PARAM._fallback = lambda x: f"unknown:{x:04X}"
-
-
 class SubParam:
     __slots__ = ("id", "length", "minimum", "maximum", "widget")
 
@@ -632,20 +623,20 @@ class SubParam:
 
 
 SUB_PARAM = {  # (byte count, minimum, maximum)
-    PARAM["ExtraCapabilities"]: None,  # ignore
+    ParamId.EXTRA_CAPABILITIES: None,  # ignore
-    PARAM["PixelZone"]: (  # TODO: replace min and max with the correct values
+    ParamId.PIXEL_ZONE: (  # TODO: replace min and max with the correct values
         SubParam("left", 2, 0x0000, 0xFFFF, "SpinButton"),
         SubParam("bottom", 2, 0x0000, 0xFFFF, "SpinButton"),
         SubParam("width", 2, 0x0000, 0xFFFF, "SpinButton"),
         SubParam("height", 2, 0x0000, 0xFFFF, "SpinButton"),
     ),
-    PARAM["RatioZone"]: (  # TODO: replace min and max with the correct values
+    ParamId.RATIO_ZONE: (  # TODO: replace min and max with the correct values
         SubParam("left", 1, 0x00, 0xFF, "SpinButton"),
         SubParam("bottom", 1, 0x00, 0xFF, "SpinButton"),
         SubParam("width", 1, 0x00, 0xFF, "SpinButton"),
         SubParam("height", 1, 0x00, 0xFF, "SpinButton"),
     ),
-    PARAM["ScaleFactor"]: (SubParam("scale", 2, 0x002E, 0x01FF, "Scale"),),
+    ParamId.SCALE_FACTOR: (SubParam("scale", 2, 0x002E, 0x01FF, "Scale"),),
 }
 
 # Spec Ids for feature GESTURE_2
@@ -765,10 +756,10 @@ class Gesture:
 
 
 class Param:
-    def __init__(self, device, low, high, next_param_index):
+    def __init__(self, device, low: int, high, next_param_index):
         self._device = device
         self.id = low
-        self.param = PARAM[low]
+        self.param = ParamId(low)
         self.size = high & 0x0F
         self.show_in_ui = bool(high & 0x1F)
         self._value = None
@@ -1820,27 +1811,27 @@ def decipher_battery_status(report: FixedBytes5) -> Tuple[Any, Battery]:
     return SupportedFeature.BATTERY_STATUS, Battery(battery_discharge_level, battery_discharge_next_level, status, None)
 
 
-def decipher_battery_voltage(report):
+def decipher_battery_voltage(report: bytes):
     voltage, flags = struct.unpack(">HB", report[:3])
     status = BatteryStatus.DISCHARGING
-    charge_sts = ERROR.unknown
+    charge_sts = ErrorCode.UNKNOWN
-    charge_lvl = CHARGE_LEVEL.average
+    charge_lvl = ChargeLevel.AVERAGE
-    charge_type = CHARGE_TYPE.standard
+    charge_type = ChargeType.STANDARD
     if flags & (1 << 7):
         status = BatteryStatus.RECHARGING
         charge_sts = CHARGE_STATUS[flags & 0x03]
     if charge_sts is None:
-        charge_sts = ERROR.unknown
+        charge_sts = ErrorCode.UNKNOWN
     elif charge_sts == CHARGE_STATUS.full:
-        charge_lvl = CHARGE_LEVEL.full
+        charge_lvl = ChargeLevel.FULL
         status = BatteryStatus.FULL
     if flags & (1 << 3):
-        charge_type = CHARGE_TYPE.fast
+        charge_type = ChargeType.FAST
     elif flags & (1 << 4):
-        charge_type = CHARGE_TYPE.slow
+        charge_type = ChargeType.SLOW
         status = BatteryStatus.SLOW_RECHARGE
     elif flags & (1 << 5):
-        charge_lvl = CHARGE_LEVEL.critical
+        charge_lvl = ChargeLevel.CRITICAL
     for level in battery_voltage_remaining:
         if level[0] < voltage:
             charge_lvl = level[1]

lib/logitech_receiver/hidpp20_constants.py

@@ -173,25 +173,45 @@ DEVICE_KIND = NamedInts(
 )
 
 
-ONBOARD_MODES = NamedInts(MODE_NO_CHANGE=0x00, MODE_ONBOARD=0x01, MODE_HOST=0x02)
+class OnboardMode(IntEnum):
+    MODE_NO_CHANGE = 0x00
+    MODE_ONBOARD = 0x01
+    MODE_HOST = 0x02
+
 
 CHARGE_STATUS = NamedInts(charging=0x00, full=0x01, not_charging=0x02, error=0x07)
 
-CHARGE_LEVEL = NamedInts(average=50, full=90, critical=5)
 
-CHARGE_TYPE = NamedInts(standard=0x00, fast=0x01, slow=0x02)
+class ChargeStatus(IntEnum):
+    CHARGING = 0x00
+    FULL = 0x01
+    NOT_CHARGING = 0x02
+    ERROR = 0x07
+
+
+class ChargeLevel(IntEnum):
+    AVERAGE = 50
+    FULL = 90
+    CRITICAL = 5
+
+
+class ChargeType(IntEnum):
+    STANDARD = 0x00
+    FAST = 0x01
+    SLOW = 0x02
+
+
+class ErrorCode(IntEnum):
+    UNKNOWN = 0x01
+    INVALID_ARGUMENT = 0x02
+    OUT_OF_RANGE = 0x03
+    HARDWARE_ERROR = 0x04
+    LOGITECH_ERROR = 0x05
+    INVALID_FEATURE_INDEX = 0x06
+    INVALID_FUNCTION = 0x07
+    BUSY = 0x08
+    UNSUPPORTED = 0x09
 
-ERROR = NamedInts(
-    unknown=0x01,
-    invalid_argument=0x02,
-    out_of_range=0x03,
-    hardware_error=0x04,
-    logitech_internal=0x05,
-    invalid_feature_index=0x06,
-    invalid_function=0x07,
-    busy=0x08,
-    unsupported=0x09,
-)
 
 # Gesture Ids for feature GESTURE_2
 GESTURE = NamedInts(
@@ -258,11 +278,11 @@ GESTURE = NamedInts(
 )
 GESTURE._fallback = lambda x: f"unknown:{x:04X}"
 
-# Param Ids for feature GESTURE_2
+
-PARAM = NamedInts(
+class ParamId(IntEnum):
-    ExtraCapabilities=1,  # not suitable for use
+    """Param Ids for feature GESTURE_2"""
-    PixelZone=2,  # 4 2-byte integers, left, bottom, width, height; pixels
+
-    RatioZone=3,  # 4 bytes, left, bottom, width, height; unit 1/240 pad size
+    EXTRA_CAPABILITIES = 1  # not suitable for use
-    ScaleFactor=4,  # 2-byte integer, with 256 as normal scale
+    PIXEL_ZONE = 2  # 4 2-byte integers, left, bottom, width, height; pixels
-)
+    RATIO_ZONE = 3  # 4 bytes, left, bottom, width, height; unit 1/240 pad size
-PARAM._fallback = lambda x: f"unknown:{x:04X}"
+    SCALE_FACTOR = 4  # 2-byte integer, with 256 as normal scale

lib/logitech_receiver/settings_templates.py

@@ -38,6 +38,7 @@ from . import hidpp20_constants
 from . import settings
 from . import special_keys
 from .hidpp10_constants import Registers
+from .hidpp20_constants import ParamId
 
 logger = logging.getLogger(__name__)
 
@@ -46,7 +47,6 @@ _DK = hidpp10_constants.DEVICE_KIND
 _F = hidpp20_constants.SupportedFeature
 
 _GG = hidpp20_constants.GESTURE
-_GP = hidpp20_constants.PARAM
 
 
 class State(enum.Enum):
@@ -1296,10 +1296,10 @@ _GESTURE2_GESTURES_LABELS = {
 }
 
 _GESTURE2_PARAMS_LABELS = {
-    _GP["ExtraCapabilities"]: (None, None),  # not supported
+    ParamId.EXTRA_CAPABILITIES: (None, None),  # not supported
-    _GP["PixelZone"]: (_("Pixel zone"), None),  # TO DO: replace None with a short description
+    ParamId.PIXEL_ZONE: (_("Pixel zone"), None),  # TO DO: replace None with a short description
-    _GP["RatioZone"]: (_("Ratio zone"), None),  # TO DO: replace None with a short description
+    ParamId.RATIO_ZONE: (_("Ratio zone"), None),  # TO DO: replace None with a short description
-    _GP["ScaleFactor"]: (_("Scale factor"), _("Sets the cursor speed.")),
+    ParamId.SCALE_FACTOR: (_("Scale factor"), _("Sets the cursor speed.")),
 }
 
 _GESTURE2_PARAMS_LABELS_SUB = {
@@ -1351,7 +1351,7 @@ class Gesture2Params(settings.LongSettings):
     description = _("Change numerical parameters of a mouse/touchpad.")
     feature = _F.GESTURE_2
     rw_options = {"read_fnid": 0x70, "write_fnid": 0x80}
-    choices_universe = hidpp20.PARAM
+    choices_universe = hidpp20_constants.ParamId
     sub_items_universe = hidpp20.SUB_PARAM
     # item (NamedInt) -> list/tuple of objects that have the following attributes
     # .id (sub-item text), .length (in bytes), .minimum and .maximum

lib/solaar/cli/show.py

@@ -237,7 +237,7 @@ def _print_device(dev, num=None):
             elif feature == SupportedFeature.REMAINING_PAIRING:
                 print(f"            Remaining Pairings: {int(_hidpp20.get_remaining_pairing(dev))}")
             elif feature == SupportedFeature.ONBOARD_PROFILES:
-                if _hidpp20.get_onboard_mode(dev) == hidpp20_constants.ONBOARD_MODES.MODE_HOST:
+                if _hidpp20.get_onboard_mode(dev) == hidpp20_constants.OnboardMode.MODE_HOST:
                     mode = "Host"
                 else:
                     mode = "On-Board"

tests/logitech_receiver/test_hidpp20_complex.py

@@ -534,10 +534,10 @@ def test_Gesture_set(responses, gest, enabled, diverted, set_result, unset_resul
 @pytest.mark.parametrize(
     "responses, prm, id, index, size, value, default_value, write1, write2",
     [
-        (fake_hidpp.responses_gestures, 4, common.NamedInt(4, "ScaleFactor"), 0, 2, 256, 256, "0080", "0180"),
+        (fake_hidpp.responses_gestures, 4, hidpp20_constants.ParamId.SCALE_FACTOR, 0, 2, 256, 256, "0080", "0180"),
     ],
 )
-def test_Param(responses, prm, id, index, size, value, default_value, write1, write2):
+def test_param(responses, prm, id, index, size, value, default_value, write1, write2):
     device = fake_hidpp.Device("GESTURE", responses=responses, feature=hidpp20_constants.SupportedFeature.GESTURE_2)
     gestures = _hidpp20.get_gestures(device)
 
@@ -548,7 +548,7 @@ def test_Param(responses, prm, id, index, size, value, default_value, write1, wr
     assert param.size == size
     assert param.value == value
     assert param.default_value == default_value
-    assert str(param) == id
+    assert param.param == id
     assert int(param) == id
     assert param.write(bytes.fromhex(write1)).hex().upper() == f"{index:02X}" + write1 + "FF"
     assert param.write(bytes.fromhex(write2)).hex().upper() == f"{index:02X}" + write2 + "FF"