OpenTracksApp / OpenTracks

OpenTracks is a sport tracking application that completely respects your privacy.
https://OpenTracksApp.com
Apache License 2.0
1.01k stars 186 forks source link

[Xiaomi] While recording, telephone vibrates repeatedly #611

Closed dennisguse closed 3 years ago

dennisguse commented 3 years ago

Describe the bug While recording, Xiaomi devices vibrate repeatedly. See https://github.com/OpenTracksApp/OpenTracks/discussions/610

To Reproduce

  1. Go to outdoors, so GPS is available
  2. Click on 'start recording'

Vibration only happens outdoors (with GPS reception); indoors no data is recorded and no vibration takes place.

Logcat: https://github.com/OpenTracksApp/OpenTracks/files/6032139/filtered_log.txt

Technical information

dennisguse commented 3 years ago

I just checked the logcat (thanks @StefanKley) and it looks good, right. At least nothing crashes - that's good news.

Some technical details: for recording OpenTracks starts a so-called foreground service and this service must show a notification, so the user is aware something is running. And for convenience, we update this notification showing the GPS accuracy every time we get a GPS location.

I guess, that the vibration is triggered, when we update the notification. Some more info here: https://stackoverflow.com/questions/48261118/cannot-disable-notification-vibration-in-android-8/52207215#52207215

If this is right, then the issue is in here: https://github.com/OpenTracksApp/OpenTracks/blob/22a1dfd354e617076c508b66419c054090bf2eda/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java

Likely it is this line: https://github.com/OpenTracksApp/OpenTracks/blob/22a1dfd354e617076c508b66419c054090bf2eda/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java#L36

This might fix the issue (using low importance):

            NotificationChannel notificationChannel = new NotificationChannel(CHANNEL_ID, context.getString(R.string.app_name), NotificationManager.IMPORTANCE_LOW);

Honestly, I don't know why we used high importance here. I mean, the GPS accuracy is not the most interesting thing at all.

@StefanKley @andrearicci Can you compile yourself?

andrearicci commented 3 years ago

sorry, that's over my skill

StefanKley commented 3 years ago

I'm interested in trying to compile but would start from scratch so this endeavor might take a while....and fail ;-) Thanks for your update!!

Further advice towards compiling would be appreciated...

dennisguse commented 3 years ago

Here is a build with IMPORTANCE_NONE. Needs to be unzipped (Github does not allow to share APKs).

OpenTracksUpstream-releasePlayStore.apk.zip

StefanKley commented 3 years ago

No success I'm afraid. I just checked version 3.15.0-3-g4e4a9abf6-4e4a9abf6- (Code 3855) The phone still vibrates wildly as soon as GPS data is being received.

Do you need another logcat?

StefanKley commented 3 years ago

I created another logcat that contains more context info in the time frame 02-24 20:36:5 to eof* where the GPS data was received and the vibrating started. Tracking started apr. 02-24 20:36:43 custom_log2.txt

dennisguse commented 3 years ago

My fault. We have two priorities: one for the notification channel (Android O/8+) and one for the notification. In the previous build I only downgraded the priority of the notification channel. And that does not have an effect on your devices...

Low priority for notification channel and notification: OpenTracksUpstream-releasePlayStore-min-priority.apk.zip

Without updating the notification: OpenTracksUpstream-releasePlayStore-no-update-notification.apk.zip

    TrackRecordingServiceNotificationManager(Context context) {
        notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
            NotificationChannel notificationChannel = new NotificationChannel(CHANNEL_ID, context.getString(R.string.app_name), NotificationManager.IMPORTANCE_NONE);
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
                notificationChannel.setAllowBubbles(true);
            }

            notificationManager.createNotificationChannel(notificationChannel);
        }

        notificationBuilder = new NotificationCompat.Builder(context, CHANNEL_ID);
        notificationBuilder
                .setDefaults(NotificationCompat.DEFAULT_ALL)
                .setPriority(NotificationCompat.PRIORITY_MIN)
                .setOnlyAlertOnce(true)
                .setOngoing(true)
                .setCategory(NotificationCompat.CATEGORY_SERVICE)
                .setContentTitle(context.getString(R.string.app_name))
                .setSmallIcon(R.drawable.ic_logo_color_24dp);
    }
    private void updateNotification() {
//        notificationManager.notify(NOTIFICATION_ID, getNotification());
    }
StefanKley commented 3 years ago

I tried "IMPORTANCE_NONE" and "PRIORITY_MIN" with my own build without success (same with your build OpenTracksUpstream-releasePlayStore-min-priority.apk.zip).

Removing the "notificationManager.notify" worked with "OpenTracksUpstream-releasePlayStore-no-update-notification.apk.zip" and my own build. (I had to double check my own builds ...didn't trust them ;-D)

So removing the notification update works! (But whats the downside of removing it?) Thank you! :-)

dennisguse commented 3 years ago

