From 4ab976b7c2b334556540659cb935f0a15d89df11 Mon Sep 17 00:00:00 2001 From: MattHag <16444067+MattHag@users.noreply.github.com> Date: Thu, 2 Jan 2025 14:16:25 +0100 Subject: [PATCH 1/3] solaar/cli: Remove outdated logger enabled checks Logger enabled checks clutter the code unnecessarily. The checks are now handled in a custom logger class. Eventually they can be completely removed in the future. Related #2664 --- lib/solaar/cli/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/solaar/cli/__init__.py b/lib/solaar/cli/__init__.py index 2f846955b2..5a4f83e860 100644 --- a/lib/solaar/cli/__init__.py +++ b/lib/solaar/cli/__init__.py @@ -116,8 +116,7 @@ def _receivers(dev_path=None): continue try: r = receiver.create_receiver(base, dev_info) - if logger.isEnabledFor(logging.DEBUG): - logger.debug("[%s] => %s", dev_info.path, r) + logger.debug("[%s] => %s", dev_info.path, r) if r: yield r except Exception as e: @@ -135,8 +134,7 @@ def _receivers_and_devices(dev_path=None): else: d = receiver.create_receiver(base, dev_info) - if logger.isEnabledFor(logging.DEBUG): - logger.debug("[%s] => %s", dev_info.path, d) + logger.debug("[%s] => %s", dev_info.path, d) if d is not None: yield d except Exception as e: From 82bd833cc6244069da2f5aef4a5dd808696c11d8 Mon Sep 17 00:00:00 2001 From: MattHag <16444067+MattHag@users.noreply.github.com> Date: Thu, 2 Jan 2025 14:18:11 +0100 Subject: [PATCH 2/3] solaar/ui: Remove outdated logger enabled checks Logger enabled checks clutter the code unnecessarily. The checks are now handled in a custom logger class. Eventually they can be completely removed in the future. Related #2664 --- lib/solaar/ui/__init__.py | 15 +++++---------- lib/solaar/ui/config_panel.py | 11 ++++------- lib/solaar/ui/desktop_notifications.py | 6 ++---- lib/solaar/ui/icons.py | 6 ++---- lib/solaar/ui/pair_window.py | 15 +++++---------- lib/solaar/ui/tray.py | 12 ++++-------- lib/solaar/ui/window.py | 6 ++---- 7 files changed, 24 insertions(+), 47 deletions(-) diff --git a/lib/solaar/ui/__init__.py b/lib/solaar/ui/__init__.py index 8b18416673..c97104ccb0 100644 --- a/lib/solaar/ui/__init__.py +++ b/lib/solaar/ui/__init__.py @@ -56,8 +56,7 @@ class GtkSignal(Enum): def _startup(app, startup_hook, use_tray, show_window): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("startup registered=%s, remote=%s", app.get_is_registered(), app.get_is_remote()) + logger.debug("startup registered=%s, remote=%s", app.get_is_registered(), app.get_is_remote()) common.start_async() desktop_notifications.init() if use_tray: @@ -67,8 +66,7 @@ def _startup(app, startup_hook, use_tray, show_window): def _activate(app): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("activate") + logger.debug("activate") if app.get_windows(): window.popup() else: @@ -81,8 +79,7 @@ def _command_line(app, command_line): if not args: _activate(app) elif args[0] == "config": # config call from remote instance - if logger.isEnabledFor(logging.INFO): - logger.info("remote command line %s", args) + logger.info("remote command line %s", args) dev = find_device(args[1]) if dev: setting = next((s for s in dev.settings if s.name == args[2]), None) @@ -92,8 +89,7 @@ def _command_line(app, command_line): def _shutdown(_app, shutdown_hook): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("shutdown") + logger.debug("shutdown") shutdown_hook() common.stop_async() tray.destroy() @@ -127,8 +123,7 @@ def run_loop( def _status_changed(device, alert, reason, refresh=False): assert device is not None - if logger.isEnabledFor(logging.DEBUG): - logger.debug("status changed: %s (%s) %s", device, alert, reason) + logger.debug("status changed: %s (%s) %s", device, alert, reason) if alert is None: alert = Alert.NONE diff --git a/lib/solaar/ui/config_panel.py b/lib/solaar/ui/config_panel.py index b52ae10cff..3f8397e540 100644 --- a/lib/solaar/ui/config_panel.py +++ b/lib/solaar/ui/config_panel.py @@ -55,8 +55,7 @@ def _do_read(s, force, sb, online, sensitive): v = s.read(not force) except Exception as e: v = None - if logger.isEnabledFor(logging.WARNING): - logger.warning("%s: error reading so use None (%s): %s", s.name, s._device, repr(e)) + logger.warning("%s: error reading so use None (%s): %s", s.name, s._device, repr(e)) GLib.idle_add(_update_setting_item, sb, v, online, sensitive, True, priority=99) ui_async(_do_read, setting, force_read, sbox, device_is_online, sensitive) @@ -694,8 +693,7 @@ def _create_sbox(s, _device): elif s.kind == settings.Kind.HETERO: control = HeteroKeyControl(sbox, change) else: - if logger.isEnabledFor(logging.WARNING): - logger.warning("setting %s display not implemented", s.label) + logger.warning("setting %s display not implemented", s.label) return None control.set_sensitive(False) # the first read will enable it @@ -821,10 +819,9 @@ def record_setting(device, setting, values): def _record_setting(device, setting_class, values): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("on %s changing setting %s to %s", device, setting_class.name, values) + logger.debug("on %s changing setting %s to %s", device, setting_class.name, values) setting = next((s for s in device.settings if s.name == setting_class.name), None) - if setting is None and logger.isEnabledFor(logging.DEBUG): + if setting is None: logger.debug( "No setting for %s found on %s when trying to record a change made elsewhere", setting_class.name, diff --git a/lib/solaar/ui/desktop_notifications.py b/lib/solaar/ui/desktop_notifications.py index 67864d78ff..dcc90d0afb 100644 --- a/lib/solaar/ui/desktop_notifications.py +++ b/lib/solaar/ui/desktop_notifications.py @@ -58,8 +58,7 @@ def init(): global available if available: if not Notify.is_initted(): - if logger.isEnabledFor(logging.INFO): - logger.info("starting desktop notifications") + logger.info("starting desktop notifications") try: return Notify.init(NAME.lower()) except Exception: @@ -70,8 +69,7 @@ def init(): def uninit(): """Stop desktop notifications.""" if available and Notify.is_initted(): - if logger.isEnabledFor(logging.INFO): - logger.info("stopping desktop notifications") + logger.info("stopping desktop notifications") _notifications.clear() Notify.uninit() diff --git a/lib/solaar/ui/icons.py b/lib/solaar/ui/icons.py index 1900fd69fd..3497317cc0 100644 --- a/lib/solaar/ui/icons.py +++ b/lib/solaar/ui/icons.py @@ -37,8 +37,7 @@ def _init_icon_paths(): return _default_theme = Gtk.IconTheme.get_default() - if logger.isEnabledFor(logging.DEBUG): - logger.debug("icon theme paths: %s", _default_theme.get_search_path()) + logger.debug("icon theme paths: %s", _default_theme.get_search_path()) if gtk.battery_icons_style == "symbolic": global TRAY_OKAY @@ -57,8 +56,7 @@ def battery(level=None, charging=False): if not _default_theme.has_icon(icon_name): logger.warning("icon %s not found in current theme", icon_name) return TRAY_OKAY # use Solaar icon if battery icon not available - elif logger.isEnabledFor(logging.DEBUG): - logger.debug("battery icon for %s:%s = %s", level, charging, icon_name) + logger.debug("battery icon for %s:%s = %s", level, charging, icon_name) return icon_name diff --git a/lib/solaar/ui/pair_window.py b/lib/solaar/ui/pair_window.py index 9e9eb27316..c89d747043 100644 --- a/lib/solaar/ui/pair_window.py +++ b/lib/solaar/ui/pair_window.py @@ -99,8 +99,7 @@ def prepare(receiver): def check_lock_state(assistant, receiver, count=2): if not assistant.is_drawable(): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("assistant %s destroyed, bailing out", assistant) + logger.debug("assistant %s destroyed, bailing out", assistant) return False return _check_lock_state(assistant, receiver, count) @@ -136,21 +135,18 @@ def _check_lock_state(assistant, receiver, count): def _pairing_failed(assistant, receiver, error): assistant.remove_page(0) # needed to reset the window size - if logger.isEnabledFor(logging.DEBUG): - logger.debug("%s fail: %s", receiver, error) + logger.debug("%s fail: %s", receiver, error) _create_failure_page(assistant, error) def _pairing_succeeded(assistant, receiver, device): assistant.remove_page(0) # needed to reset the window size - if logger.isEnabledFor(logging.DEBUG): - logger.debug("%s success: %s", receiver, device) + logger.debug("%s success: %s", receiver, device) _create_success_page(assistant, device) def _finish(assistant, receiver): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("finish %s", assistant) + logger.debug("finish %s", assistant) assistant.destroy() receiver.pairing.new_device = None if receiver.pairing.lock_open: @@ -165,8 +161,7 @@ def _finish(assistant, receiver): def _show_passcode(assistant, receiver, passkey): - if logger.isEnabledFor(logging.DEBUG): - logger.debug("%s show passkey: %s", receiver, passkey) + logger.debug("%s show passkey: %s", receiver, passkey) name = receiver.pairing.device_name authentication = receiver.pairing.device_authentication intro_text = _("%(receiver_name)s: pair new device") % {"receiver_name": receiver.name} diff --git a/lib/solaar/ui/tray.py b/lib/solaar/ui/tray.py index ef08a94c96..d0c7786748 100644 --- a/lib/solaar/ui/tray.py +++ b/lib/solaar/ui/tray.py @@ -132,8 +132,7 @@ def _scroll(tray_icon, event, direction=None): _picked_device = None _picked_device = candidate or _picked_device - if logger.isEnabledFor(logging.DEBUG): - logger.debug("scroll: picked %s", _picked_device) + logger.debug("scroll: picked %s", _picked_device) _update_tray_icon() @@ -153,8 +152,7 @@ def _scroll(tray_icon, event, direction=None): # treat unavailable versions the same as unavailable packages raise ImportError from exc - if logger.isEnabledFor(logging.DEBUG): - logger.debug(f"using {'Ayatana ' if ayatana_appindicator_found else ''}AppIndicator3") + logger.debug(f"using {'Ayatana ' if ayatana_appindicator_found else ''}AppIndicator3") # Defense against AppIndicator3 bug that treats files in current directory as icon files # https://bugs.launchpad.net/ubuntu/+source/libappindicator/+bug/1363277 @@ -212,8 +210,7 @@ def attention(reason=None): GLib.timeout_add(10 * 1000, _icon.set_status, AppIndicator3.IndicatorStatus.ACTIVE) except ImportError: - if logger.isEnabledFor(logging.DEBUG): - logger.debug("using StatusIcon") + logger.debug("using StatusIcon") def _create(menu): icon = Gtk.StatusIcon.new_from_icon_name(icons.TRAY_INIT) @@ -317,8 +314,7 @@ def _pick_device_with_lowest_battery(): picked = info picked_level = level or 0 - if logger.isEnabledFor(logging.DEBUG): - logger.debug("picked device with lowest battery: %s", picked) + logger.debug("picked device with lowest battery: %s", picked) return picked diff --git a/lib/solaar/ui/window.py b/lib/solaar/ui/window.py index e26cb7af00..62d2a47996 100644 --- a/lib/solaar/ui/window.py +++ b/lib/solaar/ui/window.py @@ -411,8 +411,7 @@ def _receiver_row(receiver_path, receiver=None): status_icon = None row_data = (receiver_path, 0, True, receiver.name, icon_name, status_text, status_icon, receiver) assert len(row_data) == len(_TREE_SEPATATOR) - if logger.isEnabledFor(logging.DEBUG): - logger.debug("new receiver row %s", row_data) + logger.debug("new receiver row %s", row_data) item = _model.append(None, row_data) if _TREE_SEPATATOR: _model.append(None, _TREE_SEPATATOR) @@ -465,8 +464,7 @@ def _device_row(receiver_path, device_number, device=None): device, ) assert len(row_data) == len(_TREE_SEPATATOR) - if logger.isEnabledFor(logging.DEBUG): - logger.debug("new device row %s at index %d", row_data, new_child_index) + logger.debug("new device row %s at index %d", row_data, new_child_index) item = _model.insert(receiver_row, new_child_index, row_data) return item or None From f14c0cd87124b914774093de6b9d760f2d4477b1 Mon Sep 17 00:00:00 2001 From: MattHag <16444067+MattHag@users.noreply.github.com> Date: Thu, 2 Jan 2025 14:20:19 +0100 Subject: [PATCH 3/3] solaar: Remove outdated logger enabled checks Logger enabled checks clutter the code unnecessarily. The checks are now handled in a custom logger class. Eventually they can be completely removed in the future. Related #2664 --- lib/solaar/configuration.py | 27 +++++++--------- lib/solaar/dbus.py | 3 +- lib/solaar/gtk.py | 5 ++- lib/solaar/listener.py | 62 +++++++++++++------------------------ lib/solaar/tasks.py | 6 ++-- 5 files changed, 38 insertions(+), 65 deletions(-) diff --git a/lib/solaar/configuration.py b/lib/solaar/configuration.py index c0c060c8e3..e0b4bb4f08 100644 --- a/lib/solaar/configuration.py +++ b/lib/solaar/configuration.py @@ -62,8 +62,7 @@ def _load(): loaded_config = _convert_json(loaded_config) else: path = None - if logger.isEnabledFor(logging.DEBUG): - logger.debug("load => %s", loaded_config) + logger.debug("load => %s", loaded_config) global _config _config = _parse_config(loaded_config, path) @@ -78,14 +77,13 @@ def _parse_config(loaded_config, config_path): loaded_version = loaded_config[0] discard_derived_properties = loaded_version != current_version if discard_derived_properties: - if logger.isEnabledFor(logging.INFO): - logger.info( - "config file '%s' was generated by another version of solaar " - "(config: %s, current: %s). refreshing detected device capabilities", - config_path, - loaded_version, - current_version, - ) + logger.info( + "config file '%s' was generated by another version of solaar " + "(config: %s, current: %s). refreshing detected device capabilities", + config_path, + loaded_version, + current_version, + ) for device in loaded_config[1:]: assert isinstance(device, dict) @@ -154,8 +152,7 @@ def do_save(): try: with open(_yaml_file_path, "w") as config_file: yaml.dump(_config, config_file, default_flow_style=None, width=150) - if logger.isEnabledFor(logging.INFO): - logger.info("saved %s to %s", _config, _yaml_file_path) + logger.info("saved %s to %s", _config, _yaml_file_path) except Exception as e: logger.error("failed to save to %s: %s", _yaml_file_path, e) @@ -251,11 +248,9 @@ def match(wpid, serial, modelId, unitId, c): break if not entry: if not device.online: # don't create entry for offline devices - if logger.isEnabledFor(logging.INFO): - logger.info("not setting up persister for offline device %s", device._name) + logger.info("not setting up persister for offline device %s", device._name) return - if logger.isEnabledFor(logging.INFO): - logger.info("setting up persister for device %s", device.name) + logger.info("setting up persister for device %s", device.name) entry = _DeviceEntry() _config.append(entry) entry.update(device.name, device.wpid, device.serial, modelId, unitId) diff --git a/lib/solaar/dbus.py b/lib/solaar/dbus.py index ff4e58a695..142b5904f5 100644 --- a/lib/solaar/dbus.py +++ b/lib/solaar/dbus.py @@ -68,8 +68,7 @@ def watch_suspend_resume( dbus_interface=_LOGIND_INTERFACE, path=_LOGIND_PATH, ) - if logger.isEnabledFor(logging.INFO): - logger.info("connected to system dbus, watching for suspend/resume events") + logger.info("connected to system dbus, watching for suspend/resume events") _BLUETOOTH_PATH_PREFIX = "/org/bluez/hci0/dev_" diff --git a/lib/solaar/gtk.py b/lib/solaar/gtk.py index 9d9aea6230..0361e96c9b 100755 --- a/lib/solaar/gtk.py +++ b/lib/solaar/gtk.py @@ -134,9 +134,8 @@ def _parse_arguments(): logging.getLogger("").addHandler(stream_handler) if not args.action: - if logger.isEnabledFor(logging.INFO): - language, encoding = locale.getlocale() - logger.info("version %s, language %s (%s)", __version__, language, encoding) + language, encoding = locale.getlocale() + logger.info("version %s, language %s (%s)", __version__, language, encoding) return args diff --git a/lib/solaar/listener.py b/lib/solaar/listener.py index 3846510146..f46a9ffa73 100644 --- a/lib/solaar/listener.py +++ b/lib/solaar/listener.py @@ -79,15 +79,13 @@ def __init__(self, receiver, status_changed_callback): receiver.status_callback = self._status_changed def has_started(self): - if logger.isEnabledFor(logging.INFO): - logger.info("%s: notifications listener has started (%s)", self.receiver, self.receiver.handle) + 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.NotificationFlag.WIRELESS.value): - logger.warning( - "Receiver on %s might not support connection notifications, GUI might not show its devices", - self.receiver.path, - ) + 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, + ) self.receiver.notification_flags = nfs self.receiver.notify_devices() self._status_changed(self.receiver) @@ -95,8 +93,7 @@ def has_started(self): def has_stopped(self): r, self.receiver = self.receiver, None assert r is not None - if logger.isEnabledFor(logging.INFO): - logger.info("%s: notifications listener has stopped", r) + logger.info("%s: notifications listener has stopped", r) # because udev is not notifying us about device removal, make sure to clean up in _all_listeners _all_listeners.pop(r.path, None) @@ -144,8 +141,7 @@ def _status_changed(self, device, alert=None, reason=None): if not device: # Device was unpaired, and isn't valid anymore. # We replace it with a ghost so that the UI has something to work with while cleaning up. - if logger.isEnabledFor(logging.INFO): - logger.info("device %s was unpaired, ghosting", device) + logger.info("device %s was unpaired, ghosting", device) device = _ghost(device) self.status_changed_callback(device, alert, reason) @@ -163,20 +159,17 @@ def _notifications_handler(self, n): # a notification that came in to the device listener - strange, but nothing needs to be done here if self.receiver.isDevice: - if logger.isEnabledFor(logging.DEBUG): - logger.debug("Notification %s via device %s being ignored.", n, self.receiver) + logger.debug("Notification %s via device %s being ignored.", n, self.receiver) return # DJ pairing notification - ignore - hid++ 1.0 pairing notification is all that is needed if n.sub_id == 0x41 and n.report_id == base.DJ_MESSAGE_ID: - if logger.isEnabledFor(logging.INFO): - logger.info("ignoring DJ pairing notification %s", n) + logger.info("ignoring DJ pairing notification %s", n) return # a device notification if not (0 < n.devnumber <= 16): # some receivers have devices past their max # devices - if logger.isEnabledFor(logging.WARNING): - logger.warning("Unexpected device number (%s) in notification %s.", n.devnumber, n) + logger.warning("Unexpected device number (%s) in notification %s.", n.devnumber, n) return already_known = n.devnumber in self.receiver @@ -221,8 +214,7 @@ def _notifications_handler(self, n): # Apply settings every time the device connects if n.sub_id == 0x41: - if logger.isEnabledFor(logging.INFO): - logger.info("connection %s for device wpid %s kind %s serial %s", n, dev.wpid, dev.kind, dev._serial) + logger.info("connection %s for device wpid %s kind %s serial %s", n, dev.wpid, dev.kind, dev._serial) # If there are saved configs, bring the device's settings up-to-date. # They will be applied when the device is marked as online. configuration.attach_to(dev) @@ -234,10 +226,8 @@ def _notifications_handler(self, n): if self.receiver.pairing.lock_open and not already_known: # this should be the first notification after a device was paired - if logger.isEnabledFor(logging.WARNING): - logger.warning("first notification was not a connection notification") - if logger.isEnabledFor(logging.INFO): - logger.info("%s: pairing detected new device", self.receiver) + logger.warning("first notification was not a connection notification") + logger.info("%s: pairing detected new device", self.receiver) self.receiver.pairing.new_device = dev elif dev.online is None: dev.ping() @@ -253,19 +243,16 @@ def _process_bluez_dbus(device: Device, path, dictionary: dict, signature): if device: if dictionary.get("Connected") is not None: connected = dictionary.get("Connected") - if logger.isEnabledFor(logging.INFO): - logger.info("bluez dbus for %s: %s", device, "CONNECTED" if connected else "DISCONNECTED") + logger.info("bluez dbus for %s: %s", device, "CONNECTED" if connected else "DISCONNECTED") device.changed(connected, reason=i18n._("connected") if connected else i18n._("disconnected")) elif device is not None: - if logger.isEnabledFor(logging.INFO): - logger.info("bluez cleanup for %s", device) + logger.info("bluez cleanup for %s", device) _cleanup_bluez_dbus(device) def _cleanup_bluez_dbus(device: Device): """Remove dbus signal receiver for device""" - if logger.isEnabledFor(logging.INFO): - logger.info("bluez cleanup for %s", device) + logger.info("bluez cleanup for %s", device) dbus.watch_bluez_connect(device.hid_serial, None) @@ -296,8 +283,7 @@ def _start(device_info: DeviceInfo): def start_all(): stop_all() # just in case this it called twice in a row... - if logger.isEnabledFor(logging.INFO): - logger.info("starting receiver listening threads") + logger.info("starting receiver listening threads") for device_info in base.receivers_and_devices(): _process_receiver_event(ACTION_ADD, device_info) @@ -306,8 +292,7 @@ def stop_all(): listeners = list(_all_listeners.values()) _all_listeners.clear() if listeners: - if logger.isEnabledFor(logging.INFO): - logger.info("stopping receiver listening threads %s", listeners) + logger.info("stopping receiver listening threads %s", listeners) for listener_thread in listeners: listener_thread.stop() configuration.save() @@ -319,8 +304,7 @@ def stop_all(): # after a resume, the device may have been off so mark its saved status to ensure # that the status is pushed to the device when it comes back def ping_all(resuming=False): - if logger.isEnabledFor(logging.INFO): - logger.info("ping all devices%s", " when resuming" if resuming else "") + logger.info("ping all devices%s", " when resuming" if resuming else "") for listener_thread in _all_listeners.values(): if listener_thread.receiver.isDevice: if resuming: @@ -363,8 +347,7 @@ def _process_add(device_info: DeviceInfo, retry): if e.errno == errno.EACCES: try: output = subprocess.check_output(["/usr/bin/getfacl", "-p", device_info.path], text=True) - if logger.isEnabledFor(logging.WARNING): - logger.warning("Missing permissions on %s\n%s.", device_info.path, output) + logger.warning("Missing permissions on %s\n%s.", device_info.path, output) except Exception: pass if retry: @@ -382,8 +365,7 @@ def _process_receiver_event(action, device_info): assert action is not None assert device_info is not None assert _error_callback - if logger.isEnabledFor(logging.INFO): - logger.info("receiver event %s %s", action, device_info) + logger.info("receiver event %s %s", action, device_info) # whatever the action, stop any previous receivers at this path listener_thread = _all_listeners.pop(device_info.path, None) if listener_thread is not None: diff --git a/lib/solaar/tasks.py b/lib/solaar/tasks.py index 195cfa5ab6..0b13685a4a 100644 --- a/lib/solaar/tasks.py +++ b/lib/solaar/tasks.py @@ -46,8 +46,7 @@ def stop(self): def run(self): self.alive = True - if logger.isEnabledFor(logging.DEBUG): - logger.debug("started") + logger.debug("started") while self.alive: task = self.queue.get() @@ -59,5 +58,4 @@ def run(self): except Exception: logger.exception("calling %s", function) - if logger.isEnabledFor(logging.DEBUG): - logger.debug("stopped") + logger.debug("stopped")