Review Request 127386: kio-extras mtp protocol: Add support for Windows Phones

Wed Mar 23 16:36:16 UTC 2016


> On March 17, 2016, 5:26 p.m., Dominik Haumann wrote:
> > The only real difference to the original version is that the line
> > 
> >     int isMtpDevice = LIBMTP_Check_Specific_Device(solidBusNum, solidDevNum);
> > 
> > is removed. This somehow implies that this check is too restrictive. If this indeed is the case, then this might very well be a bug in libmtp.
> > 
> > I haved tried to find something in the net, especially since other mtp-clients (from other DEs) are said to work, but couldn't really find something useful for now...
> 
> Dominik Haumann wrote:
>     Found something dating back to 2011: https://sourceforge.net/p/libmtp/mailman/libmtp-discuss/?viewmonth=201101&viewday=21&style=flat
>     Here it says:
>     
>         One more thing. the mtp-probe did not work on older ubuntu's than 10.10.
>         Further investigating showed that it fails at 
>         LIBMTP_Check_Specific_Device () only because libusb does not populate 
>         the correct dev->devnum (i.e. usb device number.
>     
>         However if I manually run mtp-probe, later after a few seconds of device 
>         plugged in, it works properly.
>     
>         Looks like libusb takes a while to get the correct device number. Is 
>         this a known libusb issue or something else?
>     
>     Might it be that we are hit by this issue? According to the link, newer versions of libusb should probably not have this issue.
>     
>     Anyone familiar with this?
> 
> Dominik Haumann wrote:
>     More on this, grepping the entire git repo of gvfs (https://github.com/GNOME/gvfs.git) shows that gvfs does not use LIBMTP_Check_Specific_Device() at all.
>     Similarly, grepping through simple-mtp-fs (https://github.com/phatina/simple-mtpfs) also shows no usage of LIBMTP_Check_Specific_Device().
>     However, both solutions use the other LIBMTP calls similarly to how kio-mtp does.
>     
>     What if we really just omit this check. The worst thing that can happen is that we iterate over all device numbers and get no hit, which just takes a bit longer.
>     
>     Question would be: Do we get too many devices, as Kai was saying? No idea, but this really solves the issue for many users, then this might well be worth a try.
>     
>     Opinions?
> 
> Dominik Haumann wrote:
>     I asked ok libmtp-discuss (https://sourceforge.net/p/libmtp/mailman/message/34948820/) and the answer is as follows:
>     
>         This function probes an USB device for MTP specific properties, in some form
>         of MTP property auto detection. This auto-detection does not necessarily work for all MTP devices,
>         as it is was never well defined or documemted.
>     
>         It also does not check against our existing USB ID database.
>     
>         If you have a device that is in our USB ID database already,
>         LIBMTP_Detect_Raw_Devices will do the by-id detection (and you do that later in the code).
>     
>         Only as fallback you should try LIBMTP_Check_Specific_Device().
>     
>     In other words, following this reasoning, we should not use LIBMTP_Check_Specific_Device() at all as an initial check/filter, meaning that he patch is correct.
>     
>     I don't think I will find more info about this issue without much more investigation. But given the infos we have, I would like to commit - thoungs?
>     
>     @Kai: You were talking about 3 devices that are listed, only one of these working. Can you test or comment on this again?

I got another reply, stating

    That call is in libmtp for one single reason, and it is for the udev helper
    program mtp-detect to probe a device to see if it is MTP if all other
    checks (like against the existing device database) have failed.
    
    Using it in any other code is not adviced, use the detection for raw
    devices as Marcus points out.

In other words: This patch should be correct:
- it adds a nullptr check (probably not requried, but won't hurt)
- it removes the buggy call of LIBMTP_Check_Specific_Device().


- Dominik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127386/#review93660
-----------------------------------------------------------


On March 16, 2016, 4:41 p.m., Dominik Haumann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127386/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 4:41 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Philipp Schmidt.
> 
> 
> Bugs: 325335
>     https://bugs.kde.org/show_bug.cgi?id=325335
> 
> 
> Repository: kio-extras
> 
> 
> Description
> -------
> 
> Right now, Windows Phones (Nokia HW) is not properly recognized by kio-mtp.
> 
> It seems the device is listed but dropped for some reason. This issues exists for quite a long time (also in KDE 4.x), and some people provided this patch (from bug #325335).
> 
> I personally cannot test this patch, as I don't own a Windows Phone.
> 
> However, given this patch works successfully for at least some users (see also https://www.reddit.com/r/kde/comments/4aggg0/error_with_kio_while_trying_to_open_lumia_device/), I think we at least should give it a try.
> 
> Adding group 'kdeframeworks' to reach a broader audience.
> 
> Would be cool to get a proper review for this patch, or even someone who owns such a device!
> 
> 
> Diffs
> -----
> 
>   mtp/devicecache.cpp 8c5c7cf 
> 
> Diff: https://git.reviewboard.kde.org/r/127386/diff/
> 
> 
> Testing
> -------
> 
> Compiles...
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160323/dc050725/attachment-0001.html>