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.

Sign up for GitHub

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

Jump to bottom

Fix/android/view builder and speed json #360

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+18 −12
Open

Fix/android/view builder and speed json #360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b5151f0
fix: some minor fixes for easier customization
Archdoog Nov 14, 2024
8a989e7
Apply automatic changes
Archdoog Nov 14, 2024
a1dd74b
fix: some minor fixes for easier customization
Archdoog Nov 14, 2024
8a7659b
fix: some minor fixes for easier customization
Archdoog Nov 14, 2024
a7d5d28
Apply automatic changes
Archdoog Nov 14, 2024
2d4f5f7
fix: some minor fixes for easier customization
Archdoog Nov 14, 2024
92a2b5e
Merge branch 'main' of github.com:stadiamaps/ferrostar into fix/andro…
Archdoog Nov 15, 2024
7b892e0
fix: minor improvements for view builder, fix for valhalla speed unit…
Archdoog Nov 15, 2024
44d9608
fix: minor improvements for view builder, fix for valhalla speed unit…
Archdoog Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions ...src/main/java/com/stadiamaps/ferrostar/composeui/config/NavigationViewComponentBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ import com.stadiamaps.ferrostar.composeui.views.components.TripProgressView
import com.stadiamaps.ferrostar.core.NavigationUiState

data class NavigationViewComponentBuilder(
val instructionsView: @Composable (modifier: Modifier, uiState: NavigationUiState) -> Unit,
val progressView:
internal val instructionsView:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids users accidentally trying to do:

NavigationViewComponentBuilder.Default()
    .instructionsView { }

Which is very easy to do and highly problematic.

@Composable
(modifier: Modifier, uiState: NavigationUiState) -> Unit,
internal val progressView:
@Composable
(modifier: Modifier, uiState: NavigationUiState, onTapExit: (() -> Unit)?) -> Unit,
val streetNameView:
internal val streetNameView:
@Composable
(modifier: Modifier, roadName: String?, cameraControlState: CameraControlState) -> Unit,
val customOverlayView: @Composable (BoxScope.(Modifier) -> Unit)? = null,
internal val customOverlayView: @Composable (BoxScope.(Modifier) -> Unit)? = null,
// TODO: We may reasonably be able to add the NavigationMapView here. But not sure how much
// value that would add
// since most of the hard config already exists within the overlay views which are not
Expand Down Expand Up @@ -72,6 +74,8 @@ data class NavigationViewComponentBuilder(
}
})
}

fun getCustomOverlayView(): @Composable (BoxScope.(Modifier) -> Unit)? = customOverlayView
}

