refactor: replace ERROR NamedInts by IntEnum (#2645)

* refactoring(logitech_receiver/notifications): change to enums PairingError and BoltPairingError * refactoring(logitech_receiver/notifications): change to enums PairingError and BoltPairingError (Fix pre-commit checks) * refactor(logitech_receiver/base.py): create unit tests for ping function before replacing ERRORNamedInts by IntEnum * refactor(logitech_receiver/base.py): create unit tests for request function before replacing ERROR NamedInts by IntEnum * refactor(logitech_receiver/base.py): create unit tests for ping function before replacing ERRORNamedInts by IntEnum (add exclusion for macOS) * refactor(logitech_receiver/base.py): create unit tests for ping function before replacing ERRORNamedInts by IntEnum (fix for python < 3.10) * refactor(solaar/cli./probe.py): create unit tests for run function before replacing ERROR NamedInts by IntEnum (focusing on the call order when receiving errors) * refactor(solaar/cli./probe.py): refactor register processing to handle short and long registers in a single loop structure for improved readability and reduced code duplication. * refactor(logitech_receiver/hidpp10_constants.py): replace ERROR NamedInt by IntEnum. * refactor(logitech_receiver/hidpp10_constants.py): distinguish hidpp10 and hidpp20 errors in the code for readibility. * refactor(logitech_receiver/hidpp20_constants.py): replace ERROR NamedInt by IntEnum. * refactor(logitech_receiver/hidpp20_constants.py): replace ERROR NamedInt by IntEnum. (fix problem with | operator when typing with python 3.8) * feature(hide on startup option): Visual test (not binded yet) DRAFT * refactor(solaar/cli./probe.py): create unit tests for run function before replacing ERROR NamedInts by IntEnum (focusing on the call order when receiving errors) * refactor(solaar/cli./probe.py): refactor register processing to handle short and long registers in a single loop structure for improved readability and reduced code duplication. * refactor(logitech_receiver/hidpp10_constants.py): replace ERROR NamedInt by IntEnum. * refactor(logitech_receiver/hidpp20_constants.py): replace ERROR NamedInt by IntEnum. * refactor(logitech_receiver/hidpp20_constants.py): replace ERROR NamedInt by IntEnum. (fix problem with | operator when typing with python 3.8) * feature(hide on startup option): Visual test (not binded yet) DRAFT * Merge: Refactor: hidpp20 to use enum * Merge: Refactor: hidpp20 to use enum (fix test) --------- Co-authored-by: some_developer <[email protected]>
pwr-Solaar · Nov 2, 2024 · a19461b · a19461b
1 parent c90146d
commit a19461b
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 64 deletions.
diff --git a/lib/logitech_receiver/base.py b/lib/logitech_receiver/base.py
@@ -35,10 +35,10 @@
 from . import common
 from . import descriptors
 from . import exceptions
-from . import hidpp10_constants
-from . import hidpp20_constants
 from .common import LOGITECH_VENDOR_ID
 from .common import BusID
+from .hidpp10_constants import ErrorCode as Hidpp10ErrorCode
+from .hidpp20_constants import ErrorCode as Hidpp20ErrorCode
 
 if typing.TYPE_CHECKING:
     import gi
@@ -583,9 +583,9 @@ def request(
                                 devnumber,
                                 request_id,
                                 error,
-                                hidpp10_constants.ERROR[error],
+                                Hidpp10ErrorCode(error),
                             )
-                        return hidpp10_constants.ERROR[error] if return_error else None
+                        return Hidpp10ErrorCode(error) if return_error else None
                     if reply_data[:1] == b"\xff" and reply_data[1:3] == request_data[:2]:
                         # a HID++ 2.0 feature call returned with an error
                         error = ord(reply_data[3:4])
@@ -595,7 +595,7 @@ def request(
                             devnumber,
                             request_id,
                             error,
-                            hidpp20_constants.ErrorCode(error),
+                            Hidpp20ErrorCode(error),
                         )
                         raise exceptions.FeatureCallError(
                             number=devnumber,
@@ -676,15 +676,12 @@ def ping(handle, devnumber, long_message: bool = False):
                         and reply_data[1:3] == request_data[:2]
                     ):  # error response
                         error = ord(reply_data[3:4])
-                        if error == hidpp10_constants.ERROR.invalid_SubID__command:
+                        if error == Hidpp10ErrorCode.INVALID_SUB_ID_COMMAND:
                             # a valid reply from a HID++ 1.0 device
                             return 1.0
-                        if (
-                            error == hidpp10_constants.ERROR.resource_error
-                            or error == hidpp10_constants.ERROR.connection_request_failed
-                        ):
+                        if error in [Hidpp10ErrorCode.RESOURCE_ERROR, Hidpp10ErrorCode.CONNECTION_REQUEST_FAILED]:
                             return  # device unreachable
-                        if error == hidpp10_constants.ERROR.unknown_device:  # no paired device with that number
+                        if error == Hidpp10ErrorCode.UNKNOWN_DEVICE:  # no paired device with that number
                             logger.error("(%s) device %d error on ping request: unknown device", handle, devnumber)
                             raise exceptions.NoSuchDevice(number=devnumber, request=request_id)
 

diff --git a/lib/logitech_receiver/hidpp10_constants.py b/lib/logitech_receiver/hidpp10_constants.py
@@ -87,20 +87,20 @@
     threed_gesture=0x000001,
 )
 