@StefanKley So, now we know that it is the notification! That's real progress :)

And it is really cool that you can test it yourself!

Next thing on my list is this:

boolean currentLocationWasAccurate = trackPoint.getAccuracy() < recordingGpsAccuracy;
boolean shouldAlert = !currentLocationWasAccurate && previousLocationWasAccurate;
notificationBuilder.setOnlyAlertOnce(!shouldAlert);

Here, we are checking if the accuracy of the GPS is not good enough and if not, we send a vibrate (I hope) by modifying setOnlyAlertOnce(). Can you remove this (incl. line 80: notificationBuilder.setOnlyAlertOnce(true);)? Two things might go wrong:

  1. the accuracy your phones report is way too large (or not useful at all)
  2. somehow this Android variant does not like calls to setOnlyAertOnce().

At the moment, removing the notification for now is not an issue - it only shows the GPS accuracy. But there are plans to also show the speed, distance etc - especially cool, if you have a smartwatch that shows such info.

StefanKley commented 3 years ago

I will, but that's a thing for tomorrow ;-) I'll keep you updated...

StefanKley commented 3 years ago

(obviously I doing something wrong with the code escapes!?? :-/ ) Not sure if I got this right. I did a rollback on the file (or should I have kept the first changes?)

NotificationChannel notificationChannel = new NotificationChannel(CHANNEL_ID, context.getString(R.string.app_name), NotificationManager.IMPORTANCE_HIGH); and .setPriority(NotificationCompat.PRIORITY_MAX)

and set this (line 66 and following), right? ` void updateTrackPoint(Context context, TrackPoint trackPoint, int recordingGpsAccuracy) { String formattedAccuracy = context.getString(R.string.value_none); if (trackPoint.hasAccuracy()) { formattedAccuracy = StringUtils.formatDistance(context, trackPoint.getAccuracy(), PreferencesUtils.isMetricUnits(context));

        boolean currentLocationWasAccurate = trackPoint.getAccuracy() < recordingGpsAccuracy;
        boolean shouldAlert = !currentLocationWasAccurate && previousLocationWasAccurate;
        //notificationBuilder.setOnlyAlertOnce(!shouldAlert);
        previousLocationWasAccurate = currentLocationWasAccurate;
    }

    notificationBuilder.setContentText(context.getString(R.string.track_recording_notification_accuracy, formattedAccuracy));
    updateNotification();

    //notificationBuilder.setOnlyAlertOnce(true);
}`

This at least is not solving the issue... (Pretty busy today so I cannot work on this as much as I want, will try to check back late today...)

dennisguse commented 3 years ago

Can you try this? Removing accuracy alert:

    void updateTrackPoint(Context context, TrackPoint trackPoint, int recordingGpsAccuracy) {
        String formattedAccuracy = context.getString(R.string.value_none);

        notificationBuilder.setContentText(context.getString(R.string.track_recording_notification_accuracy, formattedAccuracy));
        updateNotification();
    }

and if this does not work: downgrade the priority as here https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-785386778

@StefanKley No hurry, I am glad that you can actually test this yourself. Otherwise, it would be tricky.

dennisguse commented 3 years ago

@StefanKley What you can also try is this:

        notificationBuilder
                .setDefaults(NotificationCompat.DEFAULT_ALL)

to

        notificationBuilder
                .setDefaults(NotificationCompat.DEFAULT_LIGHTS)
StefanKley commented 3 years ago

First take https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-786089306 works! Second take https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-786093003 works too :-)

Each change build on top of base code with modification https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-785386778

Unrelated to both builds I did notice that the displayed distances/max. speeds are pretty wild - without moving the phone (indoors). Maybe the phone does deliver "unusual data acc"?! Screenshot_2021-02-25-23-25-15-379_de dennisguse opentracks debug

dennisguse commented 3 years ago

Thanks for trying investing the time. Then one final check: can you go to the settings and increase the "GPS accuracy"? Default is 50m and everything above is considered inaccurate; and that triggers the vibration. And it would also be great to get some info on what accuracy your phone actually has: can you check what accuracy is shown in the notification (that leads to the vibration)?

Implementation option: What about adding an option into the settings if the phone should vibrate or not on inaccurate GPS measurements?

About the speed question: yes, that is normal. The initial GPS fix is not really precise. And the speed estimation is even worse. So, it usually takes some seconds until the GPS fix is okay and then the speed will be "okay" as well; until then the speed has quite some variance. OpenTracks does some post processing on speed values as back in 2010 GPS was way worse. What you can do: start the GPS before actually starting the recording. This can be done from the main screen's menu. Or for cycling: use a BLE sensor.

@StefanKley Sorry, for the late response - did not saw the message.

andrearicci commented 3 years ago

Hello, today I set the tolerance to 200m and the problem was still there.

dennisguse commented 3 years ago

@andrearicci Thanks; can you check what the message in the notification is? is it something like "waiting for signal" or does it a show an accuracy in m?

