stadiamaps / ferrostar

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

Fix/android/view builder and speed json #360

Closed Archdoog closed 6 days ago

Archdoog commented 1 week ago
ahmedre commented 1 week ago

On the DemoNavigationViewModel usage of DefaultNavigationViewModel - I think it is a step forward, but I think it is not as flexible as we have with just using FerrostarCore directly, because in this case, for example, both need to emit a NavigationUiState object.

We can swap NavigationUiState for a sealed class to solve this problem specifically for the demo use case of additional states like "not navigating" (which is what we also did in our implementation, but using FerrostarCore directly), but if we want to add lane support or speed limit support later, we'll need to either add these to NavigationUiState for everyone, or have our own ui state, or have two flows (in which case are we sure they're in sync, or would someone just need to combine them together to get a combination).

If we want to go this route, should we consider making NavigationUiState an interface with some required fields and let the return type be any instance? The downside of this approach though is that people would need an instance of check, and/or we'd need to do something with generics to avoid that. Alternatively, should we consider a single ViewModel for the demo app demoing the types of things you can do using FerrostarCore - it'd be opinionated and you could use it or use the formula from the ViewModel for building your own?

ianthetechie commented 6 days ago

This looks good to me; going to merge now but happy to discuss other ways to make things clearer on today's call @ahmedre