Plane: Add radius to MAV_CMD_DO_REPOSITION #19860
Conversation
magicrub
force-pushed
the
pr/do_reposition_radius
branch
2 times, most recently
from
412d64e
to
faa3a0a
Compare
|
updated. This PR is not blocked by the mavlink PR because it's just taking advantage of an unused command_int param3. |
|
upstream mavlink PR has merged: mavlink/mavlink#1825 |
magicrub
force-pushed
the
pr/do_reposition_radius
branch
from
faa3a0a
to
64f8ba2
Compare
magicrub
force-pushed
the
pr/do_reposition_radius
branch
from
64f8ba2
to
3fb25e8
Compare
|
Anything else @WickedShell ? |
magicrub
force-pushed
the
pr/do_reposition_radius
branch
2 times, most recently
from
55b5be4
to
ff73df6
Compare
|
@WickedShell anything else? |
magicrub
force-pushed
the
pr/do_reposition_radius
branch
from
ff73df6
to
179e7d3
Compare
|
@WickedShell is requesting someone else to take a peek before he approves. |
|
I tagged this with DevCallTopic but I don't think it really needs to be discussed. It just needs to be looked at by someone else before we merge per @WickedShell 's request |
ArduPlane/Plane.h
Outdated
| @@ -944,7 +944,7 @@ class Plane : public AP_Vehicle { | |||
| #endif | |||
|
|
|||
| // commands.cpp | |||
| void set_guided_WP(const Location &loc); | |||
| void set_guided_WP(const Location &loc, const float radius = 0); // radius values: -1 = default (use param WP_LOITER_RAD), 0 = ignore (no-change), >0 is radius | |||
There was a problem hiding this comment.
Why have such complicated semantics for the second parameter?
Wouldn't it be easier for the (two) callers to call set_guided_radius directly?
There was a problem hiding this comment.
I had it like that at one point but it didn't seem right that anyone could reach down into the guided mode class and set that property anytime. Seems better as an atomic-ish thing with a single entry point
Yup, that's very true. I'll take a look into it |
ArduPlane/GCS_Mavlink.cpp
Outdated
| float radius = 0; | ||
| // Loiter radius for planes. Positive radius in meters, direction is controlled by Yaw (param4) value. | ||
| // radius values:-1 = use default param WP_LOITER_RAD, 0 = ignore (no-change), >0 radius | ||
| if (!isnan(packet.param3)) { |
There was a problem hiding this comment.
need to check > 0 as spec says +ve
ArduPlane/mode.h
Outdated
| // to: <= 0 use default param WP_LOITER_RAD, >0 radius_m | ||
| // we constrain to uint16_t because that is what update_loiter() is currently limited to | ||
| if (!is_zero(radius)) { | ||
| active_radius_m = (radius < 0) ? 0 : MIN(radius,UINT16_MAX); |
There was a problem hiding this comment.
set to default radius is <= 0, so it doesn't persist
|
adding a set_radius_and_direction() makes sense |
|
ping @magicrub - ball's in your court here - I think the requested changes are relatively small. |
magicrub
force-pushed
the
pr/do_reposition_radius
branch
from
179e7d3
to
26ba119
Compare
|
fixed |
magicrub
force-pushed
the
pr/do_reposition_radius
branch
from
26ba119
to
7470b10
Compare
mavlink change: ArduPilot/mavlink#240
Replaced #19297
This adds an adjustable radius for Guided mode vis the . This does not effect any other vehicle since the concept of radius does not apply to any vehicle that has a guided mode.
It's fully backwards compatible to existing command.
param3: was unused, now it's the positive-only radius. Ignore if <= 0
param4: controls radius, as stated in existing mavlink spec. Added option to ignore yaw if not == 0 or == 1.
This also lays the groundwork to easily add a new param GUIDED_LOIT_RAD in the future which is requested by @pompecukor
Behavior:
when entering guided mode, it uses WP_LOITER_RAD like usual. If the DO_REPOS command is received and the radius is > 0, then it overwrites the radius with this new one. Any change away and back to guided will reset you back to WP_LOITER_RAD. This is consistent with how DO_CHANGE_SPEED behaves.
This has been discussed with @peterbarker and @WickedShell