Implement new route deviation handling - off step#879
Conversation
New kind of deviation - OffStep was introduced.
Archdoog
left a comment
There was a problem hiding this comment.
This is looking like the correct path for solving that.
|
Generally I think this the right approach for solving the original issue |
|
I made some structural changes to dramatically improve readability. The current failures in CI make absolutely no sense to me though. It seems to be referencing line numbers and code that don't even exist. I will have a look again later and see what's up. This PR should be good to go, and I assume GH Actions are just hallucinating crap now. Which is truly impressive when the ask is simply "check out what's actually in this branch by git ref." |
|
Oh, I see what's happening... GitHub Actions are running against a silently merged version with main and this branch together. That's.... actually not terrible. It just happens that there are changes which cleanly applied in a diff that are invalid 😂 |
|
Thanks @ianthetechie for jumping in - seen your changes they are good 👍 and your fight with actions - you might be correct about GH actions behavior this catched me offguard many times as well 😬 FYI: we are successfully using changes from this PR in our fork for last 2 weeks and will jump back on ferrostar main as soon as its merged , let me know if my further assistance is needed here |
|
Good to hear; thanks @freszu! I think I'm getting near the end of this ;) Will let my brain settle a bit and merge tomorrow :) cc @Archdoog if you have a moment, take a look at the last commit. I think I've resolved everything to match the intent of @devnull133's latest merged PR :) This PR actually introduces a number of refinements in how the state is modeled, which led to a valid diff but logical conflict 😅 |
|
Ok; I'll look at this again in the morning. I don't understand either the iOS or Android error... |
|
For android the tests failing here is diff with fix - verified by launching them locally: Detailsdiff --git a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/FerrostarCoreTest.kt b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/FerrostarCoreTest.kt
index 63e733d7..acc55278 100644
--- a/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/FerrostarCoreTest.kt
+++ b/android/core/src/androidTest/java/com/stadiamaps/ferrostar/core/FerrostarCoreTest.kt
@@ -42,6 +42,7 @@ import uniffi.ferrostar.Waypoint
import uniffi.ferrostar.WaypointAdvanceMode
import uniffi.ferrostar.WaypointKind
import uniffi.ferrostar.DeviationCalculationPolicy
+import uniffi.ferrostar.DeviationKind
import uniffi.ferrostar.stepAdvanceDistanceFromStep
import uniffi.ferrostar.stepAdvanceDistanceToEndOfStep
import uniffi.ferrostar.stepAdvanceManual
@@ -386,11 +387,11 @@ class FerrostarCoreTest {
override fun correctiveActionForDeviation(
core: FerrostarCore,
- deviationInMeters: Double,
- remainingWaypoints: List<Waypoint>,
+ deviation: DeviationKind,
+ remainingWaypoints: List<Waypoint>
): CorrectiveAction {
called = true
- assertEquals(42.0, deviationInMeters, Double.MIN_VALUE)
+ assertEquals(DeviationKind.CompletelyOffRoute(42.0), deviation)
return CorrectiveAction.GetNewRoutes(remainingWaypoints)
}
}
@@ -459,7 +460,11 @@ class FerrostarCoreTest {
routes.first(),
NavigationControllerConfig(
WaypointAdvanceMode.WaypointWithinRange(100.0),
- stepAdvanceDistanceFromStep(16u, 32u, DeviationCalculationPolicy.Always),
+ stepAdvanceDistanceFromStep(
+ distance = 16u,
+ minimumHorizontalAccuracy = 32u,
+ calculationPolicy = DeviationCalculationPolicy.ALWAYS
+ ),
stepAdvanceDistanceToEndOfStep(16u, 32u),
routeDeviationTracking =
RouteDeviationTracking.Custom(
As for iOS - i couldn't get it running quickly locally so there might be more - i believe we miss change for demo project - i did not touch it in my initial set of changes: Detailsdiff --git a/apple/DemoApp/Demo/NavigationDelegate.swift b/apple/DemoApp/Demo/NavigationDelegate.swift
index d51206d9..a7ca3f0e 100644
--- a/apple/DemoApp/Demo/NavigationDelegate.swift
+++ b/apple/DemoApp/Demo/NavigationDelegate.swift
@@ -10,7 +10,7 @@ class NavigationDelegate: FerrostarCoreDelegate {
// TODO: Create defaults extension on FerrostarCoreDelegate
}
- func core(_: FerrostarCore, correctiveActionForDeviation _: Double,
+ func core(_: FerrostarCore, correctiveActionForDeviation _: DeviationKind,
remainingWaypoints waypoints: [Waypoint]) -> CorrectiveAction
{
// If the user is off course, we'll try to calculate a new route using the remaining waypoints. |
|
Oh lol, I guess my local "run CI-ish checks" script didn't try building the iOS demo app 😂 |
New kind of deviation - OffStep was introduced.
this is a followup to #876 , trying to address #875
Let me know if i should push this further