Bug 443861

Summary: libusb-0_1-4 incompatible to libusb (breaks USB support in gpsbabel)
Product: [openSUSE] openSUSE 11.2 Reporter: Stefan Dirsch <sndirsch>
Component: OtherAssignee: Stanislav Brabec <sbrabec>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: meissner, nadvornik, opensuse, sbrabec
Version: Factory   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: working
broken
broken
gpsbabel-modified.log.gz
libusb-compat-interrupt-in-ep.patch
gpsbabel-interrupt-in-ep.patch

Description Stefan Dirsch 2008-11-11 19:06:49 UTC
# sudo gpsbabel -t -i garmin -f usb: -o gpx -F /tmp/out.gpx
Could not start session in a reasonable number of tries.

After replacing libusb-0_1-4 (Factory) with libusb (11.0) gpsbabel works again.
Looks like the compatibility layer of libusb-0_1-4 (which makes use of libusb-1_0-0) does not work reliably. :-(
Comment 2 Stanislav Brabec 2008-11-12 13:05:38 UTC
gpsbabel -i mtk,erase=0 -f /dev/ttyUSB0 -o mtkbin -F log.bin
works for me.

=> It may be a problem of libusb usage

As you have beta4, I guess that you have latest version of libusb-compat:

rpm -q --changelog libusb-0_1-4 | grep -A4 meissner@suse.de
* Wed Oct 08 2008 meissner@suse.de
- handle case where usb_init() failed (ctx NULL), so
  we do not crash. bnc#432129
- added more NULL ptr checks, where older libusb were
  more graceful and did not crash. bnc#432129
Comment 3 Stefan Dirsch 2008-11-12 13:58:20 UTC
(In reply to comment #2 from Stanislav Brabec)
> gpsbabel -i mtk,erase=0 -f /dev/ttyUSB0 -o mtkbin -F log.bin
> works for me.

According to 

  http://www.gpsbabel.org/os/Linux_Hotplug.html 

using the garmin kernel module is not recommended. This is also what Robert Lipe <robertlipe@gpsbabel.org> recently stated on the gpsbabel-misc mailing list.

Date: Tue, 11 Nov 2008 10:57:10 -0600
From: Robert Lipe <robertlipe@gpsbabel.org>
To: Mishustin Alexey <shumkar@shumkar.ru>
Cc: gpsbabel-misc@lists.sourceforge.net
Subject: Re: [Gpsbabel-misc] How to upload tracks with all trackpoints

[...]
"Remove the kernel driver (which works badly for a large number of people) and use "usb:" instead of "/dev/ttyUSB0"."
[...]

> => It may be a problem of libusb usage

I don't think people will care about such an argument, when they suddently notice that their favorite applications no longer work. I simply don't understand why we introduced this compatibility layer, when we need to ship both library versions anyway. I see only regressions and no benefits at all here. I also didn't see any FATE request or bugreport to introduce such a compatibility layer instead of keeping the old implementation as it is (working). Also libusb 1.0 is still considered BETA.

> As you have beta4, I guess that you have latest version of libusb-compat:
> 
> rpm -q --changelog libusb-0_1-4 | grep -A4 meissner@suse.de
> * Wed Oct 08 2008 meissner@suse.de
> - handle case where usb_init() failed (ctx NULL), so
>   we do not crash. bnc#432129
> - added more NULL ptr checks, where older libusb were
>   more graceful and did not crash. bnc#432129

Sure I have including this change

* Properly obsolete old implementation of the library (bnc#437768).

I even builded every older versions of this package and rebuilded gpsbabel 
against it. Same problem.

Comment 4 Stanislav Brabec 2008-11-12 14:31:42 UTC
We introduced libusb-1_0 for better support of smart cards. Even worse, smart card support need libusb-1_0 being used via libusb-compat.

I see following work-arount, safe, easy to do, open for experimenting:

Change libusb-compat soname to libusb-0.1.so.5 and provide two conflicting development files for libusb0.

Packages build against libusb-0_1 will use libusb-0.1.so.4, packages build against libusb-1_0 will use libusb-0.1.so.5.


I am just working on it.

But it would still be nice to track all regressions.
Comment 5 Stanislav Brabec 2008-11-12 14:40:06 UTC
>> => It may be a problem of libusb usage
>
>I don't think people will care about such an argument, when they suddently
>notice that their favorite applications no longer work

Well, there are slight differences between behavior of libusb-0_1-4 and libusb-compat. In some situations, where libusb-0_1-4 behavior is "possibly crash", the behavior of libusb-compat is EINVAL (e. g. bug 432129 - double usb_close).

"Possibly crash" may mean "it actually works" and EINVAL means "it stopped to work" and then user could see a regression.
Comment 6 Stefan Dirsch 2008-11-12 14:45:10 UTC
Thanks a lot. The proposal sounds reasonable to me. JFYI

  sudo gpsbabel -i garmin -f /dev/ttyUSB0 -o gpx -F /tmp/out.gpx

hangs and gpsbabel cannot be killed even not with "-9", which confirms that
isn't really a good idea to use this kernel module.
Comment 7 Stefan Dirsch 2008-11-12 14:49:10 UTC
Thanks for explanation, Stanislav. I now understand why we introduced this compat library.
Comment 8 Stefan Dirsch 2008-11-12 16:51:02 UTC
Added dstoecker since he's maintaining gpsbabel in our buildservice.
Comment 9 Stanislav Brabec 2008-11-12 17:01:26 UTC
libusb-compat seems to be bug incompatible with libusb0.

Reverting to libusb0.

Applications can still be built against libusb-1.0 using libusb-compat-devel instead of libusb-devel. Please report failures of these packages to upstream: libusb-devel@lists.sourceforge.net

For 11.1, only packages explicitly testing libusb-1.0 support will use libusb-1.0. After 11.1, working packages can be migrated to libusb-1.0.
Comment 10 Stefan Dirsch 2008-11-12 17:17:28 UTC
Thanks a lot, Stanislav!
Comment 12 Stanislav Brabec 2008-11-19 16:36:38 UTC
Upstream requests more information to fix this issue. Could you help with tracking?

We need more details before we can attack something like this. The best 
approach would be to log the bus using usbmon, using working libusb-0.1 
and then broken libusb-compat.. then we can compare the differences.

Note: since the next beta/RC you have to recompile the application with libusb-compat-devel. With older betas, it's sufficient to replace the library.
Comment 13 Marcus Meissner 2008-11-19 16:39:02 UTC
run with
export USB_DEBUG=255   

to get lots of debugging output
Comment 14 Stanislav Brabec 2009-04-15 15:34:21 UTC
Reopening to get a solution for libusb1 and openSUSE 11.2.

The bug occurs exclusively with garmin module and nothing else.

Could you provide any debugging information?
Comment 15 Stefan Dirsch 2009-04-15 15:48:27 UTC
Any hints how to debug this? exporting USB_DEBUG=255 does not change anything in output. :-(
Comment 16 Stanislav Brabec 2009-04-15 17:00:44 UTC
I have created repositories home:sbrabec:libusb0-debug and home:sbrabec:libusb1-debug. They contains packages with debug logging enabled.

Alternatively, you may call usb_set_debug() in your application.
Comment 17 Stefan Dirsch 2009-04-15 22:48:46 UTC
I'm attaching debug log for

  sudo USB_DEBUG=255    gpsbabel -t -i garmin -f usb: -o gpx -F /tmp/out.gpx

for home:sbrabec:libusb0-debug (working) and home:sbrabec:libusb1-debug (broken).
Comment 18 Stefan Dirsch 2009-04-15 22:50:04 UTC
Created attachment 285994 [details]
working
Comment 19 Stefan Dirsch 2009-04-15 22:50:35 UTC
Created attachment 285996 [details]
broken
Comment 20 Stanislav Brabec 2009-04-16 16:31:15 UTC
I am not sure, whether the failed interrupt transfer is read or write. Guessing it is read, but endpoint is 3 (write). Looking at libusb0, it converts it to 0x83.

I have added libusb-compat-interrupt-in-ep.patch forcing read EP for usb_interrupt_read() to home:sbrabec:libusb1-debug libusb-compat package.

Could you try it again with this patch? (Changelog entry: Fix direction bit for usb_interrupt_read().)
Comment 21 Stanislav Brabec 2009-04-16 16:48:51 UTC
And vice versa: Could you please try unpatched libusb-compat with patched version of gpsbabel from home:sbrabec:branches:Application:Geo fixing the same problem on gpsbabel side?
Comment 23 Stefan Dirsch 2009-04-16 17:00:26 UTC
Created attachment 286185 [details]
broken

This is unchanged gpsbabel with libusb-compat-interrupt-in-ep.patch in libusb-compat.
Comment 24 Stefan Dirsch 2009-04-17 03:29:40 UTC
(In reply to comment #23)
> Created an attachment (id=286185) [details]
> broken
> 
> This is unchanged gpsbabel with libusb-compat-interrupt-in-ep.patch in
> libusb-compat.

... which still didn't work as you can see.
Comment 25 Stefan Dirsch 2009-04-17 03:32:35 UTC
(In reply to comment #21)
> And vice versa: Could you please try unpatched libusb-compat with patched
> version of gpsbabel from home:sbrabec:branches:Application:Geo fixing the same
> problem on gpsbabel side?

Great. This one worked for me, but now I have a lot of debug ouput. I'm attaching the log.
Comment 26 Stefan Dirsch 2009-04-17 03:35:04 UTC
Created attachment 286264 [details]
gpsbabel-modified.log.gz
Comment 27 Stefan Dirsch 2009-04-17 03:45:25 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > Created an attachment (id=286185) [details] [details]
> > broken
> > 
> > This is unchanged gpsbabel with libusb-compat-interrupt-in-ep.patch in
> > libusb-compat.
> 
> ... which still didn't work as you can see.

Sorry, I was wrong. It *does* work. I double checked this now.
Comment 28 Stefan Dirsch 2009-04-17 04:01:52 UTC
Ok. Thanks a lot, Stanislav. So where should the patch be applied? In gpsbabel or
libusb-compat? In case you think it's needed to be fixed in gpsbabel, then I can help to discuss this on the gpsbabel-code ML. Just le me know.
Comment 29 Stefan Dirsch 2009-04-17 04:12:58 UTC
Created attachment 286269 [details]
libusb-compat-interrupt-in-ep.patch
Comment 30 Stefan Dirsch 2009-04-17 04:14:01 UTC
Created attachment 286270 [details]
gpsbabel-interrupt-in-ep.patch
Comment 31 Stanislav Brabec 2009-04-17 10:07:44 UTC
It should be fixed in libusb-compat. Fix submitted to Factory and home:sbrabec:backports.

It should be fixed in gpsbabel as well. However it works in libusb0 on linux, it depends on platform dependent and undocumented behavior. For example Darwin port does not contain this code, BSD port contains even more sanity checks not present in Linux port.

I will create yet another patch, that includes BSD sanity checks. It is not appropriate for linux (broken software worked with libusb0 only on BSD).

I provided a better patch fixing both bulk and interrupt transfers. Please test from home:sbrabec:branches:Application:Geo/gpsbabel.
Comment 32 Stanislav Brabec 2009-04-17 10:37:24 UTC
libusb-compat patch including BSD sanitization:

https://sourceforge.net/tracker/?func=detail&aid=2770496&group_id=1674&atid=301674
Comment 33 Stefan Dirsch 2009-04-17 11:35:46 UTC
Thanks. I've tested your latest patches for gpsbabel and libusb-compat. All combinations work so far for me, i.e.

- only patch for gpsbabel applied
- only patch for libusb-compat applied
- both patches for gpsbabel and libusb-compat applied

I'm going to contact gpsbabel-code ML to apply your patch upstream.
Comment 34 Stefan Dirsch 2009-04-18 08:49:15 UTC
The patch has been committed to gpsbabel CVS now.

# cvs log gpslibusb.c
[...]
revision 1.43
date: 2009/04/18 04:30:13;  author: robertl;  state: Exp;  lines: +2 -2
Explictly specify input endpoint for benefit of new buggy libusb.

Date: Fri, 17 Apr 2009 23:36:41 -0500
From: Robert Lipe 
To: Stefan Dirsch 
Cc: gpsbabel-code@lists.sourceforge.net, Stanislav Brabec 
Subject: Re: [Gpsbabel-code] Patch to fix Garmin USB support when using
        usbcompat-devel

Thanx.  Applied.

<rant>
I do feel this is kind of busy work.  As libusb 1.0 managed to reach "1.0"
status (which typically means "all design goals met")  by forgoing
compatibility _and_ OS support for everything but Linux, it remains distant
on my radar.  I wish that project nothing but the best - a cross platform
USB library really does matter - but they've kind of lost the cross-platform
hook that made it interesting.   I'm looking at a new USB device right now,
and not sure if hitching to the libusb train is wise or not.
</rant>

On Fri, Apr 17, 2009 at 10:19 AM, Stefan Dirsch <sndirsch@suse.de> wrote:

> Hi
>
> I think I missed some details here. The command I'm using is
>
>  gpsbabel -t -i garmin -f usb: -o gpx -F /tmp/out.gpx
>
> I'm using gpsbabel 1.3.6.
>
> Best regards,
> Stefan
>
> On Fri, Apr 17, 2009 at 01:47:57PM +0200, Stefan Dirsch wrote:
> > Hi
> >
> > Attached you find a fix for Garmin USB support when using
> > libusb-compat. Without this patch Garmin USB support does not
> > work at all. You just see the message:
> >
> >   Could not start session in a reasonable number of tries.
> >
> > and that's it. libusb-compat is a wrapper library for libusb 1.0 and
> > is supposed to be compatible to libusb 0.1. libusb 1.0 is not
> > compatibel to libusb 0.1.
> >
> > We fixed the issue also in libusb-compat and reported it upstream, but
> > I think it's a good idea to fix it in both places, i.e. libusb-compat
> > and gpsbabel.
> >
> > For more details see
> >
> >   https://bugzilla.novell.com/show_bug.cgi?id=443861
> >
> > or ask Stanislav.
> >
> > Best regards,
> > Stefan
> >
> > --- jeeps/gpslibusb.c
> > +++ jeeps/gpslibusb.c
> > @@ -316,13 +316,13 @@
> >  #define EA(x) x & USB_ENDPOINT_ADDRESS_MASK
> >                       case USB_ENDPOINT_TYPE_BULK:
> >                               if (ep->bEndpointAddress &
> USB_ENDPOINT_DIR_MASK)
> > -                                     gusb_bulk_in_ep =
> EA(ep->bEndpointAddress);
> > +                                     gusb_bulk_in_ep =
> EA(ep->bEndpointAddress) | USB_ENDPOINT_IN;
> >                               else
> >                                       gusb_bulk_out_ep =
> EA(ep->bEndpointAddress);
> >                               break;
> >                       case USB_ENDPOINT_TYPE_INTERRUPT:
> >                               if (ep->bEndpointAddress &
> USB_ENDPOINT_DIR_MASK)
> > -                                     gusb_intr_in_ep =
> EA(ep->bEndpointAddress);
> > +                                     gusb_intr_in_ep =
> EA(ep->bEndpointAddress) | USB_ENDPOINT_IN;
> >                               break;
> >               }
> >       }
>