Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ private final class DetectorImpl: RouteDeviationDetector {
public enum SwiftRouteDeviationTracking {
case none

case staticThreshold(minimumHorizontalAccuracy: UInt16, maxAcceptableDeviation: Double)
case staticThreshold(
minimumHorizontalAccuracy: UInt16,
maxAcceptableDeviation: Double,
onRouteThreshold: Double? = nil
)

case custom(detector: @Sendable (Route, TripState) -> RouteDeviation)

Expand All @@ -28,11 +32,13 @@ public enum SwiftRouteDeviationTracking {
.none
case let .staticThreshold(
minimumHorizontalAccuracy: minimumHorizontalAccuracy,
maxAcceptableDeviation: maxAcceptableDeviation
maxAcceptableDeviation: maxAcceptableDeviation,
onRouteThreshold: onRouteThreshold
):
.staticThreshold(
minimumHorizontalAccuracy: minimumHorizontalAccuracy,
maxAcceptableDeviation: maxAcceptableDeviation
maxAcceptableDeviation: maxAcceptableDeviation,
onRouteThreshold: onRouteThreshold ?? maxAcceptableDeviation
)
case let .custom(detector: detectorFunc):
.custom(detector: DetectorImpl(detectorFunc: detectorFunc))
Expand Down
18 changes: 15 additions & 3 deletions apple/Sources/UniFFI/ferrostar.swift

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

160 changes: 157 additions & 3 deletions common/ferrostar/src/deviation_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ pub enum RouteDeviationTracking {
/// If the distance between the reported location and the expected route line
/// is greater than this threshold, it will be flagged as an off route condition.
max_acceptable_deviation: f64,
/// The threshold for returning to on-route state, in meters.
///
/// Must be less than or equal to `max_acceptable_deviation`.
/// This creates hysteresis to prevent oscillation between on/off route states.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice $100 word usage 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, indeed $100 - straight from my $100/month Claude MAX plan 😄
I'm happy to reword it, however "hysteresis" is a pretty accurate term, even though this particular implementation is dead simple. Probably it could have skipped "oscillation" not to scare humans away.

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha nah I'm fine leaving it. It's a great word. Dictionaries and LLMs exist. Excuse my reviewing style which includes plenty of humor and snide remarks when not actually asking for a change; I'll make that obvious otherwise 😂

/// For example, if `max_acceptable_deviation` is 50m and `on_route_threshold` is 40m,
/// the user must deviate more than 50m to trigger off-route, but must return within
/// 40m to be considered back on route.
///
/// If not specified or equal to `max_acceptable_deviation`, no hysteresis is applied.
on_route_threshold: f64,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the API on the Swift side is great. But at the lower level, it's currently too easy for a user to accidentally violate the invariants (e.g. you have a note about behavior when it's equal, but what about when it's greater?). See https://cliffle.com/blog/rust-typestate/.

I think we should probably create a new struct with a failable constructor that captures the desired behavior, since we can't do this with enums. Basically we should make it so it's not possible to accidentally construct a nonsensical value :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You basically mean the scenario when user provides on_route_threshold > max_acceptable_deviation, right?

Another approach could be to specify max_acceptable_deviation and buffer_width (well, or come up with some better name). Then, it'd be rather impossible (or at least more difficult) to specify incorrect values. But I wasn't sure which one is easier to understand, and finally decided to go with more explicitly named "on_route_threshold".

Copy link
Contributor

Choose a reason for hiding this comment

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

You basically mean the scenario when user provides on_route_threshold > max_acceptable_deviation, right?

Yup. Additionally, you say (at least in the comment) that it can be "not specified" but that isn't actually possible at the moment, since it's a (non-optional) float 😂

I don't actually have a strong opinion on this and I think you've done more thinking about the use case than me, but from an API perspective I'd like to see it designed so it's either impossible or at least difficult to misuse. The buffer_width (or better name) idea does seem to satisfy this pretty well, but I don't really have a strong opinion.

One more bit that may not have been obvious in my hasty comment: the reason I suggest a separate struct is because that lets you do validation work in the constructor. A public enum in Rust is public all the way down by necessity, so to introduce something like a "failable" constructor you need to add another type (e.g. a struct) in the middle.

},
// TODO: Standard variants that account for mode of travel. For example, `DefaultFor(modeOfTravel: ModeOfTravel)` with sensible defaults for walking, driving, cycling, etc.
/// An arbitrary user-defined implementation.
Expand All @@ -80,24 +90,32 @@ impl RouteDeviationTracking {
RouteDeviationTracking::StaticThreshold {
minimum_horizontal_accuracy,
max_acceptable_deviation,
on_route_threshold,
} => match trip_state {
TripState::Idle { .. } | TripState::Complete { .. } => RouteDeviation::NoDeviation,
TripState::Navigating {
user_location,
remaining_steps,
deviation,
..
} => {
if user_location.horizontal_accuracy > f64::from(*minimum_horizontal_accuracy) {
return RouteDeviation::NoDeviation;
}

// Choose threshold based on current state (hysteresis)
let threshold = match deviation {
RouteDeviation::NoDeviation => *max_acceptable_deviation,
RouteDeviation::OffRoute { .. } => *on_route_threshold,
};

let mut first_step_deviation = None;

for (index, step) in remaining_steps.iter().enumerate() {
let step_deviation = self.static_threshold_deviation_from_line(
&Point::from(*user_location),
&step.get_linestring(),
max_acceptable_deviation.clone(),
threshold,
);

if index == 0 {
Expand Down Expand Up @@ -402,7 +420,8 @@ proptest! {
) {
let tracking = RouteDeviationTracking::StaticThreshold {
minimum_horizontal_accuracy,
max_acceptable_deviation
max_acceptable_deviation,
on_route_threshold: max_acceptable_deviation
};
let current_route_step = gen_dummy_route_step(x1, y1, x2, y2);
let route = gen_route_from_steps(vec![current_route_step.clone()]);
Expand Down Expand Up @@ -478,7 +497,8 @@ proptest! {
) {
let tracking = RouteDeviationTracking::StaticThreshold {
minimum_horizontal_accuracy: horizontal_accuracy - 1,
max_acceptable_deviation
max_acceptable_deviation,
on_route_threshold: max_acceptable_deviation
};
let current_route_step = gen_dummy_route_step(x1, y1, x2, y2);
let route = gen_route_from_steps(vec![current_route_step.clone()]);
Expand Down Expand Up @@ -506,3 +526,137 @@ proptest! {
);
}
}

/// Tests that hysteresis prevents oscillation between on-route and off-route states.
/// This test verifies that:
/// 1. User goes off-route when deviation > max_acceptable_deviation
/// 2. User stays off-route until deviation <= on_route_threshold
/// 3. User stays on-route until deviation > max_acceptable_deviation again
#[cfg(test)]
#[test]
fn static_threshold_hysteresis_prevents_oscillation() {
use crate::{
models::{GeographicCoordinate, UserLocation},
navigation_controller::test_helpers::{gen_dummy_route_step, gen_route_from_steps, get_navigating_trip_state},
};

#[cfg(feature = "std")]
use std::time::SystemTime;
#[cfg(feature = "web-time")]
use web_time::SystemTime;

let max_deviation = 50.0;
let on_route_threshold = 40.0;

let tracking = RouteDeviationTracking::StaticThreshold {
minimum_horizontal_accuracy: 100,
max_acceptable_deviation: max_deviation,
on_route_threshold,
};

// Create a simple route step from (0, 0) to (0, 0.001) (~111 meters north)
let current_route_step = gen_dummy_route_step(0.0, 0.0, 0.0, 0.001);
let route = gen_route_from_steps(vec![current_route_step.clone()]);

// Start on route
let user_location_on_route = UserLocation {
coordinates: GeographicCoordinate { lng: 0.0, lat: 0.0 },
horizontal_accuracy: 5.0,
course_over_ground: None,
timestamp: SystemTime::now(),
speed: None,
};

// Initially on route
let trip_state_on_route = get_navigating_trip_state(
user_location_on_route.clone(),
vec![current_route_step.clone()],
vec![],
RouteDeviation::NoDeviation,
);

assert_eq!(
tracking.check_route_deviation(&route, &trip_state_on_route),
RouteDeviation::NoDeviation
);

// Move 45m away from route (between on_route_threshold and max_deviation)
// At this distance, should still be on-route since we started on-route
let user_location_45m = UserLocation {
coordinates: GeographicCoordinate { lng: 0.0004, lat: 0.0 }, // ~45m east
horizontal_accuracy: 5.0,
course_over_ground: None,
timestamp: SystemTime::now(),
speed: None,
};

let trip_state_45m_from_onroute = get_navigating_trip_state(
user_location_45m.clone(),
vec![current_route_step.clone()],
vec![],
RouteDeviation::NoDeviation, // Still on-route from previous state
);

// Should remain on-route because 45m < max_deviation (50m)
assert_eq!(
tracking.check_route_deviation(&route, &trip_state_45m_from_onroute),
RouteDeviation::NoDeviation
);

// Move 55m away from route (beyond max_deviation)
// Should trigger off-route
let user_location_55m = UserLocation {
coordinates: GeographicCoordinate { lng: 0.0005, lat: 0.0 }, // ~55m east
horizontal_accuracy: 5.0,
course_over_ground: None,
timestamp: SystemTime::now(),
speed: None,
};

let trip_state_55m = get_navigating_trip_state(
user_location_55m.clone(),
vec![current_route_step.clone()],
vec![],
RouteDeviation::NoDeviation, // Was on-route
);

// Should be off-route now
let deviation_result = tracking.check_route_deviation(&route, &trip_state_55m);
assert!(matches!(deviation_result, RouteDeviation::OffRoute { .. }));

// Move back to 45m (between thresholds)
// Should STAY off-route because 45m > on_route_threshold (40m)
let trip_state_45m_from_offroute = get_navigating_trip_state(
user_location_45m.clone(),
vec![current_route_step.clone()],
vec![],
RouteDeviation::OffRoute { deviation_from_route_line: 55.0 }, // Was off-route
);

// Should remain off-route because 45m > on_route_threshold (40m)
let deviation_result_2 = tracking.check_route_deviation(&route, &trip_state_45m_from_offroute);
assert!(matches!(deviation_result_2, RouteDeviation::OffRoute { .. }));

// Move to 35m (below on_route_threshold)
// Should return to on-route
let user_location_35m = UserLocation {
coordinates: GeographicCoordinate { lng: 0.00031, lat: 0.0 }, // ~35m east
horizontal_accuracy: 5.0,
course_over_ground: None,
timestamp: SystemTime::now(),
speed: None,
};

let trip_state_35m = get_navigating_trip_state(
user_location_35m.clone(),
vec![current_route_step.clone()],
vec![],
RouteDeviation::OffRoute { deviation_from_route_line: 45.0 }, // Was off-route
);

// Should be back on-route because 35m <= on_route_threshold (40m)
assert_eq!(
tracking.check_route_deviation(&route, &trip_state_35m),
RouteDeviation::NoDeviation
);
}
1 change: 1 addition & 0 deletions common/ferrostar/src/navigation_controller/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub fn get_test_navigation_controller_config(
route_deviation_tracking: RouteDeviationTracking::StaticThreshold {
minimum_horizontal_accuracy: 0,
max_acceptable_deviation: 0.0,
on_route_threshold: 0.0,
},
snapped_location_course_filtering: CourseFiltering::Raw,
step_advance_condition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ config:
StaticThreshold:
max_acceptable_deviation: 0
minimum_horizontal_accuracy: 0
on_route_threshold: 0
snapped_location_course_filtering: Raw
step_advance_condition:
DistanceToEndOfStep:
Expand Down
Loading