search

ably / ably-asset-tracking-swift

iOS client SDKs for the Ably Asset Tracking service.
Apache License 2.0
9 stars 6 forks source link

When we try to change the routing profile, the operation never completes #420

Closed lawrence-forooghian closed 1 year ago

lawrence-forooghian commented 1 year ago

The SDK appears to never be calling the callback of Publisher.changeRoutingProfile(profile:, completion:). For example, when using the "Change routing profile" button in the publisher example app.

https://user-images.githubusercontent.com/53756884/196193637-0b71dea9-5230-45f0-924b-258c5c2ecac8.mov

ikbalkaya commented 1 year ago

This bug seems to be caused partly by us partly by Mapbox that enforces a destination when providing route options. See https://github.com/ably/ably-asset-tracking-swift/blob/2a41b54585d5eb84dcf53d2326d82fbe9f4d6fc2/Sources/AblyAssetTrackingPublisher/Services/RouteProvider.swift#L59

Our code just ignores result handler when destination is set to null - so I provided a temporary fix that will at least invoke result handler. But in Android destination seems to be optional (@KacperKluka to confirm)

So we will either need to provide some arbitrary destination value or have to ask Mapbox for this

KacperKluka commented 1 year ago

The routing profile is tightly coupled with the destination. The only place where we specify a routing profile in Mapbox is while setting routes. In order to set a route we need a destination. So we need to have both destination and routing profile to properly set a route.

However, it's not required to provide a destination for a trackable as you've noticed here. So when there is no destination we simply don't set a route. I'm not exactly sure what's the issue here but it's probably an AAT problem 😉

ikbalkaya commented 1 year ago

Sounds sensible to me. However it looks like iOS deals with 'change routing profile' thinking that a trip with destination already exists and in fly. I will change this to channel the request to mapbox only if a destination is provided and change the setting when there is none.

ikbalkaya commented 1 year ago

I'm going to pause this work until we have an answer to whether UI should be invoking publisher interface or simply change a setting

lawrence-forooghian commented 1 year ago

Does this answer your question @ikbalkaya? https://github.com/ably/ably-asset-tracking-swift/pull/458#discussion_r1025051639