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.

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

feat: separate waypoint logic #457

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

Archdoog
Copy link
Collaborator

  • Separates waypoint advance logic out of step advance fn. This reduces the conditional burden required to advance a waypoint. It also enables flexible waypoint advance logic.

@@ -169,12 +169,24 @@ pub enum SpecialAdvanceConditions {
MinimumDistanceFromCurrentStepLine(u16),
}

// Condition to advance the remaining waypoints.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Condition to advance the remaining waypoints.
/// Controls when a waypoint should be marked as complete.
///
/// While a route may consist of thousands of points, waypoints are special.
/// A simple trip will have only one waypoint: the final destination.
/// A more complex trip may have several intermediate stops.
/// Just as the navigation state keeps track of which steps remain in the route,
/// it also tracks which waypoints are still remaining.
///
/// Tracking waypoints enables Ferrostar to reroute users when they stray off the route line.
/// The waypoint advance mode specifies how the framework decides
/// that a waypoint has been visited (and is removed from the list).
///
/// NOTE: Advancing to the next *step* and advancing to the next *waypoint*
/// are separate processes.
/// This will not normally cause any issues, but keep in mind that
/// manually advancing to the next step does not *necessarily* imply
/// that the waypoint will be marked as complete!

@@ -215,6 +194,16 @@ impl NavigationController {
&current_step_linestring,
remaining_steps,
);

// Trim the remaining waypoints if we should advance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Trim the remaining waypoints if we should advance.
// Trim the remaining waypoints if needed.

Comment on lines +338 to +348
if let Some(waypoint) = remaining_waypoints.first() {
let current_location: Point = snapped_user_location.coordinates.into();
let next_waypoint: Point = waypoint.coordinate.into();
match self.config.waypoint_advance {
WaypointAdvanceMode::WaypointWithinRange(range) => {
Haversine::distance(current_location, next_waypoint) < range
}
}
} else {
false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work! I think you can make this even more compact with the is_some_and like this:

remaining_waypoints.first().is_some_and(|waypoint| {
    // Rest of the inner code
}

Then you can drop the whole if/else wrapper and it's a single expression inside :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants