Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPSLogger doesn't notify the user when it can't write to files, leading to data loss #1138

Open
JacksonChen666 opened this issue May 12, 2024 · 8 comments
Milestone

Comments

@JacksonChen666
Copy link

Sometimes the permission on my phone gets messed up (no idea why or how, just that it does) and I'll have to re-grant some permissions to apps. I often forget to re-grant the permissions for GPSLogger, since it doesn't tell me when it can't write to a file (when starting a new log), so all logs that are recorded while the permissions are broken just don't exist at all.

I've had this issue happen twice, and it took me at least a month to notice the issues in the first place.

Possibly related to #1053

(/e/OS 1.21.1, GPSLogger v130, FP4, not related to battery optimizations since it's about storage permissions and everything's fine when that's fixed)

mendhak added a commit that referenced this issue May 26, 2024
mendhak added a commit that referenced this issue May 26, 2024
… service to only after the permission check is complete.

In the stop service, only stop the service if notifications are enabled, due to a weird scenario where the service might start and immediately end due to lack of notifications, resulting in an app crash.
How did we end up like this.
Issue #1138
Issue #1053
@mendhak mendhak added this to the v131 milestone May 26, 2024
@mendhak
Copy link
Owner

mendhak commented May 26, 2024

I'm currently working on this, at least partially.

I'm able to create a channel for errors.

I'm able to send an error notification when all permissions have been denied.

I'm going to start showing error notifications when file writes fail due to IOException or SecurityException. However, I'm going to try making it a single notification so it doesn't become overwhelming. It could be that each file writer fails and results in a notification but each one overwrites the other... which is a bit weird. Need to think of a way to make this efficient maybe.

Here's what the notification looks like:

image

I've chosen Visibility Public and Priority High. I'm assuming a user would want to know that their writes or the app is failing?

I can't do anything if the user denies notification permissions!

I ran into a really nasty situation where notification permissions were denied while everything else was accepted - I think it's basically this complex StackOverflow thread. The app would think it had all permissions and start the service, but because notifications were denied, it would immediately stop the service, and because the foreground service started but didn't show a notification within 5 seconds, it crashes the app, and starts a crash-restart-crash loop. I've modified the stop service code so that if notification permissions are denied it doesn't stop the app. Weird hacky workaround that's going to make zero sense in 1 month from now 😢

@mendhak
Copy link
Owner

mendhak commented May 26, 2024

When location or all permissions are denied (except notifications!)

image

@JacksonChen666
Copy link
Author

When location or all permissions are denied (except notifications!)

image

nearly invisible warning icon 👀

@mendhak
Copy link
Owner

mendhak commented May 26, 2024

So invisible I forgot it was there haha.

I was using the built-in android.R.drawable.stat_notify_error I wonder if there's something else I could do.

@JacksonChen666
Copy link
Author

I'm able to create a channel for errors.

I'm able to send an error notification when all permissions have been denied.

I was actually thinking about like a pop-up prompt in-app when you open the app or start logging, since that's usually how I start the GPS logging

Notification still makes sense for when "Start on bootup" is enabled

@mendhak
Copy link
Owner

mendhak commented May 26, 2024

There should be one already when you start logging, if the file write permission is denied, then a popup appears like this

image

And if all permissions denied then that "first time permissions" apologetic prompt should appear already right when the app is launched. Are you not seeing that?

I didn't realize you meant in-app. I'm wondering I should probably continue anyway with the notifications since it could cover both use cases, in-app and background service.

@JacksonChen666
Copy link
Author

JacksonChen666 commented May 26, 2024 via email

mendhak added a commit that referenced this issue May 27, 2024
…nyway, except for NMEA, which stops logging. This is because NMEA has a high frequency of writes.

Issue #1138 #1053
@mendhak
Copy link
Owner

mendhak commented May 27, 2024

If anyone wants/is able to test, an APK is here:

https://github.com/mendhak/gpslogger/releases/tag/v131-rc2

Note that if you use the FDroid version of this app, installing this is like new, so you'll lose previous data and settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants