Copter: Obey loiter_ccw flag for loiter turns #22834
Conversation
libraries/AC_WPNav/AC_Circle.cpp
Outdated
| switch (_direction) | ||
| { | ||
| case TurnDirection::CIRCLE_CW: | ||
| return fabsf(_rate); |
There was a problem hiding this comment.
why not just encode the direction in _rate?
There was a problem hiding this comment.
Yeah, I was thinking about it; it would make it much simpler. But that would mean if we go to circle mode after auto, we might have an inverted turn. We could save the original "_rate" and return back to default on exiting auto though..
|
Thanks this looks pretty good. Similar to what Andy is saying, I think we should perhaps change the interface so that the direction is included in the rate (e.g. rates can be -ve or +ve) or use a separate accessor and ensure the rate is always +ve. I think this would be best because otherwise different callers (or different portions of the code) could arrive at different answers as to whether they think the vehicle is moving cw or ccw. |
rishabsingh3003
force-pushed
the
fix_auto_circle
branch
from
e652fa8
to
5cc248e
Compare
|
I have gone back to just using rate like Andy suggested. Its much simpler this way |
| /// caller should set the position controller's x,y and z speeds and accelerations before calling this | ||
| void init(const Vector3p& center, bool terrain_alt); | ||
| void init(const Vector3p& center, bool terrain_alt, float rate_deg_per_sec); |
There was a problem hiding this comment.
I probably would have made the new init argument the direction rather than the rate but I guess it's ok.
It was currently impossible to set the direction of travel in loiter turns even though we have a flag in AP_Mission just to set that. Setting a radius negative would allow you to travel in ccw direction. Also, this aligns with the mavlink message description: https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_LOITER_TURNS
log of SITL testing: https://drive.google.com/drive/folders/1nYc4-lnAd6QgAIBKLO5yVyDQlQWoZ7hC?usp=share_link