izivkov / CasioGShockSmartSync

Apache License 2.0
115 stars 12 forks source link

Improvements on phone finder action #42

Closed izivkov closed 1 year ago

izivkov commented 1 year ago

@gmedina-de Let's discuss here:

Ok, I did not get notification because I had notifications disabled for the app. However, this could be a problem for other users.

One issue is the notification disappear when the watch disconnects. This most likely will be the case for most users by the time they find the phone. So, dialog is preferred. You mentioned there was a problem with the dialog, but can it not be overcome?

I considered using a foreground service before, like the Casio official apps, but I decided against it. The main reason is I dislike apps which are always "In you face", always displaying notification and telling me, they are waiting for connection. Maybe it is just me, but I got enough thing on my phone vying for attention. I prefer a low-key approach.

I'm also working to call your Find Phone action by long-pressing the lower-right button, which is a special feature of the Casio phone, so we should use it instead. I will commit the code, including your previous PR.

In the mean time, my preference is not to use Foreground service, and use dialog instead of notification. But I'm happy to discuss your ideas here.

germede commented 1 year ago

Hey there @izivkov ,

For the issue with notification being disabled (happened also to me) I would suggest to let POST_NOTIFICATION being offered to the user the same way other permission are being offered when running the App for the first time (or maybe as a must-have?).

Because the find phone notification is running on another notification channel, it can be discarded (and the ringing being stopped) individually. Problem with my first naive AlertDialog approach are:

  1. the dialog is not being shown if one has the app closed or hidden (maybe because no Activity is being shown), so that I had to force stop the whole app in the settings in order to stop ringing.
  2. the loud ringing can be stopped much easier and faster having it as a high priority notification (even with locked screen), so the user doesn't have to open the app explicitly.

I understand the issue with "in your face" notification of the foreground service as well, but they can be (at least on newer Android versions) disabled for each notification channel individually, still letting the service running in the background / letting other channels like phone finder show notifications. See for example KDE connect app, which also runs as a foreground service but notifications can be disabled, such that the user doesn't get annoyed. Only

However without this foreground service the actions functionality of this app is being heavily constrained: Whenever I try to find my phone, the app is not running if I didn't open it in the last 5 minutes and let it hidden (won't work if the app is closed).

Thank you for opening this discussion thread!

izivkov commented 1 year ago

@gmedina-de Hi, ok, let's try it your way. Do you think you can add the Notification permissions at the start of the app, and we can see what the user experience would be?

Thanks

germede commented 1 year ago

@izivkov,

I've tried it just by adding it to the permission manager. Now it's asking for it when opening the app.

I also noticed that the foreground service triggering actions only worked reliable once. MainActivity was throwing exceptions because it wasn't able to find navigation view, that's why I put some null checks when navigating to the home fragment...

The alarm volume being maximized may also be an issue for user experience. However I think this is an advantage when finding a phone. An improvement to consider might be to restore the previous volume after stop ringing. Added a commit for that.

izivkov commented 1 year ago

I've been using your branch today to test it. I like your code, you have streamlined the MainActivity, and have created separate PhoneFinder class (actually, it together with CameraCapture and Flashligh probobly belong in the actions directory, but this is another story).

So now for the problems...There are at least two hard stoppers which prevent us from merging into main.

  1. The app cannot reconnect after the first time. I have not looked into it, but I think it has to do with foreground service, and not calling onResume() which currently does a lot of the re-connection.
  2. When the notification appears, and if you wait for the watch to disconnect, the notification disappears when the disconnection message appears at the bottom of the screen. The user has to find the notification from the top of the screen and swipe it.
  3. There were a couple of crashes when navigating.

So here is my honest opinion on the foreground services.

I have got quite a bit of feedback from the users so far. Their complain about the Casio app is:

I understand some apps need FS, but actions are secondary features of this, and I rather not bear the cost. I use the Photo action quite a bit, but it is not my default action, so before I use it, I start the app manually, and set it up. Some other actions require the app to be running. For that, people use Tasker or MacroDroid to keep the app running, or start it at certain time when the Auto Time Adjustment takes place. These apps are designed for that purpose and we can never duplicate their functionality. (Having said that, on my humble Moto G Power phone with 3G RAM, the app runs in the background for day or two)

As an aside, you are obviously a very skilled Android developer. I hope you don't get discouraged and stop contributing. I have quite a bit of requests to support the GDB-200 watches, which have fitness functions. The casio Move app sucks as well, so I like to add support for these watches and integrate them with Google Fit. So this could be a good project we can collaborate on.

izivkov commented 1 year ago

I have commuted some changes to main. They include the ability to start your FindPhone activity by long pressing the Lower-RIGHT button. (Take a look at ActionRunnerLayout, line 47). I think for now, we should remove the activity from the Actions screen, and use it just by long-pressing the lower-right button.

We can re-evaluate FS in the future.

germede commented 1 year ago

Alright! thank you for the extensive feedback and the API integration of the dedicated button. I'll take a look at the main changes as well.

izivkov commented 1 year ago

I'm also thinking if there is another way to stop the ringing, instead of a Notification or a Dialog box. Maybe some gesture, like picking up the phone (https://stackoverflow.com/questions/24138678/android-detect-phone-lifting-action), something that is supported by most phones - just thinking aloud.

izivkov commented 1 year ago

@gmedina-de I like to incorporate some of your changes, like the PhoneFinder class, as well as changes to navigation you implemented in the MainActivity, into the main code. Do you have some time to work on this, or should I do it? After that, I like to experiment with Phone Lifting gesture to stop ringing. Please let me know if you like to work on these.

germede commented 1 year ago

@izivkov Unfortunately I don't have much time left due to work / studies but alternatively you may cherry pick the commits you desire from https://github.com/izivkov/CasioGShockSmartSync/compare/main...gmedina-de:CasioGShockSmartSync:main ?

izivkov commented 1 year ago

For sure. Anytime in the future if you like to contribute, let me know.

On Thu., Jul. 13, 2023, 11:33 a.m. G. Medina, @.***> wrote:

@izivkov https://github.com/izivkov Unfortunately I don't have much time left due to work / studies but alternatively you may cherry pick the commits you desire from main...gmedina-de:CasioGShockSmartSync:main https://github.com/izivkov/CasioGShockSmartSync/compare/main...gmedina-de:CasioGShockSmartSync:main ?

— Reply to this email directly, view it on GitHub https://github.com/izivkov/CasioGShockSmartSync/issues/42#issuecomment-1634458725, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7M37QMXV4PQY5WQLXL3XTXQAIOFANCNFSM6AAAAAA2GRWJAQ . You are receiving this because you were mentioned.Message ID: @.***>

izivkov commented 1 year ago

Closing this issue. I added a "Find Phone" functionality when the user long presses the LOWER-RIGHT button. The Phone will ring until the user picks up the phone, or 30 seconds have passed.