maui
maui copied to clipboard
dotnet
[Essentials] Geolocation foreground listening
Description of Change
This PR implements Geolocation foreground listening, Spec was suggested in #3678, and original Xamarin.Essentials issues are https://github.com/xamarin/Essentials/issues/1447 and https://github.com/xamarin/Essentials/issues/290
As I never got feedback from the Spec issue tickets, I'm creating a PR with my changes sitting on my HD for too long. I'm happy to discuss and change any part of the code, refactor, etc.
I'm targeting main branch, which I guess goes into .NET 7, which is fine for me. Just tell me when I should switch base.
Issues Fixed
Fixes #3678
Hey there @vividos! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hmm how do I get rid of the RS0016 errors? Visual Studio's quick action menu added the signatures to the PublicAPI.Unshipped.txt file, but it doesn't help...
Fantastic, this is the last missing functionality of https://github.com/jamesmontemagno/GeolocatorPlugin that is not in Maui.Essentials.
I did a minor refactoring of duplicated code, but otherwise this PR is ready to review.
@mattleibow @rmarinho @Eilon What are the next steps for this PR? Would you like to discuss the public API, or review the code? What about documentation? Ask someone for the Tizen implementation?
@jfversluis @davidortinau I have an open feature here that basically waits to be discussed, reviewed and finally committed since about two years, first in the Xamarin Essentials repository, and now here for MAUI essentials. Any word on how to proceed with the feature would be great from you, even a "no we don't do that". The feature is (for two of my projects, one commerical), and surely for other people, the last thing that remains from James Montemagno's plugin projects to be moved to Essentials. Every other plugin has now a more modern counterpart, being Essentials, Shiny or other well maintained community projects. So please say anything, thanks!
Still waiting for any comment from MAUI folks, see my comment from Oct 1....
Any progress on this? So long that this wasn't even inserted into Xamarin Essentials, and now it won't even be inserted into Maui?
Hey @vividos somehow this one fell through the cracks at least for me, sorry about that. It has my attention now!
Are you still up for bringing this home? Then I will make sure this gets the attention it deserves.
If yes, could you please ~rebase/update this PR so the conflicts are gone and get this building again~ (did that for you)? If no, also good, please let me know then I will see if I can take over.
Feel free to reach out with the contact details you already have or find my email address on my GitHub profile for a more direct connection. Thanks!
@jfversluis definitely! First step would be to check the API, which is provided in #3678. If that is ok, there would be some changes necessary on this PR, as interfaces were introduced some time ago, and I see that XML comments can be in code now. I can work on that from 22nd Jan on. More important is if the API is in tune with what MAUI Essentials wants to use. Thanks Gerald for picking this up!
@vividos added some notes about the API design in that linked issue. Nothing too shocking. Let's get those details out of the way and then let's add that to here and get things going.
Thanks Gerald, I made a comment on the Spec issue #3678. I can work on the Spec and code from Jan 22nd on. If anyone wants to jump in and continue working on the PR, feel free!
Thanks Gerald, I'll do the changes, no problem. I love those code docs and always wondered why Essentials didn't have them or moved them out in some XML files.
Another thing that is currently different from James' plugin is the error handler behavior: When UWP signals a LocationError, it also calls StopListeningForeground, but the other platforms do not. I honestly don't know why I omitted this and will add it, too.
always wondered why Essentials didn't have them or moved them out in some XML files.
Loooong story there. Same for Forms and now .NET MAUI. We're working on fixing that and actually whole of Essentials should be better by now and have most of it and I saw you added a lot as well so thank you for that. If you're looking for more (easier) contributions in the future, adding/improving that docs stuff is certainly useful.
Anyway, yeah, work through this feedback, add what you found was missing and we'll go from there. Thanks!
@jfversluis I think I addressed all your review comments, except the Tizen implementation. Just add more if you want me to do some more changes. I also took the opportunity to reduce code duplication, see the last commit.
I also have some #nullable enable changes for Geolocation on my HD. Should these go into a follow-up PR?
And one more question: Is the main branch the correct one? And do you want me to force-push the changes to the latest commit?
Thanks for reviewing this!