Have to agree with Wade here. sanei_usb selects a config and interface, then loops thru all configs and interfaces, and pulls out endpoints. That's just broken, and it's not only affecting this machine. We'll re-write sanei_usb after the release. I'll start a new thread at that time.
allan On Sat, Apr 25, 2009 at 8:14 AM, Wade Fitzpatrick <wade.fitzpatrick at gmail.com> wrote: > Nicolas > > My latest testing was on kernel 2.6.27-gentoo-r8. I don't see why a > different kernel version would change the interface/endpoint numbering > anyway - surely that is hardware specific to the MP730. I have the latest > version of everything: kernel 2.6.27-gentoo-r8, libusb-0.1.12-r5, sane > backends 1.0.20cvs. It's not a version problem, the MP730 has *never* worked > using libsane. It worked enough to do scans using Wittawat Yamwong's > ``scan'' utility until pixma-0.12.1 but never using libsane. We just never > got that far in testing it back then. > > Ubuntu users also complained about it but obviously never bothered to ask on > sane-devel. > http://ubuntuforums.org/showthread.php?t=592643&highlight=mp730 > http://ubuntuforums.org/showthread.php?t=962535&highlight=mp730 > > I didn't mean to suggest that the patch I have is ready for inclusion in > CVS, only that it should apply to current if somebody wants to use it for > testing. I'm well aware that fixing this properly will mean a significant > rewrite of sanei/sanei_usb.c and will probably touch most if not all backend > drivers if you solve it the way I would, which is to specify the requisite > interfaces and endpoints in the [backend]_config_t structure and pass that > into sanei_usb code. > > Let's take the remaining problems into another thread. > > Cheers, > Wade. > > 2009/4/25 Nicolas Martin <nicolas.martin at freesurf.fr> >> >> Hi Wade, >> >> Sorry if act sometimes like "guardians of the temple", but we need to be >> very accurate on the changes we make into the code, as there are lots of >> shared code in Sane backends (especially pixma) and libs. >> >> - So for points 1 and 2: we can close those. >> >> - Point 3: this is still to investigate, as we must be sure this >> endpoint issue is not brought by kernel 2.6.18-ck1 (BTW, do you know >> what does the -ck1 bring ?). This ck kernel is dated sept-2006 (on >> kernel.org), and after googling a little bit "2.6.18 usb endpoints" it >> looks like there are some issues around that... >> My suggestion would still be to test with a recent kernel, with which we >> are sure Sane and endpoints work fine (recent Ubuntu distros can help a >> lot for that) and check if we can reproduce your endpoints issue. >> My fear here is not to introduce into the Sane code, some turnarounds >> and tweaks only for specific kernels, buggy on usb. >> >> Concerning points 1 and 2 you raise, this is interesting: >> >> The pixma_mp730.c file has been left almost untouched for a while (no >> requests or testing ability so far), so it may be in a less "enhanced" >> state than other ones covering more recent Pixma models, that have been >> recently updated. >> Dennis Lou wrote a while back the pixma_imageclass.c file, based on the >> pixma_mp730.c file, and which covers several Canon ImageClass models in >> the pixma backend. The code has been nicely brushed up, and is now in a >> better shape than mp730's one. >> So for the 2 issues you raise, I'll take a look and compare the >> pixma_mp730.c code with pixma_imageclass.c and pixma_mp150.c , probably >> will help a lot to solve those issues, thanks to your cooperation for >> testing and feedback. >> >> Nicolas >> >> >> Le vendredi 24 avril 2009 ? 01:19 +1000, Wade Fitzpatrick a ?crit : >> > Hi Nicolas >> > >> > Thanks for reviewing my patches. I have to agree with you... too many >> > late nights... must be coding in my sleep... >> > ? ? ?1. I had the PIXMA_CAP_EVENTS flag removed at some stage as >> > ? ? ? ? that's what got the driver working again between pixma-0.12.1 >> > ? ? ? ? and pixma-0.12.2. It doesn't seem to make any difference now >> > ? ? ? ? and I can't explain why so just ignore it. I don't think the >> > ? ? ? ? resolution and maximum scan area parameters are correct but >> > ? ? ? ? that's an issue for another day. >> > ? ? ?2. I don't remember adding that line but I guess I must have. It >> > ? ? ? ? may have been an accidental 'p' in vim. Ignore this one too. >> > ? ? ?3. I checked out a fresh new cvs today and applied just the lines >> > ? ? ? ? you pasted below to sanei/sanei_usb.c - it works just fine >> > ? ? ? ? with that change alone. I ran through about 30 successful >> > ? ? ? ? tests too: using ADF, button-controlled, ADF >> > ? ? ? ? +button-controlled, Gray, Color and at various resolutions and >> > ? ? ? ? geometries on and off the platen glass, pressing Cancel during >> > ? ? ? ? a scan, pressing Ctrl-C during a scan, jamming the paper in >> > ? ? ? ? the ADF, batch scans etc. >> > The only problems I can find now are: >> > ? ? ?1. Scanning at 1200dpi Grayscale. Sometimes it hangs depending on >> > ? ? ? ? the x and y values - the larger the values, the sooner it >> > ? ? ? ? fails. If it does succeed, the data is always bogus. >> > ? ? ?2. The return code after a successful scan is always ECANCELED >> > ? ? ? ? which causes a problem for batch mode as it will only scan one >> > ? ? ? ? page. It doesn't seem to matter for single scans. >> > Does this hold true for other models too? >> > Does batch mode succeed on other models with ADF? >> > >> > I can't believe I've spent so much time on such a trivial change :( >> > >> > Regards, >> > Wade. >> > 2009/4/23 Nicolas <nicolas0martin at gmail.com> >> > ? ? ? ? Hi Wade, >> > >> > ? ? ? ? Good news that you succeeded in having your MP730 work after >> > ? ? ? ? 2,5 years ! >> > >> > ? ? ? ? I did carefully inspect the small changes you have done to the >> > ? ? ? ? pixma backend, but there are some points I don't understand >> > ? ? ? ? clearly, or maybe I've missed sthg, but you can surely give us >> > ? ? ? ? some more info here: >> > >> > ? ? ? ? 1/ File backend/pixma_mp730.c >> > >> > ? ? ? ? I don't understand the change you've brought here individually >> > ? ? ? ? for each device, the line that you replaced for MP730: >> > >> > ? ? ? ? - ?DEVICE ("Canon MultiPASS MP730", "MP730", MP730_PID, 1200, >> > ? ? ? ? 637, 868, PIXMA_CAP_ADF), >> > ? ? ? ? + ?DEVICE ("Canon MultiPASS MP730", "MP730", MP730_PID, 1200, >> > ? ? ? ? 637, 868, PIXMA_CAP_ADF|PIXMA_CAP_EVENTS), >> > >> > ? ? ? ? has exactly the same effect than the original one in the >> > ? ? ? ? device definition: >> > ? ? ? ? - ? ? ? ?PIXMA_CAP_GRAY|PIXMA_CAP_EVENTS|cap ? ? ? ? ? \ >> > ? ? ? ? + ? ? ? ? ? ? ?PIXMA_CAP_GRAY|cap /* capabilities */ >> > ? ? ? ? ?\ >> > >> > ? ? ? ? Here, you move the PIXMA_CAP_EVENTS flag from the global >> > ? ? ? ? device definition to the individual definition of each device. >> > >> > ? ? ? ? - For MP730, this looks to have the same effect, or where is >> > ? ? ? ? the difference ? >> > ? ? ? ? - But this changes the definition for all other devices here, >> > ? ? ? ? and I would not like to bring here any regression to other >> > ? ? ? ? devices. >> > >> > ? ? ? ? So: could you give the motivation for this modification here ? >> > >> > ? ? ? ? 2/ file pixma_io_sanei.c >> > >> > ? ? ? ? The only diff I see here adds a PDBG debug statement: >> > ? ? ? ? + PDBG (pixma_dbg (3, "sanei_usb_read_bulk returned error >> > ? ? ? ? code: %i\n", error)); >> > >> > ? ? ? ? The other line is a commented line: >> > >> > ? ? ? ? + ? ? ?/*error = pixma_map_status_errno (pixma_get_be16 >> > ? ? ? ? (buf)); */ >> > >> > ? ? ? ? And I don't understand the comment you added: >> > ? ? ? ? "The call to |pixma_map_status_errno() in | >> > ? ? ? ? backend/pixma_io_sane.c:pixma_read() >> > ? ? ? ? is in the wrong place as |pixma_read() may be re-entrant so as >> > ? ? ? ? to read a partial buffer." >> > >> > ? ? ? ? - There are _no_ calls to |pixma_map_status_errno() in current >> > ? ? ? ? cvs ||pixma_read() >> > ? ? ? ? The one here is added as part of you modifications, but is >> > ? ? ? ? commented out. >> > ? ? ? ? - This part of the pixma code is common to all pixma devices >> > ? ? ? ? - The modifications you added here do not bring any functional >> > ? ? ? ? change. >> > >> > ? ? ? ? So one again, what are those changes for ? >> > >> > ? ? ? ? 3/ File sanei_usb.c >> > >> > ? ? ? ? This is the only file where modifications appear to have a >> > ? ? ? ? functional impact: >> > ? ? ? ? |+ ? ? ? ? ? ? ? ? /* don't try to read non-scanner device >> > ? ? ? ? classes */ >> > ? ? ? ? + ? ? ? ? ? ? ? ? if (interface->bInterfaceClass == 0x07) { >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? DBG (3, "sanei_usb_open: skipping Printer >> > ? ? ? ? interface\n"); >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? continue; >> > ? ? ? ? + ? ? ? ? ? ? ? ? } >> > ? ? ? ? + ? ? ? ? ? ? ? ? if (interface->bInterfaceClass == 0x08) { >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? DBG (3, "sanei_usb_open: skipping Mass >> > ? ? ? ? Storage interface\n"); >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? continue; >> > ? ? ? ? + ? ? ? ? ? ? ? ? } >> > ? ? ? ? + ? ? ? ? ? ? ? ? if (interface->bInterfaceClass == 0xff && i >> > ? ? ? ? == 3) { >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? DBG (3, "sanei_usb_open: skipping second >> > ? ? ? ? Vendor Specific interface\n"); >> > ? ? ? ? + ? ? ? ? ? ? ? ? ? continue; >> > ? ? ? ? + ? ? ? ? ? ? ? ? } >> > >> > ? ? ? ? As Allan pointed out, this may need some deeper analysis to >> > ? ? ? ? the sanei_usb code, but modifications in this file are very >> > ? ? ? ? sensitive, as they impact all usb devices >> > ? ? ? ? supported by Sane, not only pixma. >> > ? ? ? ? As for the 1.0.20 release, I would not recommend to add this >> > ? ? ? ? change now, as it probably needs deeper testing and >> > ? ? ? ? understanding. >> > >> > ? ? ? ? In other words, to step further, Wade: >> > ? ? ? ? Could you give a try with the current Sane CVS files and patch >> > ? ? ? ? only file sanei_usb.c, see if it is still ok? >> > ? ? ? ? If not, this means there's something I did not catch, so we'll >> > ? ? ? ? need to talk again to finalize the changes, if any, required >> > ? ? ? ? for MP730. >> > >> > ? ? ? ? Nicolas >> >> > >> >> > > -- "The truth is an offense, but not a sin"
