Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added Assisted Teleop feature in Navigation2 #2575
Added Assisted Teleop feature in Navigation2 #2575
Changes from 4 commits
30a9e1f
cf9816e
9cd9745
0c7d714
60d33ea
98fd7b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reorder functions to be: configure, activate, deactivate, cleanup to be in order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove spaces between declare lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove trailing underscore from parameter name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be created in
execute
now? So that its made when requested to start?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be removed entirely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect, you're not contributing the component of Y velocity to X based on the angle, and vise versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so instead of speed_x and speed_y, the resultant of the 2 is needed (the hypot) for projection, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in the odometric frame. The velocities are in base frame. You need to take the Y component of the velocity and apply it to the position in X by the angle relative to the frame and vise versa - basic rigid body kinematics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unable to figure out the appropriate formula to perform the mentioned conversion. I did miss the fact that the velocities are not in the odom frame, to counter this I populated a Vector3Stamped object with the linear components of the Twist msg and transformed it from the base frame to the odom frame. Could you please let me know if this is an incorrect approach to the problem, and if so, what the correct approach would be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
X_new = component_x * cos(theta) + component_Y * sin(theta)
Y_new = component_x * sin(theta) + component_Y * cos(theta)
You should be able to see that formula in the backup recovery, no? Same idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just have this look keep track of the acumulated time (time_to_collision) and then if there's no collision, just pass through the command. Else, use that time_to_collision for computing the scale? There's no need to update the scale every iteration until there's a problem.
Also, that should not be a class member variable, also confusing to read.