This looks wrong

[rmburg]

Why? What?

The first commit "fixes" a line I thought looked wrong. Before merging, I'd like to confirm this assumption.

The second commit is a simplification which should not change the functionality.
Orientation2::from_vector does pretty much exactly what stood there before.

ToDo / Known Issues

• Confirm the line was wrong

How to Test

stare at the code

[rmburg]

While I'm at it... What is this * 1.0 about?

hulk/crates/control/src/motion/step_planner.rs

Lines 96 to 99 in dd430bf

	Pose2::from_parts(
	arc.start + direction * 1.0,
	Orientation2::from_vector(direction),
	)

This is just a few lines further down in the same file

[knoellle]

I think the simplification is correct, but the correctness of the direction calculation depends on interpretation, though in practice it won"t make much of a difference.

The point is, we are essentially skipping short segments because using the direction of a short and thus likely jittery segment isn't great. The original implementation uses the orientation of the path that is taken by the robot, with your change it would use the orientation of the originally planned path.

If the first line segment in the path is longer than the max step size, there is no difference.
But think of a path with a short line segment, then an arc and a longer line segment with the first two having a combined length shorter than the max step size.
line_segment in the code you changed would then refer to the third path segment, but the start of that segment is not at the Ground origin.

Not sure why the * 1.0 is there...

[rmburg]

I guess at least in Ground coordinates is kind of makes sense... But it still looks wrong to write direction = line_segment.1 as one is a direction, the other a point... And it's inconsistent with the implementation for an arc, where the target orientation is always the tangent at the endpoint and never the vector "origin to endpoint"

[knoellle]

Fair point about the arc.

This also basically doesn't matter since the max step size is tiny compared to the minimum obstacle radius, which means that if the arc is shorter than the step size, the line segments before and after the arc segment are close to colinear anyway.

[oleflb]

what is the state here @knoellle @rmburg?

[rmburg]

As far as I understand it, this makes the code more understandable and consistent.
It should not change the behavior, as long as line_segment.0 is on the origin or close to it,
which is the case if the line starts at the position of the robot in ground coordinates.