fun NavigationViewComponentBuilder.withInstructionsView(
Expand Down
4 changes: 2 additions & 2 deletions .../core/src/main/java/com/stadiamaps/ferrostar/core/annotation/SpeedSerializationAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class SpeedSerializationAdapter : JsonAdapter<Speed>() {
is Speed.NoLimit -> writer.name("none").value(true)
is Speed.Unknown -> writer.name("unknown").value(true)
is Speed.Value ->
writer.name("value").value(speed.value).name("unit").value(speed.unit.text)
writer.name("speed").value(speed.value).name("unit").value(speed.unit.text)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahmedre I believe the actual key should be "speed" when there's a value. E.g.

{
  "speed": 56,
  "unit": "km/h"
},

Let me know if there are additional cases. Definitely a bit of an obnoxious json object 😄.

Also, do we want to try catch the annotations parsing? On iOS we just convert a json parsing error to nil/null for annotations and an onError callback so that developers can log the failure. We figured better to let navigation succeed even in the case of a degraded/malformed annotations. See

ferrostar/apple/Sources/FerrostarCore/Annotations/AnnotationPublisher.swift

Lines 70 to 75 in 61e299a

do {
return try decoder.decode(Annotation.self, from: data)
} catch {
onError(error)
return nil
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are right - my mistake! agree re: try/catching the parsing with an error callback 👍

}
writer.endObject()
}
Expand All @@ -33,7 +33,7 @@ class SpeedSerializationAdapter : JsonAdapter<Speed>() {
var unit: String? = null

while (reader.hasNext()) {
when (reader.selectName(JsonReader.Options.of("none", "unknown", "value", "unit"))) {
when (reader.selectName(JsonReader.Options.of("none", "unknown", "speed", "unit"))) {
0 -> none = reader.nextBoolean()
1 -> unknown = reader.nextBoolean()
2 -> value = reader.nextDouble()
Expand Down
8 changes: 5 additions & 3 deletions android/demo-app/src/main/java/com/stadiamaps/ferrostar/DemoNavigationViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package com.stadiamaps.ferrostar

import android.util.Log
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.stadiamaps.ferrostar.core.DefaultNavigationViewModel
import com.stadiamaps.ferrostar.core.FerrostarCore
import com.stadiamaps.ferrostar.core.LocationProvider
import com.stadiamaps.ferrostar.core.LocationUpdateListener
import com.stadiamaps.ferrostar.core.NavigationUiState
import com.stadiamaps.ferrostar.core.NavigationViewModel
import com.stadiamaps.ferrostar.core.annotation.AnnotationPublisher
import com.stadiamaps.ferrostar.core.annotation.valhalla.valhallaExtendedOSRMAnnotationPublisher
import com.stadiamaps.ferrostar.core.isNavigating
import java.util.concurrent.Executors
import kotlinx.coroutines.flow.MutableStateFlow
Expand All @@ -24,7 +25,8 @@ import uniffi.ferrostar.UserLocation
class DemoNavigationViewModel(
// This is a simple example, but these would typically be dependency injected
private val ferrostarCore: FerrostarCore = AppModule.ferrostarCore,
) : ViewModel(), LocationUpdateListener, NavigationViewModel {
annotationPublisher: AnnotationPublisher<*> = valhallaExtendedOSRMAnnotationPublisher()
) : DefaultNavigationViewModel(ferrostarCore, annotationPublisher), LocationUpdateListener {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianthetechie now that this view model uses the DefaultNavigationViewModel. We should be able to refactor it to use the existing uiState from ferrostar and implement its own additional state for things like route loading, etc. Just to highlight how easy it is to customize without interrupting the default navigation behavior.

private val locationStateFlow = MutableStateFlow<UserLocation?>(null)
private val executor = Executors.newSingleThreadScheduledExecutor()

Expand Down
2 changes: 1 addition & 1 deletion ...main/java/com/stadiamaps/ferrostar/maplibreui/views/DynamicallyOrientingNavigationView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fun DynamicallyOrientingNavigationView(
}
}

views.customOverlayView?.let { customOverlayView ->
views.getCustomOverlayView()?.let { customOverlayView ->
customOverlayView(Modifier.windowInsetsPadding(WindowInsets.systemBars).padding(gridPadding))
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...ibreui/src/main/java/com/stadiamaps/ferrostar/maplibreui/views/LandscapeNavigationView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fun LandscapeNavigationView(
mapViewInsets = mapViewInsets,
onTapExit = onTapExit)

views.customOverlayView?.let { customOverlayView ->
views.getCustomOverlayView()?.let { customOverlayView ->
customOverlayView(Modifier.windowInsetsPadding(WindowInsets.systemBars).padding(gridPadding))
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...libreui/src/main/java/com/stadiamaps/ferrostar/maplibreui/views/PortraitNavigationView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ fun PortraitNavigationView(
mapViewInsets = mapViewInsets,
onTapExit = onTapExit)

views.customOverlayView?.let { customOverlayView ->
views.getCustomOverlayView()?.let { customOverlayView ->
customOverlayView(
Modifier.windowInsetsPadding(WindowInsets.systemBars).padding(gridPadding))
}
Expand Down
Loading