andrearicci commented 3 years ago

@dennisguse where shall I see that? Is it an information on the recorded trak or only while recording?

dennisguse commented 3 years ago

@andrearicci When you start a recording, a notification is shown and stays present until you stop the recording. I it as provided by Android and shown in the notification drawer (like SMS or Email notifications). Here: https://developer.android.com/images/ui/notifications/notification-drawer_2x.png

andrearicci commented 3 years ago

@dennisguse ok, tomorrow I'll check it and let you know

StefanKley commented 3 years ago

Hi @dennisguse, I checked the increased tolerance and with 200m the problem persists here too.

The notification started after a few seconds with a value below 50m and narrowed down pretty fast well below 10m. I did multiple checks with accuracy set to either 50 or 200 without difference in behavior. Vibration continues anyways...

Thanks for your explanation on the speed. Also I agree that an option to choose weather (no) vibration is wanted during GPS fix would be helpful!

andrearicci commented 3 years ago

@andrearicci When you start a recording, a notification is shown and stays present until you stop the recording. I it as provided by Android and shown in the notification drawer (like SMS or Email notifications). Here: https://developer.android.com/images/ui/notifications/notification-drawer_2x.png

Hi, today I set the tolerance back to 50m, and the notification says "incertezza GPS: 16.69m" (italian for appoximation/uncertainty). Edit: it may have been vibrating a little less. To be honest I check it touching the pocket when stopping at the crossing lights, and some times it was vibrating and other don't. Can't be more specific.

dennisguse commented 3 years ago

@andrearicci @StefanKley Ok, then one last (I hope and I hope I got it right): your XIaomi phones also vibrate if the GPS accuracy is below the threshold in the settings?

If yes, then we can add a settings to disable this functionality in OpenTracks using this approach: https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-786093003

Do you any options to modify how your phones handle notifications?

StefanKley commented 3 years ago

Yes even with acc below threshold it does vibrate. Seems like Xiaomi is misinterpreting the notification updates by opentracks?!

Indeed there is an app specific setting for notifications and vibration is enabled by default. I disabled it and enabled the "light" and in a second test "audio" option - both without any effect. I expected the notification light or the audio to cause similar type of trouble like the vibration does but nothing happens. Re-enabling vibration and the trouble started right away. Seems like that app related notification setting has trouble handling the updates by opentracks.

(Anyway I think adding an option to disable vibration in opentracks would be a useful addition.) I will have a look at https://github.com/OpenTracksApp/OpenTracks/issues/611#issuecomment-786093003 again later this week... Thanks for sticking with us @dennisguse :-)

dennisguse commented 3 years ago

@StefanKley Any news on this one?

andrearicci commented 3 years ago

Hi, in v.3.18.4 4151 the effect was still there. Now my xiaomi is dead an I got a new phone so I can't check it any more.

StefanKley commented 3 years ago

Hi Dennis,

sorry I lost track of this since my build worked and I could use it without troubles... I checked the currently available version "v3.18.5-823dc1c12- Code 4176" (f-Droid app store) and the problem persists.

I also did a build of the current code "v3.18.5". (Without any changes the ongoing vibration kicks in as expected.) When changing "DEFAULT_ALL" to "DEFAULT_LIGHTS" as described in #611 (comment) this solves the issue for me!

Also I could use the workaround to disable vibration for this app in android settings on my xiaomi phone. As stated before it might be an option to add a setting to OpenTracks that will allow users to disable vibration.

Thanks for checking back at this.

real-iceman commented 1 year ago

@dennisguse Thanks for your great work! As a newbie user of OpenTracks (v4.2.0) I still have this problem. Is it possible to add your fix to the next version?

dennisguse commented 1 year ago

@real-iceman Which device?

real-iceman commented 1 year ago

Sorry, it's a Xiaomi Smart Band 7, Firmware 1.27.0.4.

dennisguse commented 1 year ago

And you paired it with your phone? The vibration might be caused the notification that OpenTracks shows and updates. Some smart watches vibrate whenever a new notification is created OR an existing one is updated.

How did you connect the smart watch with your phone? Gadgetbridge?

real-iceman commented 1 year ago

Yes, it is paired with my phone via Gadgetbridge. Was out for another test walk and I can confirm that the device is flooded with notifications from OpenTrack, which makes it completely unsusable.

It turned out that there is a plenty of options in Gadgetbridge to control the notifications. Blacklisting notifications from OpenTrack solves my problem.

I still wonder why notifications from some other background apps are not sent to the device. E.g., DataMonitor seems to send its notifications with PRIORITY_LOW and constantly updates the current transfer rate. But even if I switch off the suppression of low priority messages in Gadgetbridge, no updates are sent to the device. I assume this is a Gadgetbridge issue.

Thank you very much for your help and sorry for bothering you!

dennisguse commented 1 year ago

Cool!

PS/ reducing the priority of the notification is an option; I have to check why we use the one we use.