|
Bugzilla – Full Text Bug Listing |
| Summary: | libusb-0_1-4 incompatible to libusb (breaks USB support in gpsbabel) | ||
|---|---|---|---|
| Product: | [openSUSE] openSUSE 11.2 | Reporter: | Stefan Dirsch <sndirsch> |
| Component: | Other | Assignee: | 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
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 (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. 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. >> => 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. 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. Thanks for explanation, Stanislav. I now understand why we introduced this compat library. Added dstoecker since he's maintaining gpsbabel in our buildservice. 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. Thanks a lot, Stanislav! See bug 439574 and bug 432129 for more regressions. Upstream report: http://sourceforge.net/mailarchive/forum.php?thread_name=1226510657.1845.60.camel%40hammer.suse.cz&forum_name=libusb-devel opensuse-factory announce: http://lists.opensuse.org/opensuse-factory/2008-11/msg00456.html 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. run with export USB_DEBUG=255 to get lots of debugging output 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? Any hints how to debug this? exporting USB_DEBUG=255 does not change anything in output. :-( 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. 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). Created attachment 285994 [details]
working
Created attachment 285996 [details]
broken
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().) 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? Reported to libusb upstream: http://sourceforge.net/mailarchive/forum.php?thread_name=1239900748.21612.215.camel%40hammer.suse.cz&forum_name=libusb-devel Created attachment 286185 [details]
broken
This is unchanged gpsbabel with libusb-compat-interrupt-in-ep.patch in libusb-compat.
(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. (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. Created attachment 286264 [details]
gpsbabel-modified.log.gz
(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. 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. Created attachment 286269 [details]
libusb-compat-interrupt-in-ep.patch
Created attachment 286270 [details]
gpsbabel-interrupt-in-ep.patch
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. libusb-compat patch including BSD sanitization: https://sourceforge.net/tracker/?func=detail&aid=2770496&group_id=1674&atid=301674 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. 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; > > } > > } > |