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

Sub: Improvements to depth hold, new depth hold tests #15247

Merged

Conversation

Williangalvani
Copy link
Contributor

@Williangalvani Williangalvani commented Sep 5, 2020

Before:
Screenshot from 2020-09-04 18-40-11
Note that it was moving upwards with the throttle down.

After:
image

Another noticeable change is that when climbing/diving Alt and DAlt do not match anymore. That is because the old code just applied thrust and relaxed the controller (basically doing DAlt = Alt).

This still needs real life testing to make sure that response is not too sluggish
I tested on a BlueROV2 and it feels good.

Log:
00000001.zip

@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 3 times, most recently from 8b68aaa to 57ed261 Compare November 13, 2020 18:09
@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 3 times, most recently from 422915f to 850596d Compare February 18, 2021 20:00
@Williangalvani Williangalvani marked this pull request as ready for review February 18, 2021 20:21
@jaxxzer
Copy link
Member

jaxxzer commented Feb 24, 2021

Open a separate pr to add position_control.get_throttle_output, and use that in place of motors.get_throttle_input please.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

I think it is best to keep passing the throttle through the attitude controller to keep consistent with Copter.

@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 2 times, most recently from f609407 to 37105cc Compare March 2, 2021 20:10
ArduSub/control_althold.cpp Outdated Show resolved Hide resolved
ArduSub/control_althold.cpp Outdated Show resolved Hide resolved
@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 2 times, most recently from 86472f7 to ce5c7c7 Compare March 4, 2021 14:57
@Williangalvani Williangalvani added this to the Sub-4.2 milestone Aug 5, 2021
@Williangalvani Williangalvani mentioned this pull request Aug 6, 2021
29 tasks
@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 3 times, most recently from eba71a6 to 6e92ac1 Compare August 13, 2021 00:01
@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 2 times, most recently from 496a181 to 0703d6c Compare September 15, 2021 17:14
@Williangalvani Williangalvani force-pushed the improveDepthHoldMaster branch 2 times, most recently from e837e8c to 9bdc340 Compare March 14, 2022 17:37
Copy link
Contributor

@lthall lthall left a comment

Choose a reason for hiding this comment

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

This looks good!

@Williangalvani
Copy link
Contributor Author

Thanks for the reviews. I addressed Randy's review.

@Williangalvani Williangalvani merged commit cb4f5ac into ArduPilot:master Apr 13, 2022
@Williangalvani Williangalvani deleted the improveDepthHoldMaster branch April 13, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants