New plugin: Find this device
ClosedPublic

Authored by kossebau on Mar 28 2018, 11:51 PM.

Details

Summary

Allows other devices to make this device discoverable via a
kdeconnect.findmyphone.request command, if running.

Currently supports playing a sound.

Counterpart to FindMyPhone plugin

Test Plan

Connect with other device running KDE Connect (with Plasmoid).
Select a working play sound in the Find My Device plugin settings.
On other device trigger Find My Phone button for this device in KDE Connect
Plasmoid and notice this device playing the configured sound.

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau requested review of this revision.Mar 28 2018, 11:51 PM
kossebau created this revision.

What use case do you have in mind?

With Plasma/KDE Connect running on mobile devices of different form factors (tablet, minilaptops, phone), which may be subject to uncontrolled roaming in the crowded flats/houses/offices due to shared use by family members, friends, staff, visitors, it can be helpful to have an option to relocate a device when location is unknown.

E.g. kids left the family laptop behind the couch, or staff left the device in meeting room or covered by boxes/papers.

So similar use-case like with FindMyPhone plugin from KDE Connect Android, just not limited to Android devices.

apol added a subscriber: apol.Mar 29 2018, 2:24 AM

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

plugins/findthisdevice/findthisdeviceplugin.cpp
42

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.

62

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?

67

fromLocalFile

kossebau updated this revision to Diff 30840.Mar 29 2018, 11:26 AM
  • remove TODO note for default sound
  • fix to use file name as shipped with plasma5 oxygen tarball instead of kdebase4 one
plugins/findthisdevice/findthisdeviceplugin.cpp
62

This defines the ring tone for this device when being called/pinged, not the other :)

With the Android version this is done the same, cmp. https://phabricator.kde.org/R225:4d5d7449200a240ebc05aa71a60e7f53f03bde7b

Ideally this could be configured once per this device (think family members connecting all their mobiles to the family laptop to be able to find it*), right now it needs to be configured for any connection. But there is no way yet to do such global connection-independent config of a plugin, right?

  • And yes, Android KDEConnect also really needs some FindWithMyPhone ;)
67

I followed here the code from knotification / knotifyconfig which both use QUrl::fromUserInput. See also related commit messages ( 1 2.

Not sure what fancy usages people might have or want to enter, but given the examples in the commit messages, I should be safe to stay consistent with that other code.

Wouldn't it make more sense to integrate this into the existing "Find my phone" plugin?

Wouldn't it make more sense to integrate this into the existing "Find my phone" plugin?

Perhaps. Depends if people think there are devices which would just be searched *for* or devices which are just searched *from*. In that case one might prefer to have the plugins separated, as proposed initially here.

For one, any mobile device is candidate both for being searched for and searched from, given their mobile nature :)
On the other hand, not any connected device and its users should be able to trigger the Find feature for any other connected device (think device for visitors/kids which should not be able to locate host's/mum's phone, but the other way around you want to be able).

That's why I proposed this as separate plugin, to allow separate enabling in which direction the Find feature works.

Wouldn't it make more sense to integrate this into the existing "Find my phone" plugin?

Perhaps. Depends if people think there are devices which would just be searched *for* or devices which are just searched *from*. In that case one might prefer to have the plugins separated, as proposed initially here.

For one, any mobile device is candidate both for being searched for and searched from, given their mobile nature :)
On the other hand, not any connected device and its users should be able to trigger the Find feature for any other connected device (think device for visitors/kids which should not be able to locate host's/mum's phone, but the other way around you want to be able).

That's why I proposed this as separate plugin, to allow separate enabling in which direction the Find feature works.

Why wouldn't you want to ability to find a device? It'll never hurt you if you don't use it, and it gives you less plugins in the list.

And if you don't want someone to be able to find a phone, why would you even connect? That doesn't make sense.

Wouldn't it make more sense to integrate this into the existing "Find my phone" plugin?

Perhaps. Depends if people think there are devices which would just be searched *for* or devices which are just searched *from*. In that case one might prefer to have the plugins separated, as proposed initially here.

For one, any mobile device is candidate both for being searched for and searched from, given their mobile nature :)
On the other hand, not any connected device and its users should be able to trigger the Find feature for any other connected device (think device for visitors/kids which should not be able to locate host's/mum's phone, but the other way around you want to be able).

That's why I proposed this as separate plugin, to allow separate enabling in which direction the Find feature works.

Why wouldn't you want to ability to find a device? It'll never hurt you if you don't use it, and it gives you less plugins in the list.

And if you don't want someone to be able to find a phone, why would you even connect? That doesn't make sense.

I think the usecase described by @kossebau makes sense: Mom and Dad have paired with their 10 year old kid's Android device and want to be able to find that device when it falls behind the sofa but don't want the kid to be able to make their computers ring whenever they tell him he can't have a cookie. Being able to switch on Find my Phone but switch off Find this Device makes sense.

Maybe the same usecase be achieved by having settings switches in the Plasmoid so that you could disable a device from being found by another device rather than having two separate plugins?

@kossebau If you have Telegram you might want to join our group https://t.me/joinchat/AOS6gA37orb2dZCLhqbZjg
Hopefully we will have an IRC Bridge soon

Why wouldn't you want to ability to find a device? It'll never hurt you if you don't use it, and it gives you less plugins in the list.

And if you don't want someone to be able to find a phone, why would you even connect? That doesn't make sense.

