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
Rover: add guided velocity controler #5735
Conversation
Just mentioning that this PR includes the commits from #5734 so be careful when merging this so that we don't end up with duplicate commits in master. |
Hi khancyr. Can you please remove the commits from #5734 as I have already merged that in. Thanks, Grant. |
@gmorph done! |
We need to test this A LOT. @khancyr - have you done much SITL and real world testing with this? |
@gmorph I used this with SITL and Gazebo mostly on skid steer configuration: see https://www.youtube.com/watch?v=o0zC7NgNBgE&index=8&list=PL6sCNLbHuYxYyFWLTq3E-R2cV1CrEoYOe for example. I don't have access to rover anymore so I can't test on pixhawk ... |
rebased on master |
I need to test this - sorry for the delay. |
rebased ! |
rebased |
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.
Stupid github - I only reviewed one commit - thought I could do a review per commit. Guess not. Must be per PR.
I'm still going.
if (verify_RTL()) { | ||
SRV_Channels::set_output_scaled(SRV_Channel::k_throttle, g.throttle_min.get()); | ||
set_mode(HOLD); | ||
} else { | ||
calc_lateral_acceleration(); | ||
calc_nav_steer(); |
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.
These changes and the one below have nothing to do guided velocity controller. They are just a re-order. Good idea but I would of preferred this to be in a separate commit. In fact if this was in a separate PR a quick test and I could of merged this straight in. But as it stands it adds time and complexity to the review process.
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 should have done that ! I extract them now
bool vel_ignore = packet.type_mask & MAVLINK_SET_POS_TYPE_MASK_VEL_IGNORE; | ||
bool acc_ignore = packet.type_mask & MAVLINK_SET_POS_TYPE_MASK_ACC_IGNORE; | ||
|
||
// record the time where the last message arrived |
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.
"when" the last message arrived - not "where"
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.
done!
APMrover2/GCS_Mavlink.cpp
Outdated
float ne_x = packet.x*rover.ahrs.cos_yaw() - packet.y*rover.ahrs.sin_yaw(); | ||
float ne_y = packet.x*rover.ahrs.sin_yaw() + packet.y*rover.ahrs.cos_yaw(); | ||
float ne_x = packet.x * rover.ahrs.cos_yaw() - packet.y * rover.ahrs.sin_yaw(); | ||
float ne_y = packet.x * rover.ahrs.sin_yaw() + packet.y * rover.ahrs.cos_yaw(); |
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.
AGAIN whitespace changes should be in a separate commit.
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.
it was in a function I modified ... so a PR for 2 spaces ... I know somebody that will to kill me ;-P
dcf72eb
to
da10041
Compare
rebased, correct the last comment : extract unrelated changes, extract style only changes |
This PR add the support of velocity control in guided mode (msg SET_POSITION). It is based on @rmackay9 previous works.
It has been tested on SITL with ROS and in gazebo