Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor hidpp and introduce classes #2346

Merged
merged 5 commits into from
Mar 2, 2024
Merged

Conversation

MattHag
Copy link
Collaborator

@MattHag MattHag commented Feb 28, 2024

No description provided.

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 28, 2024

Next,

  • Extend test of hidpp modules with fake device
  • Move read_register and write_register from hidpp10 to device class.
    • Add protocol of device class in hidpp10 and pass device object to Hidpp10.
  • hidpp20 feature_request can be moved into device class.

@MattHag MattHag force-pushed the refactor_hidpp branch 2 times, most recently from dea546e to 6c8661f Compare February 29, 2024 00:15
@pfps
Copy link
Collaborator

pfps commented Feb 29, 2024

There are errors:

idefix Solaar> bin/solaar -d
Exception in thread ReceiverListener:hidraw6:
Traceback (most recent call last):
  File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/listener.py", line 132, in run
    self.has_started()
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/listener.py", line 70, in has_started
    nfs = self.receiver.enable_connection_notifications()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/receiver.py", line 127, in enable_connection_notifications
    ok = hidpp10.set_notification_flags(self, set_flag_bits)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'logitech_receiver.hidpp10' has no attribute 'set_notification_flags'
Exception in thread ReceiverListener:hidraw7:
Traceback (most recent call last):
  File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/listener.py", line 136, in run
    self.receiver.status.changed(True, reason="initialization")
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 243, in changed
    self.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 323, in battery
    result = _hidpp20.get_battery(self, battery_feature)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1519, in get_battery
    result = battery_function(device)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1481, in get_battery_status
    return self.decipher_battery_status(report)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Hidpp20' object has no attribute 'decipher_battery_status'. Did you mean: 'get_battery_status'?
Traceback (most recent call last):
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 396, in _device_selected
    _update_info_panel(device, full=True)
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 803, in _update_info_panel
    _update_device_panel(device, _info._device, _info._buttons, full)
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 690, in _update_device_panel
    device.status.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 323, in battery
    result = _hidpp20.get_battery(self, battery_feature)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1519, in get_battery
    result = battery_function(device)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1481, in get_battery_status
    return self.decipher_battery_status(report)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Hidpp20' object has no attribute 'decipher_battery_status'. Did you mean: 'get_battery_status'?
Exception in thread ReceiverListener:hidraw10:
Traceback (most recent call last):
  File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/listener.py", line 136, in run
    self.receiver.status.changed(True, reason="initialization")
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 243, in changed
    self.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 323, in battery
    result = _hidpp20.get_battery(self, battery_feature)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1519, in get_battery
    result = battery_function(device)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1481, in get_battery_status
    return self.decipher_battery_status(report)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Hidpp20' object has no attribute 'decipher_battery_status'. Did you mean: 'get_battery_status'?

@pfps
Copy link
Collaborator

pfps commented Feb 29, 2024

Still errors:

idefix Solaar> bin/solaar -d
Exception in thread ReceiverListener:hidraw7:
Traceback (most recent call last):
  File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/listener.py", line 136, in run
    self.receiver.status.changed(True, reason="initialization")
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 243, in changed
    self.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 323, in battery
    result = _hidpp20.get_battery(self, battery_feature)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1519, in get_battery
    result = battery_function(device)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1481, in get_battery_status
    return self.decipher_battery_status(report)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Hidpp20' object has no attribute 'decipher_battery_status'. Did you mean: 'get_battery_status'?
Exception in thread ReceiverListener:hidraw10:
Traceback (most recent call last):
  File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/listener.py", line 136, in run
    self.receiver.status.changed(True, reason="initialization")
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 243, in changed
    self.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 323, in battery
    result = _hidpp20.get_battery(self, battery_feature)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1519, in get_battery
    result = battery_function(device)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/hidpp20.py", line 1481, in get_battery_status
    return self.decipher_battery_status(report)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Hidpp20' object has no attribute 'decipher_battery_status'. Did you mean: 'get_battery_status'?

@pfps
Copy link
Collaborator

pfps commented Feb 29, 2024

Still errors:

/home/local/SoftwareDownloads/Solaar 
idefix Solaar> pwd
/home/local/SoftwareDownloads/Solaar
idefix Solaar> bin/solaar 
Traceback (most recent call last):
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 396, in _device_selected
    _update_info_panel(device, full=True)
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 803, in _update_info_panel
    _update_device_panel(device, _info._device, _info._buttons, full)
  File "/home/local/SoftwareDownloads/Solaar/lib/solaar/ui/window.py", line 690, in _update_device_panel
    device.status.read_battery()
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/status.py", line 213, in read_battery
    battery = self._device.battery()
              ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/local/SoftwareDownloads/Solaar/lib/logitech_receiver/device.py", line 319, in battery
    return hidpp10.get_battery(self)
           ^^^^^^^^^^^^^^^^^^^
AttributeError: module 'logitech_receiver.hidpp10' has no attribute 'get_battery'

As there are only a few identifiers that remain in hidpp10, just search throughout the code base for users of hidpp10 that are not any of these. The same should be done for hidpp20.

@MattHag MattHag force-pushed the refactor_hidpp branch 2 times, most recently from 2f0f2bd to dda85be Compare February 29, 2024 23:47
@pfps
Copy link
Collaborator

pfps commented Mar 1, 2024

Git diff works badly here. It is the case that the vast majority of changes in hidpp10 and hidpp20 are just reindentation?

@MattHag
Copy link
Collaborator Author

MattHag commented Mar 1, 2024

Basically it is but I'd call it switch to Hidpp classes.

@pfps
Copy link
Collaborator

pfps commented Mar 1, 2024

Sorry, I meant "textual" changes.

@MattHag
Copy link
Collaborator Author

MattHag commented Mar 1, 2024

Yes, there's no change in behaviour and all the lines are as before.

@pfps pfps merged commit fb9dbb9 into pwr-Solaar:master Mar 2, 2024
5 checks passed
@MattHag MattHag deleted the refactor_hidpp branch June 2, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants