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
Add support for system ID of position controller #24801
base: master
Are you sure you want to change the base?
Conversation
01d324a
to
a49ab87
Compare
@lthall could you review this to make sure I did this right. I have flown this in a heli and got velocity drb results. |
This all looks great but using loiter like this is over kill I think. I would suggest using the simple reposition code or just holding position without human input. People can always switch out of sysid mode. |
65f09f6
to
59a43a4
Compare
@rmackay9 functionally checked this in SITL and have conducted several sysid flights on a real heli using the position controller sweep settings. Everything checks out. Ready to be merged. |
Vector3f comb_pos = curr_pos; | ||
comb_pos.x += _disturb_pos.x; | ||
comb_pos.y += _disturb_pos.y; |
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 curr_pos.xy() += _disturb_pos;
will work. You would have to make curr_pos
none const of course.
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 only pass in an xy disturbance, not xyz. So I have to breakout the assignment to the 3 axis vector.
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.
The curr_pos.xy()
breaks out just the xy part.
ea0dd2a
to
63ed9a4
Compare
63ed9a4
to
1ac652f
Compare
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.
A few minor things but basically looks good.
Vector3f comb_pos = curr_pos; | ||
comb_pos.x += _disturb_pos.x; | ||
comb_pos.y += _disturb_pos.y; |
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.
The curr_pos.xy()
breaks out just the xy part.
if ((AxisType)axis.get() == AxisType::DISTURB_POS_LAT || (AxisType)axis.get() == AxisType::DISTURB_POS_LONG | ||
|| (AxisType)axis.get() == AxisType::DISTURB_VEL_LAT || (AxisType)axis.get() == AxisType::DISTURB_VEL_LONG | ||
|| (AxisType)axis.get() == AxisType::INPUT_LOITER_LAT || (AxisType)axis.get() == AxisType::INPUT_LOITER_LONG) { | ||
ret = true; | ||
} |
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 would be neater as a switch statement.
@@ -3,6 +3,7 @@ | |||
#include "Copter.h" | |||
#include <AP_Math/chirp.h> | |||
#include <AP_ExternalControl/AP_ExternalControl_config.h> // TODO why is this needed if Copter.h includes this | |||
#include <AP_Math/control.h> |
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 could just be included in the cpp.
This PR adds additional inputs to conduct System ID on the position controller. It adds the following inputs
longitudinal velocity (body axis fwd/aft)
lateral velocity (body axis left/right)
position disturbance in body longitudinal axis
position disturbance in body lateral axis
velocity disturbance in body longitudinal axis
velocity disturbance in body lateral axis
this also includes the PSC logging during the system ID at the system ID logging rate. It allows the pilot to make velocity command inputs over top of the axis being swept but it is debatable whether this is even necessary since it should hold position except for the axis being swept.
so far i have tested this in SITL. would like @lthall to look at it. Not getting the expected response for the pilot velocity commanded inputs.