D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler updated this revision to Diff 43074. akrutzler marked 4 inline comments as done. akrutzler added a comment. - Rebase - Reserve memory for lists/vectors in advance. - Prepend "static" to all class-level functions - Fix typos REPOSITORY R320 KIO Extras CHANGES SINCE LAST

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

dfaure accepted this revision. dfaure added a comment. Great stuff. Only found some typos and very minor things, feel free to fix and push. INLINE COMMENTS > mtpdevice.cpp:119 > +{ > +QList list; > +for (const MTPStorage *storage : m_storages) { Technically this is missing a

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler added inline comments. INLINE COMMENTS > mlaurent wrote in mtpstorage.cpp:176 > QString() directly no ? Of course, thanks! :) REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15277 To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure, mlaurent

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler updated this revision to Diff 42685. akrutzler marked 5 inline comments as done. akrutzler added a comment. - Use QString() directly instead of QStringLiteral("") - Use explicit specifier - Call either finished or error in slave methods REPOSITORY R320 KIO Extras CHANGES

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

mlaurent requested changes to this revision. mlaurent added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > mtpstorage.cpp:176 > +case LIBMTP_FILETYPE_UNDEF_AUDIO: > +return QStringLiteral(""); > +case LIBMTP_FILETYPE_WMV: QString() directly no

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler added inline comments. INLINE COMMENTS > dfaure wrote in mtpstorage.cpp:499 > DBus timeouts are actually configurable, if you need to block until the copy > is done, btw. In my previous approach I increased the timeout to 10 minutes. This worked fine until I downloaded a file of

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler updated this revision to Diff 42450. akrutzler marked 13 inline comments as done. akrutzler added a comment. - Rebase - Remove unrelated changes - Remove Q_FUNC_INFO - Remove private static methods from header file - Use currentDateTimeUtc instead of currentDateTime -

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler added a comment. Thanks for the review. I will address your comments and hopefully have a patch ready in the next days. :) REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15277 To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure Cc:

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

dfaure added inline comments. INLINE COMMENTS > elvisangelaccio wrote in kmtpd.json:10 > Currently the module is spawned by the ioslave on-demand. But imho it should > be possible to browse the device using only dbus, without having to open > dolphin first. > > Can we try to autostart the

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. This is excellent work, thanks a lot for doing this. I just have "a few" minor comments... ;) INLINE COMMENTS > kio_mtp.cpp:107 > { > -qCDebug(LOG_KIO_MTP) << url.path(); >

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

elvisangelaccio added inline comments. INLINE COMMENTS > kmtpd.json:10 > +"X-KDE-DBus-ServiceName": "kmtpd", > +"X-KDE-Kded-autoload": false, > +"X-KDE-Kded-load-on-demand": true Currently the module is spawned by the ioslave on-demand. But imho it should be possible to browse the

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

elvisangelaccio added a reviewer: dfaure. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15277 To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham,

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

elvisangelaccio added a comment. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. From a quick look, the architecture is sound and respects what was discussed in T9390 . And indeed it works pretty well,