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

2016-03-31 Thread Dominik Haumann 

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

(Updated March 31, 2016, 1:12 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Faure and Philipp Schmidt.


Changes
---

Submitted with commit cba927613b6da1b36ecbd5cfadfff3bff8729f86 by Dominik 
Haumann to branch master.


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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-23 Thread Dominik Haumann 


> 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=21=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

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

2016-03-20 Thread Dominik Haumann 


> On March 16, 2016, 1:24 a.m., David Edmundson wrote:
> > mtp/devicecache.cpp, line 116
> > 
> >
> > int isMtpDevice = LIBMTP_Check_Specific_Device(solidBusNum, 
> > solidDevNum);
> > 
> > this line looks important
> > 
> > from what I can tell, this patch simply tries to use every possible 
> > device as an MTP client skipping all the checks.
> > 
> > I would have though that would have had some downsides.
> > 
> > Does viewing mtp:/ list any extra entries?
> 
> Dominik Haumann wrote:
> Yes, as I understand, this scans all devices. However, non-functional 
> mtp-devices are then filtered out by the call of err = 
> LIBMTP_Detect_Raw_Devices(, ); in line 127 (left 
> pane).
> 
> So this patch should just be less pedantic in the first place. I have 
> asked the guy on reddit to try the original version, so let's see whether 
> this is really needed.
> 
> Kai Uwe Broulik wrote:
> I have an issue where it shows three devices for my phone, two of which 
> don't work, and I fear that this may make it worse or re-introduce such an 
> issue.

@Kai: Can you try this new patch then? The updated version doesn't list all 
devices anymore.


- Dominik


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


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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-19 Thread Dominik Haumann 

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



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


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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-19 Thread Dominik Haumann 


> 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...

Found something dating back to 2011: 
https://sourceforge.net/p/libmtp/mailman/libmtp-discuss/?viewmonth=201101=21=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


---
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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-19 Thread Dominik Haumann 


> On März 17, 2016, 5:26 nachm., 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=21=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?

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?


- Dominik


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


On März 16, 2016, 4:41 nachm., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127386/
> ---
> 
> (Updated März 16, 2016, 4:41 nachm.)
> 
> 
> 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:

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

2016-03-19 Thread Dominik Haumann 


> 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=21=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?

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


---
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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-19 Thread Dominik Haumann 

---
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.


Changes
---

Next version, which uses again only the 
Solid::Device::listFromType(Solid::DeviceInterface::PortableMediaPlayer, 
QString()).

Following the comments on 
https://www.reddit.com/r/kde/comments/4aggg0/error_with_kio_while_trying_to_open_lumia_device/
this still works compared to the unpatched version.


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 (updated)
-

  mtp/devicecache.cpp 8c5c7cf 

Diff: https://git.reviewboard.kde.org/r/127386/diff/


Testing
---

Compiles...


Thanks,

Dominik Haumann

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-16 Thread Kai Uwe Broulik 


> On März 16, 2016, 1:24 vorm., David Edmundson wrote:
> > mtp/devicecache.cpp, line 116
> > 
> >
> > int isMtpDevice = LIBMTP_Check_Specific_Device(solidBusNum, 
> > solidDevNum);
> > 
> > this line looks important
> > 
> > from what I can tell, this patch simply tries to use every possible 
> > device as an MTP client skipping all the checks.
> > 
> > I would have though that would have had some downsides.
> > 
> > Does viewing mtp:/ list any extra entries?
> 
> Dominik Haumann wrote:
> Yes, as I understand, this scans all devices. However, non-functional 
> mtp-devices are then filtered out by the call of err = 
> LIBMTP_Detect_Raw_Devices(, ); in line 127 (left 
> pane).
> 
> So this patch should just be less pedantic in the first place. I have 
> asked the guy on reddit to try the original version, so let's see whether 
> this is really needed.

I have an issue where it shows three devices for my phone, two of which don't 
work, and I fear that this may make it worse or re-introduce such an issue.


- Kai Uwe


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


On März 15, 2016, 5:52 nachm., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127386/
> ---
> 
> (Updated März 15, 2016, 5:52 nachm.)
> 
> 
> 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-16 Thread Dominik Haumann 


> On March 16, 2016, 1:24 a.m., David Edmundson wrote:
> > mtp/devicecache.cpp, line 116
> > 
> >
> > int isMtpDevice = LIBMTP_Check_Specific_Device(solidBusNum, 
> > solidDevNum);
> > 
> > this line looks important
> > 
> > from what I can tell, this patch simply tries to use every possible 
> > device as an MTP client skipping all the checks.
> > 
> > I would have though that would have had some downsides.
> > 
> > Does viewing mtp:/ list any extra entries?

Yes, as I understand, this scans all devices. However, non-functional 
mtp-devices are then filtered out by the call of err = 
LIBMTP_Detect_Raw_Devices(, ); in line 127 (left pane).

So this patch should just be less pedantic in the first place. I have asked the 
guy on reddit to try the original version, so let's see whether this is really 
needed.


- Dominik


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


On March 15, 2016, 5:52 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127386/
> ---
> 
> (Updated March 15, 2016, 5:52 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-15 Thread David Edmundson 

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




mtp/devicecache.cpp (line 113)


int isMtpDevice = LIBMTP_Check_Specific_Device(solidBusNum, solidDevNum);

this line looks important

from what I can tell, this patch simply tries to use every possible device 
as an MTP client skipping all the checks.

I would have though that would have had some downsides.

Does viewing mtp:/ list any extra entries?


- David Edmundson


On March 15, 2016, 5:52 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127386/
> ---
> 
> (Updated March 15, 2016, 5:52 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2016-03-15 Thread Dominik Haumann 

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

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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel