Skip to content
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

ArduPlane: add guided timeout #28943

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Ryanf55
Copy link
Collaborator

@Ryanf55 Ryanf55 commented Dec 24, 2024

Purpose

Allow configurable guided timeout. For an application expecting to send 50hz guided velocity commands, if the offboard controller dies, ArduPlane should fall back to loiter much sooner. This allows a user to bump up the expected controller rate.

Difference from copter

The parameter is the same, but this one calls constrain_float on the parameter. Copter only enforces the minimum value.
I also assumed that the plane will fall back to a guided loiter when control ends. Perhaps copter should enforce the max?

I also bumped the minimum period down to 0.01s. Copter doesn't seem to expect anything greater than 100mS between commands, but we know people operate controllers much faster than that. Perhaps copter's minimum should be bumped down too.

Testing Performed

  • N/A - will add autotest or manual test instructions soon.

@Ryanf55 Ryanf55 requested a review from IamPete1 December 24, 2024 18:04
* Preserve default of 3s

Signed-off-by: Ryan Friedman <[email protected]>
@Ryanf55 Ryanf55 force-pushed the plane-guided-timeout-configurable branch from 5387901 to 73dbd8c Compare December 24, 2024 18:29
ArduPlane/Parameters.cpp Outdated Show resolved Hide resolved
@IamPete1
Copy link
Member

We were talking about adding offboard mode. That would leave guided with only commands that don't timeout (ie positions), so maybe the param is better referring to the new mode.

@@ -1303,6 +1303,14 @@ const AP_Param::GroupInfo ParametersG2::var_info[] = {
// @User: Standard
AP_GROUPINFO("RNGFND_LND_ORNT", 36, ParametersG2, rangefinder_land_orient, ROTATION_PITCH_270),
#endif

// @Param: GUID_TIMEOUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the existing guided parameters use the prefix GUIDED_,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That came from Copter. Looks like Plane and Copter chose different prefixes for the same mode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I matched GUIDED_ prefix. For some reason the other GUIDED_ parameters are part of plane and not ModeGuided.

@timtuxworth
Copy link
Contributor

This seems to be the same as my #28526

@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Dec 25, 2024

We were talking about adding offboard mode. That would leave guided with only commands that don't timeout (ie positions), so maybe the param is better referring to the new mode.

How would you like it done? Can we call it GUIDED_TIMEOUT when backported to 4.6, and OFFBOARD_TIMEOUT on 4.7+ assuming we get offboard mode written?

@timtuxworth
Copy link
Contributor

It's not really a "timeout" though. Pretty sure it doesn't do this:

Allow configurable guided timeout. For an application expecting to send 50hz guided velocity commands, if the offboard controller dies, ArduPlane should fall back to loiter much sooner. This allows a user to bump up the expected controller rate.

I used it in Plane Follow because from my testing it acts to throttle the messages, so that it won't accept new guided commands if they arrive < this value after the previous message. That's why I called it "UPD_LIM" - it acts to limit the frequency of updates received.

@ArduPilot ArduPilot deleted a comment from Lalbabu2001 Dec 25, 2024
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Dec 27, 2024

This seems to be the same as my #28526

It's different because

  • I copied the implementation from Copter, reducing cognitive load on our users by keeping the param named the same and described the same as Copter and Rover
  • I enforce the limits of the parameter in the getter, you don't
  • My param is part of guided mode instead of plane so if you were to make guided mode optional, Plane doesn't have the flash cost.
  • You call the variable frequency_limit, but I prefer Copter's naming of calling it a timeout becuase that's what it does. If you don't send guided commands fast enough, it will fall back to a position loiter point.

I agree with Pete's assessment here: #28526 (comment)

Functionally, they achieve similar purposes to allow the plane to either

  • Recognize the companion computer stopped sending low level commands and fall back to the safer position loiter more quickly
  • Provide a way to reduce the timeout if you are sending multiple different low level control commands (Pete's optimization here make sense to add as a follow up for our offboard mode: ArduPlane: make Guided update frequency limit configurable #28526 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants