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

Only the first trackable added to the publisher emits a didChangeConnectionState:forTrackable: callback #425

Open
lawrence-forooghian opened this issue Oct 17, 2022 · 6 comments · May be fixed by #481
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Oct 17, 2022

Description

When the first trackable is added to a publisher, it emits a delegate didChangeConnectionState: .online, forTrackable: call. It does not do this for subsequent trackables added to the publisher (or sometimes, it does, but after a long delay).

Here's an example of how this looks in the publisher example app on my device (after the UI changes of #419):

RPReplay_Final1666015659.mov

Conditions to reproduce

I noticed this issue on my device running iOS 16.0 and on the iOS 15.5 simulator. It does not happen on the iOS 16.0 simulator.

I think this was just because my iOS 15.5 simulator was simulating a static location and my iOS 16.0 simulator was simulating a drive. Seems like you can actually reproduce this with any simulator that's simulating a static location, or with a stationary device.

@lawrence-forooghian lawrence-forooghian added the bug Something isn't working. It's clear that this does need to be fixed. label Oct 17, 2022
@sync-by-unito
Copy link

sync-by-unito bot commented Oct 17, 2022

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-2729

@ikbalkaya
Copy link
Contributor

After some investigation, this seems to be a result of delegate methods being called as a result of publish in sendEnhancedLocation. There is also a callback set where the subscription is set for subscribeForChannelStateChange which is never called. I think that must be the right place to invoke this particular delegate method which I will investigate why it's not the case

@ikbalkaya
Copy link
Contributor

I checked Android codebase and there too statuses are updated as result of enhanced location publishes too. So as a result I do not really think that's a bug. when the first trackable is added the sending enhanced location happens immediately but the subsequent ones happens in certain frequency. Sometimes the location updates will get a bit late, but I think that's the result of our location provider not providing locations in some circumstances. @KacperKluka can you please confirm that that should be the correct behaviour? i couldn't use Android publisher to test this as it crashes. I also think we can't add more than one trackable for Android publisher

@lawrence-forooghian
Copy link
Collaborator Author

@ikbalkaya Are you saying that you're seeing the callbacks happen, but after a delay? If so, what kind of delay are you seeing?

@KacperKluka
Copy link
Contributor

In the Android example app you can add multiple trackables and each of them has its own state displayed 😉 The trackable state is online only when: Ably connection state is connected, channel state is connected, and at least one location update was sent for this trackable. Whether a location update is sent for a trackable depends on its current resolution. So it might be that some trackables will publish locations once it's received from the location engine and others will save them in the 'skippedLocations' to be sent later.

lawrence-forooghian added a commit that referenced this issue Dec 1, 2022
I noticed that on a stationary device, or on a simulator set to simulate
a static location, the first trackable added would come online almost
immediately, but subsequent ones would not come online or sometimes
would come online after a long delay.

This is because a trackable will not come online until, after the
trackable is added, we receive a location update from LocationService.
The frequency of location updates from PassiveLocationManager is
determined by that of CLLocationManager, which usually only emits
location updates when the device has moved a sufficient distance (where
“sufficient” is determined by the publisher’s resolution policy).

So, we provide a mechanism for the publisher to request a location
update from CLLocationManager independently of the device’s motion, and
we request a location update when any trackable (except for the first)
is added.

It’s probably worth also explaining how Android behaves in this scenario
and why. My notes here are based on commit 3a0d833.

There are two different things that happen in the Android asset tracking
SDK, either of which has the side effect of causing the publisher to
receive a location update when a new trackable is added:

1. The publisher calls DefaultMapbox.changeResolution, which in turn
   ends up removing and re-adding all of (Google SDK)
   FusedLocationProviderClient’s location observers (via its
   requestLocationUpdates method). The documentation for
   FusedLocationProviderClient.requestLocationUpdates implies that
   observers will always receive a location update after this method is
   called (although it might be an old location).

2. (Only in the case where the added trackable is set as the active
   trackable – i.e. when it is added using .track and not .add)

   The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox
   SDK) MapboxNavigation.setNavigationRoutes. Through some process that I
   haven’t looked into, this ends up calling (Mapbox SDK)
   MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an
   explicit call to updateLocationMatcherResult, which emits a location
   update.

I think that the solution I’ve implemented for iOS turns Android side
effect 1 into an explicit behaviour.

Closes #425.
lawrence-forooghian added a commit that referenced this issue Dec 1, 2022
I noticed that on a stationary device, or on a simulator set to simulate
a static location, the first trackable added would come online almost
immediately, but subsequent ones would not come online or sometimes
would come online after a long delay.

This is because a trackable will not come online until, after the
trackable is added, we receive a location update from LocationService.
The frequency of location updates from PassiveLocationManager is
determined by that of CLLocationManager, which usually only emits
location updates when the device has moved a sufficient distance (where
“sufficient” is determined by the publisher’s resolution policy).

So, we provide a mechanism for the publisher to request a location
update from CLLocationManager independently of the device’s motion, and
we request a location update when any trackable (except for the first)
is added.

It’s probably worth also explaining how Android behaves in this scenario
and why. My notes here are based on the codebase at commit 3a0d833.

There are two different things that happen in the Android asset tracking
SDK, either of which has the side effect of causing the publisher to
receive a location update when a new trackable is added:

1. The publisher calls DefaultMapbox.changeResolution, which in turn
   ends up removing and re-adding all of (Google SDK)
   FusedLocationProviderClient’s location observers (via its
   requestLocationUpdates method). The documentation for
   FusedLocationProviderClient.requestLocationUpdates implies that
   observers will always receive a location update after this method is
   called (although it might be an old location).

2. (Only in the case where the added trackable is set as the active
   trackable – i.e. when it is added using .track and not .add)

   The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox
   SDK) MapboxNavigation.setNavigationRoutes. Through some process that I
   haven’t looked into, this ends up calling (Mapbox SDK)
   MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an
   explicit call to updateLocationMatcherResult, which emits a location
   update.

I think that the solution I’ve implemented for iOS turns Android side
effect 1 into an explicit behaviour.

Closes #425.
lawrence-forooghian added a commit that referenced this issue Dec 14, 2022
I noticed that on a stationary device, or on a simulator set to simulate
a static location, the first trackable added would come online almost
immediately, but subsequent ones would not come online or sometimes
would come online after a long delay.

This is because a trackable will not come online until, after the
trackable is added, we receive a location update from LocationService.
The frequency of location updates from PassiveLocationManager is
determined by that of CLLocationManager, which usually only emits
location updates when the device has moved a sufficient distance (where
“sufficient” is determined by the publisher’s resolution policy).

So, we provide a mechanism for the publisher to request a location
update from CLLocationManager independently of the device’s motion, and
we request a location update when any trackable (except for the first)
is added.

It’s probably worth also explaining how Android behaves in this scenario
and why. My notes here are based on the codebase at commit 3a0d833.

There are two different things that happen in the Android asset tracking
SDK, either of which has the side effect of causing the publisher to
receive a location update when a new trackable is added:

1. The publisher calls DefaultMapbox.changeResolution, which in turn
   ends up removing and re-adding all of (Google SDK)
   FusedLocationProviderClient’s location observers (via its
   requestLocationUpdates method). The documentation for
   FusedLocationProviderClient.requestLocationUpdates implies that
   observers will always receive a location update after this method is
   called (although it might be an old location).

2. (Only in the case where the added trackable is set as the active
   trackable – i.e. when it is added using .track and not .add)

   The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox
   SDK) MapboxNavigation.setNavigationRoutes. Through some process that I
   haven’t looked into, this ends up calling (Mapbox SDK)
   MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an
   explicit call to updateLocationMatcherResult, which emits a location
   update.

I think that the solution I’ve implemented for iOS turns Android side
effect 1 into an explicit behaviour.

Closes #425.
lawrence-forooghian added a commit that referenced this issue Dec 15, 2022
While testing a fix for #425, I wanted to have access to the location
history data in a log message.

This adds a switch in a new "Developer settings" section of the Settings
page, which when turned on will emit a JSON serialization of the
location history data when it’s received from the SDK. It’s turned off
by default, because it's only for my specific use case.
lawrence-forooghian added a commit that referenced this issue Dec 15, 2022
While testing a fix for #425, I wanted to have access to the location
history data in a log message.

This adds a switch in a new "Developer settings" section of the Settings
page, which when turned on will emit a JSON serialization of the
location history data when it’s received from the SDK. It’s turned off
by default, because it's only for my specific use case.
lawrence-forooghian added a commit that referenced this issue Dec 15, 2022
While testing a fix for #425, I wanted to have access to the location
history data in a log message.

This adds a switch in a new "Developer settings" section of the Settings
page, which when turned on will emit a JSON serialization of the
location history data when it’s received from the SDK. It’s turned off
by default, because it's only for my specific use case.
lawrence-forooghian added a commit that referenced this issue Dec 15, 2022
I noticed that on a stationary device, or on a simulator set to simulate
a static location, the first trackable added would come online almost
immediately, but subsequent ones would not come online or sometimes
would come online after a long delay.

