From 25521192740335c632fcde9e7729421ce7dc8e05 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Fri, 13 Mar 2026 18:22:05 +0530 Subject: [PATCH 1/4] fix(Race Condition): FerrostarWarapper race condition --- .../routing/FerrostarWrapperRepository.kt | 202 +++++++++++------- .../ui/directions/DirectionsViewModel.kt | 6 + 2 files changed, 132 insertions(+), 76 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt index 4d5c0b7..40b582f 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt @@ -25,8 +25,13 @@ import earth.maps.cardinal.data.LocationRepository import earth.maps.cardinal.data.OrientationRepository import earth.maps.cardinal.data.RoutingMode import earth.maps.cardinal.data.room.RoutingProfileRepository +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.first import javax.inject.Inject import javax.inject.Singleton +import kotlin.concurrent.thread @Singleton class FerrostarWrapperRepository @Inject constructor( @@ -36,100 +41,145 @@ class FerrostarWrapperRepository @Inject constructor( private val orientationRepository: OrientationRepository, private val routingProfileRepository: RoutingProfileRepository ) { - lateinit var walking: FerrostarWrapper - lateinit var cycling: FerrostarWrapper - lateinit var driving: FerrostarWrapper - lateinit var truck: FerrostarWrapper - lateinit var motorScooter: FerrostarWrapper - lateinit var motorcycle: FerrostarWrapper + private val _isInitialized = MutableStateFlow(false) + val isInitialized = _isInitialized.asStateFlow() + + private var _walking: FerrostarWrapper? = null + private var _cycling: FerrostarWrapper? = null + private var _driving: FerrostarWrapper? = null + private var _truck: FerrostarWrapper? = null + private var _motorScooter: FerrostarWrapper? = null + private var _motorcycle: FerrostarWrapper? = null + + val walking: FerrostarWrapper get() = _walking ?: throw IllegalStateException("Walking wrapper not initialized") + val cycling: FerrostarWrapper get() = _cycling ?: throw IllegalStateException("Cycling wrapper not initialized") + val driving: FerrostarWrapper get() = _driving ?: throw IllegalStateException("Driving wrapper not initialized") + val truck: FerrostarWrapper get() = _truck ?: throw IllegalStateException("Truck wrapper not initialized") + val motorScooter: FerrostarWrapper get() = _motorScooter ?: throw IllegalStateException("MotorScooter wrapper not initialized") + val motorcycle: FerrostarWrapper get() = _motorcycle ?: throw IllegalStateException("Motorcycle wrapper not initialized") + + private val pendingOptions = mutableMapOf() val androidTtsObserver = AndroidTtsObserver(context) + /** + * Suspends until the repository is initialized with a Valhalla endpoint. + */ + suspend fun awaitInitialization() { + _isInitialized.filter { it }.first() + } + fun setValhallaEndpoint(endpoint: String) { - walking = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.PEDESTRIAN, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - cycling = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.BICYCLE, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - driving = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.AUTO, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - truck = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.TRUCK, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - motorScooter = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.MOTOR_SCOOTER, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - motorcycle = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.MOTORCYCLE, - endpoint, - androidTtsObserver, - routingProfileRepository - ) + thread { + Thread.sleep(30000) + _walking = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.PEDESTRIAN, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _cycling = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.BICYCLE, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _driving = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.AUTO, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _truck = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.TRUCK, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _motorScooter = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.MOTOR_SCOOTER, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _motorcycle = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.MOTORCYCLE, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + + _isInitialized.value = true + + } + + // Apply pending options + synchronized(pendingOptions) { + pendingOptions.forEach { (mode, options) -> + setOptionsForMode(mode, options) + } + pendingOptions.clear() + } } /** * Updates the routing options for the specified mode by modifying the existing wrapper. + * If the wrapper is not yet initialized, the options are stored and applied once initialized. */ fun setOptionsForMode(mode: RoutingMode, routingOptions: RoutingOptions) { - when (mode) { - RoutingMode.PEDESTRIAN -> walking.setOptions(routingOptions) - RoutingMode.BICYCLE -> cycling.setOptions(routingOptions) - RoutingMode.AUTO -> driving.setOptions(routingOptions) - RoutingMode.TRUCK -> truck.setOptions(routingOptions) - RoutingMode.MOTOR_SCOOTER -> motorScooter.setOptions(routingOptions) - RoutingMode.MOTORCYCLE -> motorcycle.setOptions(routingOptions) - else -> {} + val wrapper = getWrapperForMode(mode) + if (wrapper != null) { + wrapper.setOptions(routingOptions) + } else { + synchronized(pendingOptions) { + pendingOptions[mode] = routingOptions + } } } /** * Resets the routing options for the specified mode to defaults by recreating the wrapper. + * If the wrapper is not yet initialized, the default options are stored and applied once initialized. */ fun resetOptionsToDefaultsForMode(mode: RoutingMode) { val defaultOptions = routingProfileRepository.createDefaultOptionsForMode(mode) - when (mode) { - RoutingMode.PEDESTRIAN -> walking.setOptions(defaultOptions) - RoutingMode.BICYCLE -> cycling.setOptions(defaultOptions) - RoutingMode.AUTO -> driving.setOptions(defaultOptions) - RoutingMode.TRUCK -> truck.setOptions(defaultOptions) - RoutingMode.MOTOR_SCOOTER -> motorScooter.setOptions(defaultOptions) - RoutingMode.MOTORCYCLE -> motorcycle.setOptions(defaultOptions) - else -> {} + val wrapper = getWrapperForMode(mode) + if (wrapper != null) { + wrapper.setOptions(defaultOptions) + } else { + defaultOptions?.let { + synchronized(pendingOptions) { + pendingOptions[mode] = it + } + } } } + + private fun getWrapperForMode(mode: RoutingMode): FerrostarWrapper? = when (mode) { + RoutingMode.PEDESTRIAN -> _walking + RoutingMode.BICYCLE -> _cycling + RoutingMode.AUTO -> _driving + RoutingMode.TRUCK -> _truck + RoutingMode.MOTOR_SCOOTER -> _motorScooter + RoutingMode.MOTORCYCLE -> _motorcycle + else -> null + } } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt index 20a3594..4ad555f 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/directions/DirectionsViewModel.kt @@ -108,6 +108,9 @@ class DirectionsViewModel @Inject constructor( suspend fun initializeRoutingMode() { + // Wait for FerrostarWrapperRepository to be initialized before setting options + ferrostarWrapperRepository.awaitInitialization() + // Set initial routing mode from preferences selectedRoutingMode = appPreferenceRepository.lastRoutingMode.value.let { modeString -> RoutingMode.entries.find { it.value == modeString } ?: RoutingMode.AUTO @@ -161,6 +164,9 @@ class DirectionsViewModel @Inject constructor( private fun fetchDrivingDirections(origin: Place, destination: Place) { viewModelScope.launch { + // Wait for FerrostarWrapperRepository to be initialized + ferrostarWrapperRepository.awaitInitialization() + routeStateRepository.setLoading(true) try { -- GitLab From d745f91549a039f50a4089b71d8d0cfdfa20aa28 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Fri, 13 Mar 2026 22:46:39 +0530 Subject: [PATCH 2/4] fix(test): FerrostarWarapper for Race Condition --- .../routing/FerrostarWrapperRepository.kt | 114 +++++++++--------- .../ui/directions/DirectionsViewModelTest.kt | 22 ++++ 2 files changed, 77 insertions(+), 59 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt index 40b582f..8240170 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt @@ -70,66 +70,62 @@ class FerrostarWrapperRepository @Inject constructor( } fun setValhallaEndpoint(endpoint: String) { - thread { - Thread.sleep(30000) - _walking = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.PEDESTRIAN, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - _cycling = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.BICYCLE, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - _driving = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.AUTO, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - _truck = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.TRUCK, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - _motorScooter = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.MOTOR_SCOOTER, - endpoint, - androidTtsObserver, - routingProfileRepository - ) - _motorcycle = FerrostarWrapper( - context, - locationRepository, - orientationRepository, - RoutingMode.MOTORCYCLE, - endpoint, - androidTtsObserver, - routingProfileRepository - ) + _walking = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.PEDESTRIAN, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _cycling = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.BICYCLE, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _driving = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.AUTO, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _truck = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.TRUCK, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _motorScooter = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.MOTOR_SCOOTER, + endpoint, + androidTtsObserver, + routingProfileRepository + ) + _motorcycle = FerrostarWrapper( + context, + locationRepository, + orientationRepository, + RoutingMode.MOTORCYCLE, + endpoint, + androidTtsObserver, + routingProfileRepository + ) - _isInitialized.value = true - - } + _isInitialized.value = true // Apply pending options synchronized(pendingOptions) { diff --git a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt index 4c9b73e..0e3adb2 100644 --- a/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt +++ b/cardinal-android/app/src/test/java/earth/maps/cardinal/ui/directions/DirectionsViewModelTest.kt @@ -1,3 +1,21 @@ +/* + * Cardinal Maps + * Copyright (C) 2025 Cardinal Maps Authors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package earth.maps.cardinal.ui.directions import earth.maps.cardinal.MainCoroutineRule @@ -95,6 +113,8 @@ class DirectionsViewModelTest { ) coEvery { mockRoutingProfileRepository.getProfilesForMode(any()) } returns flowOf(emptyList()) + // Mock FerrostarWrapperRepository + coEvery { mockFerrostarWrapperRepository.awaitInitialization() } returns Unit coEvery { mockFerrostarWrapperRepository.resetOptionsToDefaultsForMode(any()) } returns Unit viewModel = DirectionsViewModel( @@ -253,6 +273,8 @@ class DirectionsViewModelTest { address = null ) + mockFerrostarWrapperRepository.awaitInitialization() + // Setup mock to throw an exception val mockFerrostarWrapper = mockk() coEvery { mockFerrostarWrapperRepository.driving } returns mockFerrostarWrapper -- GitLab From c21d7f8a4c7063700fb1a879ca577d99a5729ba5 Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Fri, 13 Mar 2026 23:06:19 +0530 Subject: [PATCH 3/4] fix: removed unused import --- .../earth/maps/cardinal/routing/FerrostarWrapperRepository.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt index 8240170..8d4dec3 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt @@ -31,7 +31,6 @@ import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import javax.inject.Inject import javax.inject.Singleton -import kotlin.concurrent.thread @Singleton class FerrostarWrapperRepository @Inject constructor( -- GitLab From 0c95866f69ed4610234ed8784ef69224fbab7e3a Mon Sep 17 00:00:00 2001 From: mitulsheth Date: Mon, 16 Mar 2026 17:17:59 +0530 Subject: [PATCH 4/4] docs: migrate Javadoc to idiomatic KDoc in FerrostarWrapperRepository --- .../routing/FerrostarWrapperRepository.kt | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt index 8d4dec3..36fa3bd 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/routing/FerrostarWrapperRepository.kt @@ -62,7 +62,10 @@ class FerrostarWrapperRepository @Inject constructor( val androidTtsObserver = AndroidTtsObserver(context) /** - * Suspends until the repository is initialized with a Valhalla endpoint. + * Suspends the caller until the repository has been initialized with a Valhalla endpoint. + * + * Once [_isInitialized] becomes true, this function resumes. If the repository + * is already initialized, it returns immediately. */ suspend fun awaitInitialization() { _isInitialized.filter { it }.first() @@ -136,8 +139,14 @@ class FerrostarWrapperRepository @Inject constructor( } /** - * Updates the routing options for the specified mode by modifying the existing wrapper. - * If the wrapper is not yet initialized, the options are stored and applied once initialized. + * Updates the [RoutingOptions] for the specified [mode] by modifying the existing wrapper. + * + * If the wrapper for the given [mode] is not yet initialized (i.e., [setValhallaEndpoint] + * hasn't been called), the options are stored in [pendingOptions] and applied + * automatically once initialization completes. + * + * @param mode The [RoutingMode] to update (e.g., PEDESTRIAN, AUTO). + * @param routingOptions The new configuration options to apply. */ fun setOptionsForMode(mode: RoutingMode, routingOptions: RoutingOptions) { val wrapper = getWrapperForMode(mode) @@ -151,8 +160,16 @@ class FerrostarWrapperRepository @Inject constructor( } /** - * Resets the routing options for the specified mode to defaults by recreating the wrapper. - * If the wrapper is not yet initialized, the default options are stored and applied once initialized. + * Resets the [RoutingOptions] for the specified [mode] to their default values. + * + * This retrieves the defaults from [routingProfileRepository]. If the [FerrostarWrapper] + * for this mode is already initialized, the options are applied immediately. + * + * If the wrapper is not yet initialized (i.e., [setValhallaEndpoint] hasn't been called), + * the default options are stored in [pendingOptions] and applied automatically + * during initialization. + * + * @param mode The [RoutingMode] to reset (e.g., PEDESTRIAN, BICYCLE). */ fun resetOptionsToDefaultsForMode(mode: RoutingMode) { val defaultOptions = routingProfileRepository.createDefaultOptionsForMode(mode) @@ -168,6 +185,12 @@ class FerrostarWrapperRepository @Inject constructor( } } + /** + * Resolves the internal [FerrostarWrapper] instance associated with a specific [RoutingMode].* + * @param mode The [RoutingMode] for which to retrieve the wrapper. + * @return The corresponding [FerrostarWrapper] (e.g., [_walking], [_cycling]), + * or `null` if the mode is unrecognized or the wrapper hasn't been initialized via [setValhallaEndpoint]. + */ private fun getWrapperForMode(mode: RoutingMode): FerrostarWrapper? = when (mode) { RoutingMode.PEDESTRIAN -> _walking RoutingMode.BICYCLE -> _cycling -- GitLab