-ERROR = NamedInts(
-    invalid_SubID__command=0x01,
-    invalid_address=0x02,
-    invalid_value=0x03,
-    connection_request_failed=0x04,
-    too_many_devices=0x05,
-    already_exists=0x06,
-    busy=0x07,
-    unknown_device=0x08,
-    resource_error=0x09,
-    request_unavailable=0x0A,
-    unsupported_parameter_value=0x0B,
-    wrong_pin_code=0x0C,
-)
+
+class ErrorCode(IntEnum):
+    INVALID_SUB_ID_COMMAND = 0x01
+    INVALID_ADDRESS = 0x02
+    INVALID_VALUE = 0x03
+    CONNECTION_REQUEST_FAILED = 0x04
+    TOO_MANY_DEVICES = 0x05
+    ALREADY_EXISTS = 0x06
+    BUSY = 0x07
+    UNKNOWN_DEVICE = 0x08
+    RESOURCE_ERROR = 0x09
+    REQUEST_UNAVAILABLE = 0x0A
+    UNSUPPORTED_PARAMETER_VALUE = 0x0B
+    WRONG_PIN_CODE = 0x0C
 
 
 class PairingError(IntEnum):

diff --git a/lib/solaar/cli/probe.py b/lib/solaar/cli/probe.py
@@ -15,8 +15,8 @@
 ## 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 from logitech_receiver import base
-from logitech_receiver import hidpp10_constants
 from logitech_receiver.common import strhex
+from logitech_receiver.hidpp10_constants import ErrorCode
 from logitech_receiver.hidpp10_constants import Registers
 
 from solaar.cli.show import _print_device
@@ -95,31 +95,18 @@ def run(receivers, args, find_receiver, _ignore):
 
     print("")
     for reg in range(0, 0xFF):
-        last = None
-        for sub in range(0, 0xFF):
-            rgst = base.request(receiver.handle, 0xFF, 0x8100 | reg, sub, return_error=True)
-            if isinstance(rgst, int) and rgst == hidpp10_constants.ERROR.invalid_address:
-                break
-            elif isinstance(rgst, int) and rgst == hidpp10_constants.ERROR.invalid_value:
-                continue
-            else:
-                if not isinstance(last, bytes) or not isinstance(rgst, bytes) or last != rgst:
-                    print(
-                        "    Register Short   %#04x %#04x: %s"
-                        % (reg, sub, "0x" + strhex(rgst) if isinstance(rgst, bytes) else str(rgst))
-                    )
-            last = rgst
-        last = None
-        for sub in range(0, 0xFF):
-            rgst = base.request(receiver.handle, 0xFF, 0x8100 | (0x200 + reg), sub, return_error=True)
-            if isinstance(rgst, int) and rgst == hidpp10_constants.ERROR.invalid_address:
-                break
-            elif isinstance(rgst, int) and rgst == hidpp10_constants.ERROR.invalid_value:
-                continue
-            else:
-                if not isinstance(last, bytes) or not isinstance(rgst, bytes) or last != rgst:
-                    print(
-                        "    Register Long    %#04x %#04x: %s"
-                        % (reg, sub, "0x" + strhex(rgst) if isinstance(rgst, bytes) else str(rgst))
-                    )
-            last = rgst
+        for offset, reg_type in [(0x00, "Short"), (0x200, "Long")]:
+            last = None
+            for sub in range(0, 0xFF):
+                rgst = base.request(receiver.handle, 0xFF, 0x8100 | (offset + reg), sub, return_error=True)
+                if isinstance(rgst, int) and rgst == ErrorCode.INVALID_ADDRESS:
+                    break
+                elif isinstance(rgst, int) and rgst == ErrorCode.INVALID_VALUE:
+                    continue
+                else:
+                    if not isinstance(last, bytes) or not isinstance(rgst, bytes) or last != rgst:
+                        print(
+                            "    Register %s   %#04x %#04x: %s"
+                            % (reg_type, reg, sub, "0x" + strhex(rgst) if isinstance(rgst, bytes) else str(rgst))
+                        )
+                last = rgst
diff --git a/tests/logitech_receiver/test_base.py b/tests/logitech_receiver/test_base.py
@@ -1,6 +1,7 @@
 import struct
 import sys
 
