Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Geolocation foreground listener #1579

Closed
wants to merge 7 commits into from
Closed

Geolocation foreground listener #1579

wants to merge 7 commits into from

Conversation

vividos
Copy link
Contributor

@vividos vividos commented Dec 9, 2020

Description of Change

This is an initial implementation of a foreground-only listener for Location changes. It consists of initial work from #1043 (done by Alex Reich), the API proposed in #1447 (Spec by me), and code from James Montemagno's Geolocator-Plugin.

Bugs Fixed

API Changes

See Spec in #1447

Behavioral Changes

None.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

Tests not added yet, as well as mdoc docs.

@vividos vividos marked this pull request as draft December 9, 2020 20:06
@vividos
Copy link
Contributor Author

vividos commented Dec 9, 2020

@mattleibow @jamesmontemagno @Redth Any comments about the current state of the PR are very welcome! Will do the UWP implementation next. Help for WPF and Tizen platforms are welcome!

@vividos vividos marked this pull request as ready for review December 11, 2020 17:27
@mattleibow
Copy link
Contributor

This is looking really nice. I'll have a closer look, but so far it all makes sense.

Thanks for this PR!

@vividos
Copy link
Contributor Author

vividos commented Dec 12, 2020 •
edited
Loading

Thanks for the feedback! One thing that occurred to me was that the code is using GeolocationRequest.Timeout as the minimum time value, which is a bit misleading. Also don't know if StopListeningForegroundAsync needs a return type at all.

@vividos
Copy link
Contributor Author

vividos commented Jan 22, 2021

I rebased my PR on top of current main, in order to keep the PR current and to check if it still builds (which I guess).

@mattleibow mattleibow added this to the 1.7.0 milestone Jan 22, 2021
@mattleibow mattleibow mentioned this pull request Jan 22, 2021
Closed
3 tasks
@jamesmontemagno jamesmontemagno added awaiting-review This PR needs to have a set of eyes on it needs-more-discussion This needs more discussion or info before discussing it as a proposal. labels Apr 13, 2021
@vividos
Copy link
Contributor Author

vividos commented Apr 20, 2021

@jamesmontemagno I saw you're added the needs-more-discussion tag. Let's discuss! I can't wait to get this PR into Xamarin.Essentials! :-D

vividos added a commit to vividos/WhereToFly that referenced this pull request Apr 21, 2021
@vividos
added Geolocation class extensions
2e95326 
API from my Xamarin.Essentials PR:
xamarin/Essentials#1579
also added implementation based on the Xam.Plugin.Geolocator plugin
@vividos
Copy link
Contributor Author

vividos commented Apr 21, 2021

One thing that I think doesn't match exactly is the re-use of the GeolocationRequest class to pass options to StartListeningForegroundAsync(); here, I'm reusing request.Timeout for the minimum time. I could add a new property ListeningMinimumTime to the request object to clarify that it's for the listening part of Geolocation. What do you think?

@vividos
Copy link
Contributor Author

vividos commented Apr 21, 2021

I also just rebased the PR on top of current main, just to be current.

@janusw
Copy link
Contributor

janusw commented Apr 28, 2021

I can't wait to get this PR into Xamarin.Essentials! :-D

Me too :D

+1 for merging this! 👍

@jamesmontemagno jamesmontemagno modified the milestones: 1.7.0, .NET MAUI Jun 9, 2021
@alexreich @vividos
Geolocation - Location Updates #290 - added ContinuousLocationListene…
80cccea 
…r ported from Geolocator.Plugin.GeolocationContinuousListener

(cherry picked from commit 7c043df)
@vividos vividos mentioned this pull request Jul 14, 2021
Closed
@vividos
Copy link
Contributor Author

vividos commented Jul 14, 2021

I rebased the commits onto the latest main branch (that's also tagged with 1.7.0) squashed together some commits that fixed bug in the previous commits and added the new ListeningRequest class.

@vividos vividos mentioned this pull request Dec 6, 2021
Closed
@vividos
Copy link
Contributor Author

vividos commented Dec 28, 2021

It seems there won't be any development in Essentials anymore - closing this PR.

@vividos vividos closed this Dec 28, 2021
@vividos vividos deleted the geolocation-foreground-listener branch December 28, 2021 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
awaiting-review This PR needs to have a set of eyes on it needs-more-discussion This needs more discussion or info before discussing it as a proposal.
Projects
None yet
Milestone
.NET MAUI
Development

Successfully merging this pull request may close these issues.

[Spec] Listening for location changes in the foreground Geolocation - Location Updates
5 participants
@vividos @mattleibow @janusw @jamesmontemagno @alexreich