stadiamaps / ferrostar

A FOSS navigation SDK built from the ground up for the future
https://stadiamaps.github.io/ferrostar/
Other
179 stars 25 forks source link

Make display of snapped location configurable #209

Open ianthetechie opened 2 months ago

ianthetechie commented 2 months ago

Currently mobile platforms show the user's snapped location only. We should allow showing the raw location as this will be a practical requirement for non automotive navigation.

ianthetechie commented 2 months ago

Here's some background on how things work today and what needs to happen to make snapping configurable.

Android

The location is read from the snappedLocation property of NavigationUiState. This happens inside the NavigationMapView composable here: https://github.com/stadiamaps/ferrostar/blob/22f408f82bec2127dff89959a25fecb13a44d10f/android/maplibreui/src/main/java/com/stadiamaps/ferrostar/maplibreui/NavigationMapView.kt#L52.

The NavigationUiState is created from the TripState generated by the core (Rust). The location comes from here (for the purposes of this issue; you don't need to look at where the location provider comes from): https://github.com/stadiamaps/ferrostar/blob/22f408f82bec2127dff89959a25fecb13a44d10f/android/core/src/main/java/com/stadiamaps/ferrostar/core/NavigationViewModel.kt#L71.

iOS

On iOS, things are slightly less thought out (actually a bit surprising! Normally it's reversed!). We don't store any location in the NavigationState at the top level.

Ideas for implementation

I think that it would be a mistake to put this in TripState, because we want to support scenarios where the user is not actively "navigating" somewhere. Instead, I think that it belongs in the models observed by the UI.

On Android, this is already in the right place (the snappedLocation property of NavigationUiState). This already uses optional fields. For iOS, I think the right place to put this is in a new optional property on NavigationState (maybe we should unify the naming here? Thoughts @Archdoog?). In both cases the new/renamed property should be named userLocation. The Android implementation (linked above) will need to change a bit since it's currently switching to whichever is available in a way that the user can't control.

Then, we can extend the composable UI (SwiftUI / Jetpack Compose) components with a boolean property controlling whether to use the snapped location or not, when available (there are a few layers of views; the top level is the dynamically orienting variety, which is what most users will interact with).

cu8code commented 2 months ago

@ianthetechie thanks for the clarification. As I was looking into the android code, I had some additional questions. in uiState.value.snappedLocation were is the snapping occurring.

https://github.com/stadiamaps/ferrostar/blob/622a7875e8fa01aef544f432e49eec7be8d05d80/android/maplibreui/src/main/java/com/stadiamaps/ferrostar/maplibreui/NavigationMapView.kt#L52

we have got two providers but Android and Simulation, which are giving us the location. But i could not find the code were they are getting snapped.

I wanted to understand how android is deciding to give the snapped location ?

ianthetechie commented 2 months ago

So, following the chain...

The startNavigation method returns the requisite state object:

https://github.com/stadiamaps/ferrostar/blob/622a7875e8fa01aef544f432e49eec7be8d05d80/android/core/src/main/java/com/stadiamaps/ferrostar/core/FerrostarCore.kt#L261

In this case, we use the DefaultNavigationViewModel, which is basically a view into the internal FerrostarCore state:

https://github.com/stadiamaps/ferrostar/blob/622a7875e8fa01aef544f432e49eec7be8d05d80/android/core/src/main/java/com/stadiamaps/ferrostar/core/NavigationViewModel.kt#L60

Internally, you can think of the FerrostarCore class as starting an event loop. Once you get a route and start navigation, it subscribes to location inputs and, every time something changes, it feeds that input into the Rust NavigationController to get a new state back (it is basically a finite state machine; each input deterministically results in a new state, and the Kotlin and Swift platform wrappers hide that in a higher level interface so most devs don't have to think about it).

It is in the Rust code that the snapping logic actually occurs. Kotlin code is just exposing the snapped location from TripState to the view model.

https://github.com/stadiamaps/ferrostar/blob/622a7875e8fa01aef544f432e49eec7be8d05d80/common/ferrostar/src/navigation_controller/models.rs#L42

So it's not really "deciding" to give the snapped location; it is always giving the snapped location from TripState unless there is no active trip.

I'm suggesting that we add a new field to NavigationUiState like val location: UserLocation?, and pull that from the locationProvider (which I guess now needs to be a private val too). Then finally, we would amend the call to fromFerrostar so that we're passing the real location every time. The snapped location is already available from the state, as shown here, so we don't need to do this in the map:

https://github.com/stadiamaps/ferrostar/blob/22f408f82bec2127dff89959a25fecb13a44d10f/android/core/src/main/java/com/stadiamaps/ferrostar/core/NavigationViewModel.kt#L71.

Hopefully I've understood the question + explained it :) If anything is unclear, just ask!