+from typing import Union
 from unittest import mock
 
 import pytest
@@ -9,7 +10,8 @@
 from logitech_receiver import exceptions
 from logitech_receiver.base import HIDPP_SHORT_MESSAGE_ID
 from logitech_receiver.base import request
-from logitech_receiver.hidpp10_constants import ERROR
+from logitech_receiver.hidpp10_constants import ErrorCode as Hidpp10Error
+from logitech_receiver.hidpp20_constants import ErrorCode as Hidpp20Error
 
 
 @pytest.mark.parametrize(
@@ -125,12 +127,14 @@ def test_get_next_sw_id():
 @pytest.mark.parametrize(
     "prefix, error_code, return_error, raise_exception",
     [
-        (b"\x8f", ERROR.invalid_SubID__command, False, False),
-        (b"\x8f", ERROR.invalid_SubID__command, True, False),
-        (b"\xff", ERROR.invalid_SubID__command, False, True),
+        (b"\x8f", Hidpp10Error.INVALID_SUB_ID_COMMAND, False, False),
+        (b"\x8f", Hidpp10Error.INVALID_SUB_ID_COMMAND, True, False),
+        (b"\xff", Hidpp20Error.UNKNOWN, False, True),
     ],
 )
-def test_request_errors(prefix: bytes, error_code: ERROR, return_error: bool, raise_exception: bool):
+def test_request_errors(
+    prefix: bytes, error_code: Union[Hidpp10Error, Hidpp20Error], return_error: bool, raise_exception: bool
+):
     handle = 0
     device_number = 66
 
@@ -160,13 +164,13 @@ def test_request_errors(prefix: bytes, error_code: ERROR, return_error: bool, ra
 @pytest.mark.parametrize(
     "simulated_error, expected_result",
     [
-        (ERROR.invalid_SubID__command, 1.0),
-        (ERROR.resource_error, None),
-        (ERROR.connection_request_failed, None),
-        (ERROR.unknown_device, exceptions.NoSuchDevice),
+        (Hidpp10Error.INVALID_SUB_ID_COMMAND, 1.0),
+        (Hidpp10Error.RESOURCE_ERROR, None),
+        (Hidpp10Error.CONNECTION_REQUEST_FAILED, None),
+        (Hidpp10Error.UNKNOWN_DEVICE, exceptions.NoSuchDevice),
     ],
 )
-def test_ping_errors(simulated_error: ERROR, expected_result):
+def test_ping_errors(simulated_error: Hidpp10Error, expected_result):
     handle = 1
     device_number = 1
 

diff --git a/tests/solaar/ui/test_probe.py b/tests/solaar/ui/test_probe.py
@@ -0,0 +1,53 @@
+from unittest import mock
+
+from logitech_receiver.hidpp10_constants import ErrorCode
+from logitech_receiver.hidpp10_constants import Registers
+from solaar.cli.probe import run
+
+
+# Mock receiver class
+class MockReceiver:
+    handle = 1
+    isDevice = False
+
+    def read_register(self, register, *args):
+        return 0 if register == Registers.RECEIVER_INFO else b"\x01\x03"
+
+
+def test_run_register_errors():
+    mock_args = mock.Mock()
+    mock_args.receiver = False
+
+    mock_receiver = MockReceiver()
+
+    # Define expected addresses to be called in order
+    expected_addresses = []
+
+    for reg in range(0, 0xFF):
+        expected_addresses.append((0x8100 | reg, 0))  # First short call, returns invalid_value (continue)
+        expected_addresses.append((0x8100 | reg, 1))  # Second short call, returns invalid_address (stop here)
+
+        expected_addresses.append((0x8100 | (0x200 + reg), 0))  # First long call, returns invalid_value (continue)
+        expected_addresses.append((0x8100 | (0x200 + reg), 1))  # Second long call, returns invalid_address (stop here)
+
+    # To record the actual addresses called
+    called_addresses = []
+
+    def mock_base_request(handle, devnumber, reg, sub, return_error=False):
+        called_addresses.append((reg, sub))
+        if sub == 0:
+            return ErrorCode.INVALID_VALUE
+        elif sub == 1:
+            return ErrorCode.INVALID_ADDRESS
+        return b"\x01\x02"
+
+    with mock.patch("logitech_receiver.base.request", side_effect=mock_base_request), mock.patch(
+        "solaar.cli.probe._print_receiver", return_value=None
+    ):
+        # Call the run function with mocked receivers and args (passing real find_receiver function)
+        run([mock_receiver], mock_args, None, None)
+
+        # Evaluate that the addresses called match the expected addresses
+        assert (
+            called_addresses == expected_addresses
+        ), f"Called addresses {called_addresses} do not match expected {expected_addresses}"