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

Fix location provider #912

Merged
merged 3 commits into from
Jan 13, 2024
Merged

Fix location provider #912

merged 3 commits into from
Jan 13, 2024

Conversation

Altonss
Copy link
Collaborator

@Altonss Altonss commented Jan 11, 2024

This is a quick fix that should close #911.
@Kelvino9 @licaon-kter could you try it out?

@cla-bot cla-bot bot added the cla-signed ✔️ The Contributor Licence Agreement was signed by all contributors. label Jan 11, 2024
@Altonss Altonss added bug 🐞 A functional defect or unexpected behavior. priority 🚨️ This issue will be most probably addressed before the next release. labels Jan 11, 2024
@licaon-kter
Copy link

licaon-kter commented Jan 11, 2024

rebuilt and confirm that it fixes the issue, no crash

APK for testing: de.grobox.liberario_124.apk.ZIP (remove .ZIP from name)

@Altonss Altonss requested review from grote and ialokim January 11, 2024 14:27
@Kelvino9
Copy link

Kelvino9 commented Jan 11, 2024

Thank you @Altonss for the fix and @licaon-kter for providing an APK.

Transportr does not crash upon start with this build, but also no longer is able to get a fix on my positon like in previous releases. Is there no way to get that function back?

I do not depend on this function as I can use any navigation app to get more information on my surroundings on an unknown area, but it would be a nice to have feature since it was successfully included before.

Thanks anyway for the immediate bug fix.

@licaon-kter
Copy link

@Kelvino9 so you say that if you lock GPS location in GPSTest app, and then you open this app, it won't get your location?

Works fine for me here.

@Kelvino9
Copy link

Kelvino9 commented Jan 11, 2024

@licaon-kter I deleted everything, rebooted and tested again. It seems to work as intended after waiting for a few moments.

I did install GPSTest this time, because you mentioned it above. It worked. I then uninstalled GPSTest and reinitialized Transportr and it again worked as intended. I hope there isn't any data left from GPSTest and Transportr will continue to work without the need of running GPSTest before.

Was I just impatient when trying to get a fix on my position in Trasnportr? I didn't got a warning from Transportr earlier (before I used GPSTest) that it couldn't retrieve my position yet, but it notified me in later tests which actually led me to wait a bit.

@licaon-kter
Copy link

GPS takes time to get a lock, I guess this app needs a bigger icon for "looking for location" :)

Copy link
Collaborator

@ialokim ialokim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Altonss for the quick investigation and fix! I've looked into the PositionController again and also found the for-loop in onInactive() to be useless. Maybe you can remove that too in this PR.

@Altonss Altonss requested a review from ialokim January 12, 2024 15:50
@Altonss
Copy link
Collaborator Author

Altonss commented Jan 12, 2024

Just applied the changes you suggested @ialokim, and everything works fine for me.
Only little issue I have, but idk if it's supposed to be normal, is that Transportr never asks me again about the location permission once I refuse (even if I click on the grey locate button).

@Kelvino9
Copy link

In Organics Maps you'll be notified to change location permission manually once you denied location permission.

@ialokim
Copy link
Collaborator

ialokim commented Jan 13, 2024

Only little issue I have, but idk if it's supposed to be normal, is that Transportr never asks me again about the location permission once I refuse (even if I click on the grey locate button).

Good remark, let's follow up on this in #916 .

Copy link
Collaborator

@ialokim ialokim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ialokim ialokim merged commit 29821e4 into grote:master Jan 13, 2024
4 checks passed
@Altonss Altonss deleted the gps branch November 4, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 A functional defect or unexpected behavior. cla-signed ✔️ The Contributor Licence Agreement was signed by all contributors. priority 🚨️ This issue will be most probably addressed before the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transportr 2.2.0 crashes after initial setup.
4 participants