This is because a trackable will not come online until, after the
trackable is added, we receive a location update from LocationService.
The frequency of location updates from PassiveLocationManager is
determined by that of CLLocationManager, which usually only emits
location updates when the device has moved a sufficient distance (where
“sufficient” is determined by the publisher’s resolution policy).

So, we provide a mechanism for the publisher to request a location
update from CLLocationManager independently of the device’s motion, and
we request a location update when any trackable (except for the first)
is added.

It’s probably worth also explaining how Android behaves in this scenario
and why. My notes here are based on the codebase at commit 3a0d833.

There are two different things that happen in the Android asset tracking
SDK, either of which has the side effect of causing the publisher to
receive a location update when a new trackable is added:

1. The publisher calls DefaultMapbox.changeResolution, which in turn
   ends up removing and re-adding all of (Google SDK)
   FusedLocationProviderClient’s location observers (via its
   requestLocationUpdates method). The documentation for
   FusedLocationProviderClient.requestLocationUpdates implies that
   observers will always receive a location update after this method is
   called (although it might be an old location).

2. (Only in the case where the added trackable is set as the active
   trackable – i.e. when it is added using .track and not .add)

   The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox
   SDK) MapboxNavigation.setNavigationRoutes. Through some process that I
   haven’t looked into, this ends up calling (Mapbox SDK)
   MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an
   explicit call to updateLocationMatcherResult, which emits a location
   update.

I think that the solution I’ve implemented for iOS turns Android side
effect 1 into an explicit behaviour.

Closes #425.
lawrence-forooghian added a commit that referenced this issue Dec 19, 2022
While testing a fix for #425, I wanted to have access to the location
history data in a log message.

This adds a switch in a new "Developer settings" section of the Settings
page, which when turned on will emit a JSON serialization of the
location history data when it’s received from the SDK. It’s turned off
by default, because it's only for my specific use case.
lawrence-forooghian added a commit that referenced this issue Dec 19, 2022
While testing a fix for #425, I wanted to have access to the location
history data in a log message.

This adds a switch in a new "Developer settings" section of the Settings
page, which when turned on will emit a JSON serialization of the
location history data when it’s received from the SDK. It’s turned off
by default, because it's only for my specific use case.
lawrence-forooghian added a commit that referenced this issue Dec 19, 2022
I noticed that on a stationary device, or on a simulator set to simulate
a static location, the first trackable added would come online almost
immediately, but subsequent ones would not come online or sometimes
would come online after a long delay.

This is because a trackable will not come online until, after the
trackable is added, we receive a location update from LocationService.
The frequency of location updates from PassiveLocationManager is
determined by that of CLLocationManager, which usually only emits
location updates when the device has moved a sufficient distance (where
“sufficient” is determined by the publisher’s resolution policy).

So, we provide a mechanism for the publisher to request a location
update from CLLocationManager independently of the device’s motion, and
we request a location update when any trackable (except for the first)
is added.

It’s probably worth also explaining how Android behaves in this scenario
and why. My notes here are based on the codebase at commit 3a0d833.

There are two different things that happen in the Android asset tracking
SDK, either of which has the side effect of causing the publisher to
receive a location update when a new trackable is added:

1. The publisher calls DefaultMapbox.changeResolution, which in turn
   ends up removing and re-adding all of (Google SDK)
   FusedLocationProviderClient’s location observers (via its
   requestLocationUpdates method). The documentation for
   FusedLocationProviderClient.requestLocationUpdates implies that
   observers will always receive a location update after this method is
   called (although it might be an old location).

2. (Only in the case where the added trackable is set as the active
   trackable – i.e. when it is added using .track and not .add)

   The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox
   SDK) MapboxNavigation.setNavigationRoutes. Through some process that I
   haven’t looked into, this ends up calling (Mapbox SDK)
   MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an
   explicit call to updateLocationMatcherResult, which emits a location
   update.

I think that the solution I’ve implemented for iOS turns Android side
effect 1 into an explicit behaviour.

Closes #425.
@lawrence-forooghian
Copy link
Collaborator Author

I did some investigation into this, as described in #481. The issue isn't as severe as I thought; once the device starts moving then we can expect it to eventually emit a didChangeConnectionState:forTrackable: callback. That PR contains a potential solution, but I was asked about the potential impact and started looking into it. I don't think that this issue is a priority now and I suggest we return it to the backlog.

@lawrence-forooghian lawrence-forooghian removed their assignment Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

Successfully merging a pull request may close this issue.

3 participants