Add hysteresis to route deviation tracking #740
Conversation
Adds `on_route_threshold` parameter to `StaticThreshold` route deviation tracking. When set lower than `max_acceptable_deviation`, this creates a buffer zone that prevents users from oscillating between on-route and off-route states near the threshold boundary. For example, with max_acceptable_deviation=50m and on_route_threshold=40m: - User goes off-route when deviation exceeds 50m - User returns on-route only when deviation drops to 40m or less - This 10m buffer prevents rapid state changes when GPS accuracy fluctuates Includes comprehensive test coverage for the hysteresis behavior.
| /// 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. |
There was a problem hiding this comment.
Nice $100 word usage 😂
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😂
| /// 40m to be considered back on route. | ||
| /// | ||
| /// If not specified or equal to `max_acceptable_deviation`, no hysteresis is applied. | ||
| on_route_threshold: f64, |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
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.
Co-authored-by: Ian Wagner <ianwagner+github@switzerlandmail.ch>
…uffer Replace inline enum fields with StaticThresholdConfig struct that validates invariants at construction time, making invalid states impossible to create. - Add StaticThresholdConfig with failable constructor - Change on_route_threshold parameter to return_buffer for clearer semantics - Add StaticThresholdError for validation failures - Update Swift FFI wrapper and Kotlin demo app
|
@ianthetechie, I’ve just pushed the updates. I decided to go with return_buffer, since I think it’s easier to understand and less likely to lead to confusing configuration. Do these changes align with what you had in mind? I’m not really a “vibe coder” ;) but I do rely heavily on LLMs for Rust and anything Android-related, since I don’t know those technologies and don’t currently have time to go deeper. So I mostly act as a conceptual reviewer, assuming that syntax and basic patterns are handled better by LLMs than by me at this point. Just a heads-up that you may want to double-check for any issues and hidden nonsense. Feel free to make edits of any size in my code if that feels more efficient. |
ianthetechie
left a comment
ianthetechie
left a comment
There was a problem hiding this comment.
Just one more thing to look into; I can probably just do this myself today and also fix the CI issues.
Thanks!
| NegativeReturnBuffer { value: f64 }, | ||
| /// The return buffer must not exceed the maximum acceptable deviation. | ||
| #[cfg_attr( | ||
| feature = "std", |
There was a problem hiding this comment.
Unrelated to this PR, I just realized we don't need to gate this anymore. Tracked separately in #747.
| pub minimum_horizontal_accuracy: u16, | ||
| /// The maximum acceptable deviation from the route line, in meters. | ||
| /// | ||
| /// 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. | ||
| pub max_acceptable_deviation: f64, | ||
| /// The buffer distance used for hysteresis when returning to on-route state, in meters. | ||
| /// | ||
| /// The actual threshold for returning to on-route is calculated as: | ||
| /// `max_acceptable_deviation - return_buffer` | ||
| /// | ||
| /// For example, if `max_acceptable_deviation` is 50m and `return_buffer` is 10m, | ||
| /// the user must deviate more than 50m to trigger off-route, but must return within | ||
| /// 40m to be considered back on route. | ||
| /// | ||
| /// Set to 0 for no hysteresis (same threshold for going off-route and returning). | ||
| pub return_buffer: f64, |
There was a problem hiding this comment.
We have a constructor, but it looks like you can bypass this all with the pub fields. I think we can just hide these and expose accessors, but I'll have to check; @Archdoog noted that maybe UniFFI records require this but I'll check.
2 participants
Adds
on_route_thresholdparameter toStaticThresholdroute deviation tracking. When set lower thanmax_acceptable_deviation, this creates a buffer zone that prevents users from oscillating between on-route and off-route states near the threshold boundary.For example, with max_acceptable_deviation=50m and on_route_threshold=40m:
This is especially useful for slow-paced navigation, such as when walking. Otherwise, when the user is at the max_acceptable_deviation, on- and off-route alerts keep triggering constantly, which can be quite annoying.