findmyphone: Turn on camera flash as well
AbandonedPublic
Actions

Authored by albertvaka on Oct 6 2017, 5:12 PM.

Details

Reviewers

nicolasfella
arjun

Group Reviewers

KDE Connect

Summary

Turning on the camera flash would make it easier to find it in the dark.

Diff Detail

Repository

R225 KDE Connect - Android application

Lint

Lint Skipped

Unit

Unit Tests Skipped

arjun created this revision.Oct 6 2017, 5:12 PM

We have had this patch in the past I think.

This requires having Camera privileges, which feels rather shady to me...

I'm also on the side that this permission is not worth asking just for this plugin...

Since it's only an optional Permission IMHO it would be okay to ask for it and provide an explanation. The user can still decline it if unwanted.

res/values/strings.xml
219	Please give a better explanation why the flash is needed

nicolasfella added inline comments.Oct 9 2017, 7:50 PM

AndroidManifest.xml
36	Do we really need the Camera Permission if we got the Flashlight Permission?

arjun added inline comments.Oct 10 2017, 2:37 PM

AndroidManifest.xml
36	Not sure, I'm new to android. It seems you can't turn on the flashlight without the camera permission.
res/values/strings.xml
219	Like what? "If you want the flash to turn on while ringing, please grant access to the camera" ?

In D8161#153854, @nicolasfella wrote:

Since it's only an optional Permission IMHO it would be okay to ask for it and provide an explanation. The user can still decline it if unwanted.

When will the user decide if she wants the flash to trigger when there's an alarm?

In D8161#165243, @apol wrote:

In D8161#153854, @nicolasfella wrote:

Since it's only an optional Permission IMHO it would be okay to ask for it and provide an explanation. The user can still decline it if unwanted.

When will the user decide if she wants the flash to trigger when there's an alarm?

Like with other optional permissions, it's shown in the list of controls for a paired device. So you can decide beforehand if you want the functionality (and grant camera permissions) or not.

I think the functionality is a nice addition. That being said, it does not work for me (even after granting permission). I only get the following messages in the logcat:

E/MediaPlayer-JNI: JNIMediaPlayerFactory: bIsQCMediaPlayerPresent 0
E/MediaPlayer-JNI: JNIMediaPlayerFactory: bIsQCMediaPlayerPresent 0
D/MediaPlayer: setSubtitleAnchor in MediaPlayer
D/Ringtone: Successfully created local player
E/MediaPlayer-JNI: JNIMediaPlayerFactory: bIsQCMediaPlayerPresent 0
E/MediaPlayer-JNI: JNIMediaPlayerFactory: bIsQCMediaPlayerPresent 0
D/MediaPlayer: setSubtitleAnchor in MediaPlayer
D/Ringtone: Successfully created local player

AndroidManifest.xml
37	Does such a permission exist? It's not in the list at https://developer.android.com/reference/android/Manifest.permission.html
res/values/strings.xml
219	Something like "To turn on the flashlight in the find-my-phone functionality, you need to give permission to access the phone's camera" seems good to me. It emphasizes for what the functionality is needed, and should make it clearer what happens if you deny the permission.

Doesn't work for me either

This revision now requires changes to proceed.Dec 20 2017, 4:58 PM

albertvaka commandeered this revision.Mar 25 2018, 11:50 AM

albertvaka abandoned this revision.

albertvaka added a reviewer: arjun.

Revision Contents
Changeset List

			Path	Packages
M			AndroidManifest.xml (5 lines)
M			res/values/strings.xml (1 line)
M			src/org/kde/kdeconnect/Plugins/FindMyPhonePlugin/FindMyPhoneActivity.java (23 lines)
M			src/org/kde/kdeconnect/Plugins/FindMyPhonePlugin/FindMyPhonePlugin.java (14 lines)