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

Make display of snapped location configurable #209

Open
1 of 3 tasks
ianthetechie opened this issue Aug 31, 2024 · 3 comments
Open
1 of 3 tasks

Make display of snapped location configurable #209

ianthetechie opened this issue Aug 31, 2024 · 3 comments
Labels
android enhancement New feature or request good first issue Good for newcomers iOS UI/UX Related to the UI/UX (on any platform) web

Comments

@ianthetechie
Copy link
Contributor

ianthetechie commented Aug 31, 2024

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 ianthetechie added android enhancement New feature or request good first issue Good for newcomers iOS UI/UX Related to the UI/UX (on any platform) web labels Aug 31, 2024
@ianthetechie
Copy link
Contributor Author

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:

locationEngine.lastLocation = uiState.value.snappedLocation?.toAndroidLocation()
.

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):

.

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
Copy link
Contributor

cu8code commented Sep 12, 2024

@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.

locationEngine.lastLocation = uiState.value.snappedLocation?.toAndroidLocation()

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
Copy link
Contributor Author

So, following the chain...

The startNavigation method returns the requisite state object:

return DefaultNavigationViewModel(this, spokenInstructionObserver, locationProvider)

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

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.

snapped_user_location: UserLocation,

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:

.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android enhancement New feature or request good first issue Good for newcomers iOS UI/UX Related to the UI/UX (on any platform) web
Projects
Status: No status
Development

No branches or pull requests

2 participants