Skip to content

[WIP] Allow updating location stream settings #1664

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.

Sign up for GitHub

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

Jump to bottom
Open
orkun1675 wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[WIP] Allow updating location stream settings #1664

orkun1675 wants to merge 11 commits into from

Conversation

@orkun1675
Copy link

@orkun1675 orkun1675 commented Mar 21, 2025

This PR allows updating the location stream after one is started. It adds a new API endpoint:

  /// Updates the parameters of the active position stream.
  ///
  /// Throws a [NotSubscribedException] when there is no active position stream
  /// to update.
  static Future<void> updatePositionStream({
    required LocationSettings locationSettings,
  }) =>
      GeolocatorPlatform.instance.updatePositionStream(
        locationSettings: locationSettings,
      );

This endpoint allows plugin users to change settings, such as distanceFilter or intervalDuration, without having to stop & start the stream. This is critical if the update is hapenning while the app is in the background since Android does not allow starting a location stream unless the app is in focus.

Fixes #1569. Improved version of #1406.

If there is interest in merging this PR I will clean it up and send for review.

Pre-launch Checklist

  • I made sure the project builds.
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is does not need version changes.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I rebased onto main.
  • I added new tests to check the change I am making, or this PR does not need tests.
  • I made sure all existing and new tests are passing.
  • I ran dart format . and committed any changes.
  • I ran flutter analyze and fixed any errors.

@orkun1675
Copy link
Author

orkun1675 commented Mar 21, 2025

@TimHoogstrate PLMK if there is interest in merging this PR and I can clean it up.

@TimHoogstrate
Copy link
Contributor

TimHoogstrate commented Mar 31, 2025

Dear @orkun1675,

Thanks for your PR. We are looking forward to review this. Looks like an interesting addition. I'll do a quick review, try to do a little test and you can clean it up if possible.

Kind regard,

TimHoogstrate
TimHoogstrate reviewed Mar 31, 2025
View reviewed changes
...ator_android/android/src/main/java/com/baseflow/geolocator/location/FusedLocationClient.java
}

@Override
public void updateLocationOptions(LocationOptions options) {
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

We should add a @nonnull or @nullable annotation here.

...ator_android/android/src/main/java/com/baseflow/geolocator/location/FusedLocationClient.java
@Override
public void updateLocationOptions(LocationOptions options) {
this.locationOptions = options;
if (running) {
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

I think it is a good idea to notice the developer that the location updates could not be made. Due to .... Or skip keeping track if it is running at all and let the developer manage this. It will prevent errors.

...cator_android/android/src/main/java/com/baseflow/geolocator/location/GeolocationManager.java
locationClient.startPositionUpdates(activity, positionChangedCallback, errorCallback);
}

public boolean updateLocationOptions(LocationOptions options) {
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

@Nullable LocationOptions options

...cator_android/android/src/main/java/com/baseflow/geolocator/location/GeolocationManager.java
private static final String TAG = "GeolocationManager";

private static GeolocationManager geolocationManagerInstance = null;
private static GeolocationManager geolocationManagerInstance = null;
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

@nullable annotation

geolocator_apple/darwin/geolocator_apple/Sources/geolocator_apple/Handlers/GeolocationHandler.m
CLLocationManager *locationManager = [self getLocationManager];
locationManager.desiredAccuracy = desiredAccuracy;
locationManager.distanceFilter = distanceFilter;
if (@available(iOS 6.0, macOS 10.15, *)) {
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

Do we need that @available(iOS 6.0 ?

geolocator_apple/darwin/geolocator_apple/Sources/geolocator_apple/Handlers/GeolocationHandler.m
}

#if TARGET_OS_IOS
if (@available(iOS 9.0, macOS 11.0, *)) {
Copy link
Contributor

@TimHoogstrate TimHoogstrate Mar 31, 2025

Choose a reason for hiding this comment

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

same here, do we need the @available(iOS 9.0, macOS 11.0, *)

@TimHoogstrate
Copy link
Contributor

TimHoogstrate commented Mar 31, 2025

I've tested this on iOS and Android (Pixel 7) en it seems to work as expected.

@TimHoogstrate TimHoogstrate self-assigned this Mar 31, 2025
@TimHoogstrate
Copy link
Contributor

TimHoogstrate commented Mar 31, 2025

The idea is to split up the PR in desperate PR's. Separate packages, platform interface and app facing package. If you need any help from us let us know.

Kind regards,

@orkun1675 orkun1675 mentioned this pull request Nov 22, 2025
Open
10 tasks
@orkun1675
Copy link
Author

orkun1675 commented Nov 22, 2025

Thank you for the review. I'll address the native plugin comments after #1760 lands.

@orkun1675 orkun1675 changed the title Allow updating location stream settings [WIP] Allow updating location stream settings Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimHoogstrate TimHoogstrate TimHoogstrate left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@TimHoogstrate TimHoogstrate

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Question]: Updating accuracy/distanceFilter while the Position stream is running

2 participants

@orkun1675 @TimHoogstrate