Bug#972898: hplip: no network scanner detection with 3.20.9

2020-10-27 Thread Ahzo
Oct 26, 2020, 14:58 by claremont...@gmail.com:

> Thank you for your report, Ahzo. I have forwarded it upstream. Please
> review what I have written and correct or add to it as you see fit.
>

Thanks for forwarding the report, as Lauchpad still doesn't work for me.
Your summary in the upstream report covers the main aspects of the problem.

Regards,
Ahzo



Bug#972898: hplip: no network scanner detection with 3.20.9

2020-10-26 Thread Brian Potkin
forwarded 972898 https://bugs.launchpad.net/hplip/+bug/1901556
thanks 



On Sun 25 Oct 2020 at 22:47:08 +0100, Ahzo wrote:

> Package: hplip
> Version: 3.20.9+dfsg0-4
> Severity: important
> Tags: patch

[...]

> PS: I tried to report this upstream, but my Launchpad login attempts
> always fail, because "something just went wrong in Launchpad".

Thank you for your report, Ahzo. I have forwarded it upstream. Please
review what I have written and correct or add to it as you see fit.

Regards,

Brian.



Bug#972898: hplip: no network scanner detection with 3.20.9

2020-10-25 Thread Ahzo
Package: hplip
Version: 3.20.9+dfsg0-4
Severity: important
Tags: patch

Dear Maintainer,

hplip version 3.20.9 replaced the custom mDNS implementation for scanner 
discovery (protocol/discovery/mdns.c) with using avahi 
(protocol/discovery/avahiDiscovery.c).
While this is a welcome change in principle, unfortunately the new code does 
not work.

The main problem is that it does not wait until all callbacks are called before 
stopping via avahi_simple_poll_quit.
Thus there is a 50/50 chance whether the '_scanner._tcp.local' or the 
'_uscan._tcp.local' mDNS reply gets processed (in the browse_callback) and it 
is practically impossible that any scanner gets processed (in the 
resolve_callback), because avahi_simple_poll_quit is called when the first mDNS 
reply has been processed.
It seems like this code was never really tested with an actual scanner on the 
network.

Attached is a patch fixing this by implementing a check_terminate function 
similar to the one used by avahi-browse.

The second problem is that the code expects the 'ty' part of the mDNS reply, 
which contains the device name, to start with 'HP'. However this is not always 
the case.
Previous versions of hplip would simply remove the first three letters of the 
scanner name and continue, which could be worked around by also removing these 
three letters in the models.dat. That issue has been reported two years ago 
upstream without response from HP. (see: 
https://bugs.launchpad.net/hplip/+bug/1797501)
The new code however  discards the scanner if its 'ty' name does not start with 
'HP', breaking that workaround.
Fortunately, since the new code now uses avahi, a proper fix is relatively 
simple: If the 'ty' part of the mDNS response does not start with 'HP', check 
whether the 'mfg' part does.

The second attached patch implements this fix.

While debugging this I also noticed that the log level for one message is 
wrong, causing spurious errors to be reported.

The third attached patch changes the log level for this message from BUG to DBG 
to fix this.

I hope HP solves this eventually, but until then please include these patches 
in the Debian package, so that scanner detection works again.

Thanks in advance,
Ahzo

PS: I tried to report this upstream, but my Launchpad login attempts always 
fail, because "something just went wrong in Launchpad".

>From 67dbad25f503e1d8c6794efba3f17718c77848ea Mon Sep 17 00:00:00 2001
From: Ahzo 
Date: Sun, 25 Oct 2020 22:17:04 +0100
Subject: [PATCH 1/3] avahiDiscovery: wait for all callbacks

When calling avahi_simple_poll_quit too early, not all callbacks will be called.
---
 protocol/discovery/avahiDiscovery.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/protocol/discovery/avahiDiscovery.c b/protocol/discovery/avahiDiscovery.c
index 8d325ffc0..395aec088 100644
--- a/protocol/discovery/avahiDiscovery.c
+++ b/protocol/discovery/avahiDiscovery.c
@@ -28,6 +28,7 @@ char ipAddressBuff[MAX_IP_ADDR_LEN]={'\0'};
 static int aBytesRead = 0;
 static AvahiSimplePoll *aSimplePoll = NULL;
 static int aMemAllocated = 0;
+static int n_all_for_now = 0, n_resolving = 0;
 
 /*
 This function will fill the dictionary arguments for the dbus function call
@@ -237,6 +238,15 @@ static bool getHPScannerModel(AvahiStringList *iStrList, const char *ikey,char *
 return aValueFound;  
 }
 
+static void check_terminate() {
+
+assert(n_all_for_now >= 0);
+assert(n_resolving >= 0);
+
+if (n_all_for_now <= 0 && n_resolving <= 0)
+avahi_simple_poll_quit(aSimplePoll);
+}
+
 /*
 This function will gets called whenever a service has been resolved successfully or timed out
 */
@@ -300,6 +310,9 @@ static void resolve_callback(
 }
 }
 //avahi_service_resolver_free(r);
+assert(n_resolving > 0);
+n_resolving--;
+check_terminate();
 }
 /* Called whenever a new services becomes available on the LAN or is removed from the LAN */
 static void browse_callback(
@@ -337,11 +350,14 @@ static void browse_callback(
 
 if (!(avahi_service_resolver_new(c, interface, protocol, name, type, domain, AVAHI_PROTO_INET, (AvahiLookupFlags)0, resolve_callback, c)))
 BUG( "Failed to resolve service '%s': %s\n", name, avahi_strerror(avahi_client_errno(c)));
+else
+n_resolving++;
 
 break;
 
 case AVAHI_BROWSER_ALL_FOR_NOW:
- avahi_simple_poll_quit(aSimplePoll);
+ n_all_for_now--;
+ check_terminate();
  break;
 }
 }
@@ -444,6 +460,7 @@ static void avahi_setup(const int iCommandType, const char* iHostName)
 goto fail;
 }
}
+   n_all_for_now++;
 
/* Create the service browser */
if (!(sb = avahi_service_browser_new(client, AVAHI_IF_UNSPEC, AVAHI_PROTO_INET, "_scanner._tcp", NULL, (AvahiLookupFlags)0, browse_callback, client))) 
@@ -451,6 +468,7 @@ static void avahi_setup(const int