From 6ae165c64dd3fddc2c761a5eaeed2d47f605d803 Mon Sep 17 00:00:00 2001 From: SinghCod3r Date: Wed, 5 Aug 2026 17:40:16 +0000 Subject: [PATCH] Delay displaying discovered devices until DNS-SD discovery completes Buffer discovered devices until DNS-SD resolution finishes so the UI is updated only once. This avoids the brief appearance and removal of legacy USB printers when an IPP-over-USB device is discovered. Signed-off-by: SinghCod3r --- cupshelpers/cupshelpers.py | 10 ++- dnssdresolve.py | 149 +++++++++++++++++++++++++++---- newprinter.py | 102 ++++++++++++++++++---- scp-dbus-service.py | 7 +- test_PhysicalDevice.py | 18 ++++ test_dnssdresolve.py | 174 +++++++++++++++++++++++++++++++++++++ 6 files changed, 420 insertions(+), 40 deletions(-) create mode 100644 test_dnssdresolve.py diff --git a/cupshelpers/cupshelpers.py b/cupshelpers/cupshelpers.py index 8fe7ead4a..a646856aa 100755 --- a/cupshelpers/cupshelpers.py +++ b/cupshelpers/cupshelpers.py @@ -21,6 +21,7 @@ import cups, pprint, os, tempfile, re, string import locale +import urllib.parse from . import _debugprint from . import config from functools import reduce @@ -535,9 +536,12 @@ def __init__(self, uri, **kw): self.id_dict = parseDeviceID (self.id) - s = uri.find("serial=") - if s != -1 and not self.id_dict.get ('SN',''): - self.id_dict['SN'] = uri[s + 7:] + if not self.id_dict.get ('SN', ''): + serial = urllib.parse.parse_qs ( + urllib.parse.urlsplit (uri).query, + keep_blank_values=True).get ('serial', []) + if len (serial) > 0: + self.id_dict['SN'] = serial[0] def __repr__ (self): return "" % self.uri diff --git a/dnssdresolve.py b/dnssdresolve.py index 7dd06da2f..4a14399bf 100644 --- a/dnssdresolve.py +++ b/dnssdresolve.py @@ -22,6 +22,128 @@ import urllib.parse from debug import * + +def _txt_value (txt, key): + prefix = key + "=" + for value in txt: + value = _txt_text (value) + if value.startswith (prefix): + return value[len (prefix):] + return '' + + +def _txt_text (value): + if isinstance (value, str): + return value + if isinstance (value, bytes): + return value.decode ("utf-8", "replace") + if isinstance (value, dbus.ByteArray): + return bytes (value).decode ("utf-8", "replace") + if isinstance (value, dbus.Array): + try: + return bytes (value).decode ("utf-8", "replace") + except (TypeError, ValueError): + return ''.join (_txt_text (item) for item in value) + return str (value) + + +def _service_tuple_from_uri (uri): + parsed = urllib.parse.urlparse (uri) + if parsed.scheme == 'dnssd': + hostname = parsed.netloc + elif parsed.scheme in ('ipp', 'ipps') and \ + parsed.netloc.find ("._ipp._tcp.local") != -1: + hostname = parsed.netloc + else: + return None + + elements = hostname.rsplit (".", 3) + if len (elements) != 4: + return None + + name, stype, protocol, domain = elements + name = urllib.parse.unquote (name) + stype += "." + protocol # e.g. _printer._tcp + return (name, stype, domain) + + +def needs_service_resolution (uri): + return _service_tuple_from_uri (uri) is not None + + +def _device_serial (device): + if hasattr (device, 'id_dict'): + return device.id_dict.get ('SN', '') + if hasattr (device, 'sn'): + return device.sn + return '' + + +def is_ipp_over_usb_device (device): + parsed = urllib.parse.urlparse (device.uri) + return (parsed.scheme in ('ipp', 'ipps') and + _service_tuple_from_uri (device.uri) is not None) + + +def _is_legacy_usb_device (device): + return (getattr (device, 'device_class', '') == 'direct' and + getattr (device, 'type', '') == 'usb') + + +def ipp_usb_serials (devices): + devices = list (devices) + serials = set () + for device in devices: + if is_ipp_over_usb_device (device): + serial = _device_serial (device) + if serial != '': + serials.add (serial) + return serials + + +class LegacyUSBDeviceCache: + """Track legacy USB device serials across sequential discovery batches.""" + + def __init__ (self): + self._usb_serials = set () + + def note_devices (self, devices): + for device in devices: + if _is_legacy_usb_device (device): + serial = _device_serial (device) + if serial != '': + self._usb_serials.add (serial) + + def suppress(self, devices): + devices = list(devices) + + ipp_serials = ipp_usb_serials(devices) + + if not ipp_serials: + return devices + + filtered = [] + for device in devices: + if (_is_legacy_usb_device(device) and + _device_serial(device) in ipp_serials): + continue + + filtered.append(device) + + return filtered + + def superseded_usb_serials (self, devices): + """Return cached USB serials superseded by IPP-over-USB in devices.""" + return self._usb_serials & ipp_usb_serials (devices) + + +def suppress_legacy_usb_devices (devices, cache=None): + devices = list (devices) + if cache is None: + cache = LegacyUSBDeviceCache () + cache.note_devices (devices) + return cache.suppress (devices) + class DNSSDHostNamesResolver: def __init__ (self, devices): self._devices = devices @@ -44,29 +166,19 @@ def resolve (self, reply_handler): return for uri, device in self._devices.items (): - if not uri.startswith ("dnssd://"): + service = _service_tuple_from_uri (uri) + if service is None: self._unresolved -= 1 continue - # We need to resolve the DNS-SD hostname in order to - # compare with other network devices. - result = urllib.parse.urlparse (uri) - hostname = result.netloc - elements = hostname.rsplit (".", 3) - if len (elements) != 4: - self._resolved () - continue - - name, stype, protocol, domain = elements - name = urllib.parse.unquote (name) - stype += "." + protocol # e.g. _printer._tcp + name, stype, domain = service try: obj = bus.get_object ("org.freedesktop.Avahi", "/") server = dbus.Interface (obj, "org.freedesktop.Avahi.Server") self._device_uri_by_name[(name, stype, domain)] = uri - debugprint ("Resolving address for %s" % hostname) + debugprint ("Resolving address for %s" % uri) server.ResolveService (-1, -1, name, stype, domain, -1, 0, @@ -88,13 +200,18 @@ def _resolved (self): def _reply (self, interface, protocol, name, stype, domain, host, aprotocol, address, port, txt, flags): uri = self._device_uri_by_name[(name, stype, domain)] - self._devices[uri].address = address + device = self._devices[uri] + device.address = address hostname = host p = hostname.find(".") if p != -1: hostname = hostname[:p] debugprint ("%s is at %s (%s)" % (uri, address, hostname)) - self._devices[uri].hostname = hostname + device.hostname = hostname + if hasattr (device, 'id_dict') and not device.id_dict.get ('SN', ''): + serial = _txt_value (txt, 'usb_SER') + if serial != '': + device.id_dict['SN'] = serial self._resolved () def _error (self, uri, error): diff --git a/newprinter.py b/newprinter.py index c94d94d0c..5d183643c 100644 --- a/newprinter.py +++ b/newprinter.py @@ -1901,6 +1901,9 @@ def setNPButtons(self): try: uri = self.getDeviceURI () valid = validDeviceURI (uri) + except AttributeError: + # No device selected yet. + valid = False except: nonfatalException () self.btnNPForward.set_sensitive(valid) @@ -2023,47 +2026,71 @@ def error_getting_devices (self, conn, exc): self.fetchDevices_conn._end_operation () self.fetchDevices_conn.destroy () self.fetchDevices_conn = None + # Display any devices buffered before the error, then clear. + if self._pending_devices is not None: + pending = self._pending_devices + self._pending_devices = None + self.add_devices (pending, None, no_more=True) def local_devices_reply (self, conn, result, current_uri): self.dec_spinner_task () - # Now we've got the local devices, start a request for the - # network devices. - self.fetchDevices (network=True, current_uri=current_uri) + # Buffer local devices rather than displaying them immediately. + # This prevents the UI from flickering when a legacy USB device + # is briefly shown and then later suppressed/replaced by its + # IPP-over-USB counterpart discovered during the network/DNS-SD phase. + self._pending_devices = result.copy() - # Add the local devices to the list. - self.add_devices (result, current_uri) + # Now start a request for the network devices. + self.fetchDevices (network=True, current_uri=current_uri) def network_devices_reply (self, conn, result, current_uri): self.fetchDevices_conn._end_operation () self.fetchDevices_conn.destroy () self.fetchDevices_conn = None - # Add the network devices to the list. - no_more = True + # Separate devices that need DNS-SD resolution. need_resolving = {} for uri, device in result.items (): - if uri.startswith ("dnssd://"): + if dnssdresolve.needs_service_resolution (uri): need_resolving[uri] = device - no_more = False for uri in need_resolving.keys (): del result[uri] - self.add_devices (result, current_uri, no_more=no_more) + # Merge non-DNS-SD network devices into the pending buffer. + if self._pending_devices is None: + self._pending_devices = result.copy() + else: + self._pending_devices.update (result) if len (need_resolving) > 0: + # DNS-SD resolution required — keep buffering. resolver = dnssdresolve.DNSSDHostNamesResolver (need_resolving) self.inc_spinner_task () resolver.resolve (reply_handler=lambda devices: self.dnssd_resolve_reply (current_uri, devices)) + else: + # No DNS-SD resolution needed — display the final device list. + pending = self._pending_devices + self._pending_devices = None + self.add_devices (pending, current_uri, no_more=True) self.dec_spinner_task () self.check_firewall () def dnssd_resolve_reply (self, current_uri, devices): - self.add_devices (devices, current_uri, no_more=True) + # Merge resolved DNS-SD devices into the pending buffer and + # display the complete device list exactly once. + if self._pending_devices is None: + self._pending_devices = {} + self._pending_devices.update (devices) + + pending = self._pending_devices + self._pending_devices = None + self.add_devices (pending, current_uri, no_more=True) + self.dec_spinner_task () self.check_firewall () @@ -2265,6 +2292,8 @@ def fillDeviceTab(self, current_uri=None): self.devices_find_nw_iter = find_nw_iter self.devices_network_iter = network_iter self.devices_network_fetched = False + self._legacy_usb_cache = dnssdresolve.LegacyUSBDeviceCache () + self._pending_devices = None self.tvNPDevices.set_model (model) self.entNPTDevice.set_text ('') self.expNPDeviceURIs.hide () @@ -2323,14 +2352,16 @@ def start_fetching_devices (self): self.fetchDevices_conn._begin_operation (_("fetching device list")) self.fetchDevices (network=False, current_uri=self.current_uri) del self.current_uri - def add_devices (self, devices, current_uri, no_more=False): + current_from_batch = False if current_uri: if current_uri in devices: current = devices.pop(current_uri) + current_from_batch = True elif current_uri.replace (":9100", "") in devices: current_uri = current_uri.replace (":9100", "") current = devices.pop(current_uri) + current_from_batch = True elif no_more: current = cupshelpers.Device (current_uri) current.info = "Current device" @@ -2338,6 +2369,11 @@ def add_devices (self, devices, current_uri, no_more=False): current_uri = None devices = list(devices.values()) + devices = dnssdresolve.suppress_legacy_usb_devices (devices, self._legacy_usb_cache) + if current_from_batch and current_uri and \ + not any (device.uri == current_uri for device in devices): + current_uri = None + current = None for device in devices: if device.type == "socket": @@ -2375,6 +2411,29 @@ def replace_generic (device): "hal", "beh", "smb", "scsi", "http", "bjnp", "delete")] + + ipp_serials = dnssdresolve.ipp_usb_serials (devices) + if ipp_serials: + to_remove = [] + for phys in self.devices: + devs = phys.get_devices () + if devs and any (getattr (d, 'type', '') == 'usb' for d in devs): + if phys.sn in ipp_serials: + debugprint ("Removing stale USB PhysicalDevice with SN %s" % phys.sn) + to_remove.append (phys) + + if to_remove: + model = self.tvNPDevices.get_model () + for phys in to_remove: + self.devices.remove (phys) + if model: + it = model.get_iter_first () + while it: + if model.get_value (it, 1) == phys: + model.remove (it) + break + it = model.iter_next (it) + newdevices = [] for device in devices: debugprint("Adding device with URI %s" % device.uri) @@ -2433,13 +2492,18 @@ def replace_generic (device): # An actual network printer device. Put this at the top. iter = model.insert_before (network_iter, find_nw_iter, row=row) - - # If this is the currently selected device we need - # to expand the "Network Printer" row so that it - # is visible. - if device == current_device: - network_path = model.get_path (network_iter) - self.tvNPDevices.expand_row (network_path, False) + if device == current_device or dnssdresolve.is_ipp_over_usb_device(devs[0]): + network_path = model.get_path(network_iter) + child_path = model.get_path(iter) + self.tvNPDevices.expand_row(network_path, False) + + def _select_ipp_device(tv, path): + tv.scroll_to_cell(path, None, True, 0.5, 0.0) + col = tv.get_column(0) + tv.set_cursor(path, col, False) + return False + GLib.idle_add(_select_ipp_device, + self.tvNPDevices, child_path) else: # Just a method of finding one. iter = model.append (network_iter, row=row) diff --git a/scp-dbus-service.py b/scp-dbus-service.py index 6d7889989..d1370de99 100644 --- a/scp-dbus-service.py +++ b/scp-dbus-service.py @@ -244,7 +244,7 @@ def __init__ (self, devices, reply_handler, error_handler): for device_uri, device_dict in self.devices.items (): deviceobj = cupshelpers.Device (device_uri, **device_dict) self.deviceobjs[device_uri] = deviceobj - if device_uri.startswith ("dnssd://"): + if dnssdresolve.needs_service_resolution (device_uri): need_resolving[device_uri] = deviceobj if len (need_resolving) > 0: @@ -263,8 +263,11 @@ def _group (self, resolved_devices=None): # We can ignore resolved_devices because the actual objects # (in self.devices) have been modified. try: + cache = dnssdresolve.LegacyUSBDeviceCache () + devices = dnssdresolve.suppress_legacy_usb_devices ( + self.deviceobjs.values (), cache) self.physdevs = [] - for device_uri, deviceobj in self.deviceobjs.items (): + for deviceobj in devices: newphysicaldevice = PhysicalDevice.PhysicalDevice (deviceobj) matched = False try: diff --git a/test_PhysicalDevice.py b/test_PhysicalDevice.py index bf96f6b22..fa5c8e56f 100644 --- a/test_PhysicalDevice.py +++ b/test_PhysicalDevice.py @@ -26,6 +26,14 @@ except ImportError: cups = None + +def _make_device(uri, device_class, serial, make_and_model='Xerox B235 MFP'): + return cupshelpers.Device( + uri, + **{'device-class': device_class, + 'device-make-and-model': make_and_model, + 'device-id': 'MFG:Xerox;MDL:B235 MFP;SN:%s;' % serial}) + @pytest.mark.skipif(cups is None, reason="cups module not available") def test_ordering(): # See https://bugzilla.redhat.com/show_bug.cgi?id=1154686 @@ -73,3 +81,13 @@ def test_ordering(): devices = phys.get_devices () assert devices[0] < devices[1] assert devices[0].uri.startswith ("hp") + + +@pytest.mark.skipif(cups is None, reason="cups module not available") +def test_usb_serial_drops_interface_suffix(): + device = cupshelpers.Device( + "usb://HP/Color%20LaserJet%20CP3525?serial=34004H030206H&interface=1", + **{'device-id':'MFG:Hewlett-Packard;CMD:PJL,MLC,BIDI-ECP,PJL,PCLXL,PCL,POSTSCRIPT,PDF;MDL:HP Color LaserJet CP3525;CLS:PRINTER;DES:Hewlett-Packard Color LaserJet CP3525;', + 'device-make-and-model':'HP Color LaserJet CP3525', + 'device-class':'direct'}) + assert device.id_dict['SN'] == '34004H030206H' diff --git a/test_dnssdresolve.py b/test_dnssdresolve.py new file mode 100644 index 000000000..899c275f3 --- /dev/null +++ b/test_dnssdresolve.py @@ -0,0 +1,174 @@ +import pytest + +pytest.importorskip("dbus") + +import dbus +import dnssdresolve + + +class DummyDevice: + def __init__(self): + self.id_dict = {'SN': ''} + + +def test_dns_sd_usb_ser_populates_sn(): + resolver = dnssdresolve.DNSSDHostNamesResolver.__new__(dnssdresolve.DNSSDHostNamesResolver) + device = DummyDevice() + resolver._devices = {'dnssd://printer': device} + resolver._device_uri_by_name = {('printer', '_ipp._tcp', 'local'): 'dnssd://printer'} + resolver._unresolved = 1 + resolver._reply_handler = lambda devices: None + + resolver._reply('iface', 'proto', 'printer', '_ipp._tcp', 'local', + 'printer.local', 0, '192.0.2.1', 631, + ['usb_SER=34004H030206H', 'pdl=application/pdf'], 0) + + assert device.id_dict['SN'] == '34004H030206H' + + +class DummyIPPUSBDevice: + def __init__(self, uri, device_class, serial): + self.uri = uri + self.device_class = device_class + self.type = uri.split(':', 1)[0] + self.id_dict = {'SN': serial} + + +def test_ipp_usb_helper_suppresses_legacy_usb_when_serial_matches(): + devices = [ + DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H'), + DummyIPPUSBDevice('ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + 'network', '34004H030206H'), + ] + + filtered = dnssdresolve.suppress_legacy_usb_devices(devices) + + assert [device.uri for device in filtered] == [ + 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/' + ] + + +def test_ipp_usb_helper_keeps_usb_when_no_matching_ipp_usb_exists(): + devices = [ + DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H'), + ] + + filtered = dnssdresolve.suppress_legacy_usb_devices(devices) + + assert [device.uri for device in filtered] == [ + 'usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1' + ] + + +def test_ipp_usb_helper_leaves_lan_ipp_unchanged(): + devices = [ + DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H'), + DummyIPPUSBDevice('ipp://printer.example.com/ipp/print', + 'network', '34004H030206H'), + ] + + filtered = dnssdresolve.suppress_legacy_usb_devices(devices) + + assert [device.uri for device in filtered] == [ + 'usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'ipp://printer.example.com/ipp/print', + ] + + +def test_ipp_over_usb_uri_is_resolved_but_lan_ipp_is_not(): + assert dnssdresolve.needs_service_resolution( + 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/') + assert not dnssdresolve.needs_service_resolution( + 'ipp://printer.example.com/ipp/print') + + +def test_ipp_over_usb_usb_ser_populates_sn(): + uri = 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/' + resolver = dnssdresolve.DNSSDHostNamesResolver.__new__(dnssdresolve.DNSSDHostNamesResolver) + device = DummyDevice() + resolver._devices = {uri: device} + service = dnssdresolve._service_tuple_from_uri(uri) + resolver._device_uri_by_name = {service: uri} + resolver._unresolved = 1 + resolver._reply_handler = lambda devices: None + + resolver._reply('iface', 'proto', 'Xerox(R) B235 MFP (USB)', '_ipp._tcp', 'local', + 'printer.local', 0, '192.0.2.1', 631, + ['usb_SER=34004H030206H', 'pdl=application/pdf'], 0) + + assert device.id_dict['SN'] == '34004H030206H' + +def test_ipp_usb_cache_suppresses_previously_discovered_usb_when_serial_matches(): + cache = dnssdresolve.LegacyUSBDeviceCache() + usb = DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H') + ipp_usb = DummyIPPUSBDevice('ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + 'network', '34004H030206H') + + visible = dnssdresolve.suppress_legacy_usb_devices([usb], cache) + superseded = cache.superseded_usb_serials([ipp_usb]) + assert superseded == {'34004H030206H'} + visible = [d for d in visible + if not (d.device_class == 'direct' and d.type == 'usb' and d.id_dict.get('SN', '') in superseded)] + visible.extend(dnssdresolve.suppress_legacy_usb_devices([ipp_usb], cache)) + + assert [device.uri for device in visible] == [ + 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/' + ] + + +def test_ipp_usb_cache_keeps_both_devices_when_serials_differ(): + cache = dnssdresolve.LegacyUSBDeviceCache() + usb = DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H') + ipp_usb = DummyIPPUSBDevice('ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + 'network', 'DIFFERENT') + + visible = dnssdresolve.suppress_legacy_usb_devices([usb], cache) + superseded = cache.superseded_usb_serials([ipp_usb]) + assert superseded == set() + visible.extend(dnssdresolve.suppress_legacy_usb_devices([ipp_usb], cache)) + + assert [device.uri for device in visible] == [ + 'usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + ] + + +def test_ipp_usb_cache_does_not_affect_lan_ipp(): + cache = dnssdresolve.LegacyUSBDeviceCache() + usb = DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'direct', '34004H030206H') + ipp_lan = DummyIPPUSBDevice('ipp://printer.example.com/ipp/print', + 'network', '34004H030206H') + + visible = dnssdresolve.suppress_legacy_usb_devices([usb], cache) + superseded = cache.superseded_usb_serials([ipp_lan]) + assert superseded == set() + visible.extend(dnssdresolve.suppress_legacy_usb_devices([ipp_lan], cache)) + + assert [device.uri for device in visible] == [ + 'usb://Xerox/B235%20MFP?serial=34004H030206H&interface=1', + 'ipp://printer.example.com/ipp/print', + ] + + +def test_ipp_usb_cache_does_not_suppress_usb_without_serial(): + cache = dnssdresolve.LegacyUSBDeviceCache() + usb = DummyIPPUSBDevice('usb://Xerox/B235%20MFP?serial=&interface=1', + 'direct', '') + ipp_usb = DummyIPPUSBDevice('ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + 'network', '34004H030206H') + + visible = dnssdresolve.suppress_legacy_usb_devices([usb], cache) + superseded = cache.superseded_usb_serials([ipp_usb]) + assert superseded == set() + visible.extend(dnssdresolve.suppress_legacy_usb_devices([ipp_usb], cache)) + + assert [device.uri for device in visible] == [ + 'usb://Xerox/B235%20MFP?serial=&interface=1', + 'ipp://Xerox(R)%20B235%20MFP%20(USB)._ipp._tcp.local/', + ]