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

fix: Use absolute target velocity to calculate end phase distance #150

Merged
merged 2 commits into from
Mar 22, 2022

Conversation

PaulVerhoeckx
Copy link
Contributor

Solves #145

@cesar-lopez-mar
Copy link

I think we should check how d_end_phase is used instead of changing is calculation. Negative d_end_phase is allowed, indicating the robot will end the current section at a distance behind the current position

@PaulVerhoeckx
Copy link
Contributor Author

I'm pretty sure the implementation is wrong. d_end_phase is set equal to the current theoretical stopping distance plus some factor which should compensate for delays. The sign of this theoretical stopping distance does not depend on the sign of current_x_vel while the compensation part does.

For example, take:
target_end_x_vel = 0
current_x_vel = target_x_vel = 1
target_x_decc = 1
dt=1/20

This yields:

t_end_phase_current = (target_end_x_vel - current_x_vel) / (-target_x_decc) = 1
d_end_phase = current_x_vel * t_end_phase_current -
                  0.5 * (target_x_decc) * t_end_phase_current * t_end_phase_current +
                  target_x_vel * 2.0 * dt = 1 - 0.5 + 0.1 = 0.6

While for when driving backwards, with:
current_x_vel = target_x_vel = -1

This yields:

t_end_phase_current = (target_end_x_vel - current_x_vel) / (-target_x_decc) = -1
d_end_phase = current_x_vel * t_end_phase_current -
                  0.5 * (target_x_decc) * t_end_phase_current * t_end_phase_current +
                  target_x_vel * 2.0 * dt = 1 - 0.5 - 0.1 = 0.4

@cesar-lopez-mar
Copy link

cesar-lopez-mar commented Mar 22, 2022

t_end_phase_current = (target_end_x_vel - current_x_vel) / (-target_x_decc) = -1

Time should not be negative. The bug is in this equation. During braking when going backwards we are actually accelerating! so the minus sign we added here should depend on the sign of the target velocity

@PaulVerhoeckx
Copy link
Contributor Author

I proposed a different fix to the problem in a new commit

@PaulVerhoeckx PaulVerhoeckx self-assigned this Mar 22, 2022
@PaulVerhoeckx PaulVerhoeckx merged commit 8f26afc into main Mar 22, 2022
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.

Controller's end phase distance is calculated wrong for backwards motion
2 participants