I think the usecase described by @kossebau makes sense: Mom and Dad have paired with their 10 year old kid's Android device and want to be able to find that device when it falls behind the sofa but don't want the kid to be able to make their computers ring whenever they tell him he can't have a cookie. Being able to switch on Find my Phone but switch off Find this Device makes sense.

@mtijink I agree with you it makes little/no sense for devices which all belong to the same person and only accessible to them.
But IMHO kdeconnect also offers solutions to use cases involving devices used by many different persons, at which point you want to have more tight control whom's device (or the group of people having access to it with the given login) can do what to your own device or someone elses.
And the device (re)discover feature would be one where I, based on real life stories both in families and work, can see that one want to have selective control about which devices/users can explore the location of which other devices/users. Nothing everyone needs, but those who do will find it good to have, as otherwise the complete feature is no-go. And supporting those by a separate toggle should not hurt the rest too much. But see also braindump about plugin/features in general below, seems to me this is rather an example for a principle challenge yet to be solved.

Maybe the same usecase be achieved by having settings switches in the Plasmoid so that you could disable a device from being found by another device rather than having two separate plugins?

Raises these questions with me:

  • where would people look to disable/enable a feature? what features do they map to plugins, so expect toggles at that level, what do they expect at other
  • is this something people want to toggle on/off often, so it needs to be more quickly to reach?

Doing this as separate plugin has been for now rather pragmatic, it gave an UI option to toggle the service for free, no need to write a custom config (for now for the plasmoid, but also any future platform specific UI).

I also understand the desire to keep the plugin list short, for the need of simple overview. Just, given kdeconnect is plugin-based by nature, if people go crazy and write lots of different plugins for different features (which is a goal,. no?), this desire cannot be fulfilled by principle, once the number of plugins outgrows 7 or whatever number people can manage in their mind at the same time. And the plugin managing UI needs some means to deal with this anyway (like having tags or at least group plugins by categories).
So keeping the actual number of plugins low might be less a criteria than how to provide people a simple consistent UI concept, so they can quickly setup/configure all the services provided to (an)other device(s) and services used from (an)other device(s).

From my own experiments so far (with developer mind, but also user one), I actually would like a kdeconnect configuration UI which is less driven by the technical idea of plugins, but more of (my?) mental model of services/features provided _to_ another device and used _from_ another device, where the client-server model matches, and then peer2peer ones, where the feature is bidirectional, like chat.

Currently for things like sharing clipboard, notifications, data/files, etc., one has to read very precisely the description of a plugin to make sure if this plugin works in the correct direction. And for some features there are separate plugins per direction (like notifications), while for others there are only single ones for both directions (like share).
So while those plugins assumingly have been done based on most common use-case, ideally they would be generalized, so not-so-common use-cases can be covered as well.

I have no opinion yet if this splitting by principle into server and client parts per features should be done by implementation and thus separate plugins, or rather only on the UI level. Needs some rethinking.

Too much text already, but no time to cut it short, so just stopping the braindump here :) Guess this should be better moved into an own principal discussion? Would see to write an email to the mailinglist about this.

@kossebau If you have Telegram you might want to join our group https://t.me/joinchat/AOS6gA37orb2dZCLhqbZjg
Hopefully we will have an IRC Bridge soon

Thanks for invitation :) Not on telegram though, so will have to wait for IRC bridge.

I can't test it, but it looks good.

I guess the usecase for separate plugins is good, but it might indeed need some future thinking. I'd say that all plugins with "two sides" should be merged into one plugin, with (easily accessible) option to control the which sides are enabled. Or something which corresponds with that visually.

nicolasfella accepted this revision.May 12 2018, 4:03 PM

Works fine for me and I can't find anything obviously wrong in the code

This revision is now accepted and ready to land.May 12 2018, 4:03 PM
Restricted Application added a subscriber: kdeconnect. · View Herald TranscriptMay 12 2018, 4:03 PM
This revision was automatically updated to reflect the committed changes.
nicolasfella reopened this revision.May 15 2018, 7:11 PM

Sorry, but your commit broke the build, I reverted it for now
/home/nico/workspace/kdeconnect-kde/plugins/findthisdevice/findthisdevice_config.cpp:58:86: error: 'changed' is a protected member of 'KCModule'

this, static_cast<void (KdeConnectPluginKcm::*)()>(&KdeConnectPluginKcm::changed));
                                                    ~~~~~~~~~~~~~~~~~~~~~^~~~~~~

/usr/include/KF5/KConfigWidgets/kcmodule.h:377:10: note: must name member using the type of the current context 'FindThisDeviceConfig'

void changed();
     ^
This revision is now accepted and ready to land.May 15 2018, 7:11 PM
nicolasfella requested changes to this revision.May 15 2018, 7:11 PM
This revision now requires changes to proceed.May 15 2018, 7:11 PM

Hm, strange, it had build for me and also on CI. Oh, though not in the Windows build I now see. Will investigate.

Okay, think the reason is found: seems Qt with some compilers somehow is more strict in the connect() logic for handling function pointers and does not accept using base class names with the function pointer passed to connect() call. Cmp. also the "note: must name member using the type of the current context 'FindThisDeviceConfig'" in the build log.

krop on irc hinted me to d7f93680 in knotes where a similar fix was needed.

Will use the same approach and recommit :) (QOverload is since Qt 5.7, so matching min version of kdeconnect-kde)

This revision was not accepted when it landed; it landed in state Needs Revision.May 15 2018, 8:16 PM
This revision was automatically updated to reflect the committed changes.

Thanks, works for me!