D11773: New plugin: Find this device

[Aleix Pol Gonzalez]

apol added a comment.


  +1 overall. Will be needed both for sailfish and plasma mobile anyway, I've never lost my laptop (or desktop x'D) at home, but I don't think it hurts.

INLINE COMMENTS

> findthisdeviceplugin.cpp:41
> +namespace Strings {
> +// TODO: what would be a good default?
> +inline QString defaultSound() { return QStringLiteral("KDE-Im-Phone-Ring.ogg"); }

Looks to me as good as any, I'd remove the TODO. I think this one is okay, if we ever want to change we change.

> findthisdeviceplugin.cpp:61
> +
> +    const QString soundFilename = config()->get<QString>(QStringLiteral("ringtone"), Strings::defaultSound());
> +

I'm not sure it makes sense to send which ringtone needs to be sounding on the other side. Is this how we do it for Android?

> findthisdeviceplugin.cpp:66
> +    for (const QString &dataLocation : dataLocations) {
> +        soundURL = QUrl::fromUserInput(soundFilename,
> +                                       dataLocation + QStringLiteral("/sounds"),

fromLocalFile

REPOSITORY
  R224 KDE Connect

REVISION DETAIL
  https://phabricator.kde.org/D11773

To: kossebau, #kde_connect
Cc: apol, nicolasfella, adeen-s, SemperPeritus, ahmedbesbes, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180329/21b35776/attachment.html>