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

Copter: add weathervaneing #21609

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Sep 1, 2022

This adds quadplane style weathervaning to copter. It is only really valid when hovering, can significantly reduce power required for none symmetrical "aerodynamic" vehicles. Should also be good for helis.

Replaces #17333

Note that there is both a global enable parameter and a per mode for auto and guided. So to get weathervaning in auto one would set WVANE_ENABLE 1 and AUTO_OPTIONS 128.

Tested in SITL manually and with auto test. Actual weathervaneing method is well tested via quadplane, this just adds the calls to use it.

@IamPete1 IamPete1 added the Copter label Sep 1, 2022
Copy link
Member Author

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

Rather than per mode enable with AUTO_OPTIONS and GUIDED_OPTIONS we could use the WVANE_ENABLE to enable globally and then if we need it in the future we could add per mode disable.

//////////////////////////////////////////////////////////////////////////////
// Weathervane - allow vehicle to yaw into wind
#ifndef WEATHERVANE_ENABLED
# define WEATHERVANE_ENABLED !HAL_MINIMIZE_FEATURES
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure HAL_MINIMIZE_FEATURES is right threshold for this, although I'm not really sure what would be better, we could do BOARD_FLASH_SIZE > 1024.

@khancyr
Copy link
Contributor

khancyr commented Sep 2, 2022

isn't this live tested every days?

@@ -121,6 +121,7 @@ void RC_Channel_Copter::init_aux_function(const aux_func_t ch_option, const AuxS
case AUX_FUNC::AIRMODE:
case AUX_FUNC::FORCEFLYING:
case AUX_FUNC::CUSTOM_CONTROLLER:
case AUX_FUNC::WEATHER_VANE_ENABLE:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in the parent RC_Channel class by now

@@ -618,6 +619,22 @@ bool RC_Channel_Copter::do_aux_function(const aux_func_t ch_option, const AuxSwi
break;
#endif

#if WEATHERVANE_ENABLED == ENABLED
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in parent class, gated on AP_WEATEHRVANE_ENABLED

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that is is currently always enabled on quadplane. So we would endup having to do dummy methods on copter. Since this only ever going to be copter and plane I don't think its worth it, its not like some features that we want on all vehicles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really all that fussed. But when we get subs or boats weather-vaning in current I am going to remind you of this comment....

Tools/autotest/arducopter.py Outdated Show resolved Hide resolved
Tools/autotest/arducopter.py Outdated Show resolved Hide resolved
@rmackay9
Copy link
Contributor

rmackay9 commented Sep 6, 2022

Update from the dev call:

  • @lthall says he'd prefer to have the auto_yaw controller enhanced so we don't need to add the weatervane.update() calls in each mode.
  • @IamPete1 agreed to give a restructure a try although it will be a bigger patch and cost more flash.

@IamPete1
Copy link
Member Author

IamPete1 commented Nov 1, 2022

Re-based ontop of yaw re-work.

@IamPete1 IamPete1 force-pushed the Copter_WeatherVane branch 3 times, most recently from 7a62bbe to 633c663 Compare November 2, 2022 00:40
@rmackay9
Copy link
Contributor

This looks pretty good to me. The only thing I'm not a huge fan of is how the weathervane autoyaw mode changes the flight code's autoyaw mode. This splits the logic for how the autoyaw mode changes into two places which makes it more complex to understand... and thus more prone to errors.

@davidbuzz
Copy link
Collaborator

randy mentioned on dev call that hes ok with this post-rebase, but pete wants to do a bit more testing before it goes in

ArduCopter/mode.h Outdated Show resolved Hide resolved
ArduCopter/mode.h Outdated Show resolved Hide resolved
ArduCopter/mode.h Outdated Show resolved Hide resolved
@IamPete1
Copy link
Member Author

Rebased and re-tested.

@bnsgeyer
Copy link
Contributor

@IamPete1 were you able to test this on heli?

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.

Approved assuming that the TradHeli testing is successful.

@bnsgeyer
Copy link
Contributor

@IamPete1 Can you test this or do you need my help

@IamPete1
Copy link
Member Author

Rebased

@rmackay9 @bnsgeyer A quick heli demo, all seems to work as expected.

https://youtu.be/ycrhHy-ghKg

@rmackay9
Copy link
Contributor

@IamPete1, nice video thanks.

@bnsgeyer, good enough for merging?

ArduCopter/autoyaw.cpp Outdated Show resolved Hide resolved
@tridge tridge removed the DevCallEU label Dec 14, 2022
@bnsgeyer
Copy link
Contributor

@IamPete1 did that fix the issue? Just wondering if you had a chance to test it.

@IamPete1
Copy link
Member Author

@bnsgeyer Yes, fixed it now includes the roll trim as you suggested (642bc83). I have not re-tested, but I think that is a transparent change, clearly 0 on multirotor and constrained to +-10 on heli, so I don't foresee any issues.

@bnsgeyer
Copy link
Contributor

@IamPete1 i was more interested that you verified through testing that my suggested change fixed the problem you saw with heli. I am pretty confident it will but always good to verify there wasn’t some sign error or something. Thanks!

@IamPete1
Copy link
Member Author

@IamPete1 i was more interested that you verified through testing that my suggested change fixed the problem you saw with heli. I am pretty confident it will but always good to verify there wasn’t some sign error or something. Thanks!

Just had another go in realflight, it is better with the correct roll trim set, you can keep a smaller deadzone.

@bnsgeyer bnsgeyer self-requested a review December 14, 2022 21:05
Copy link
Contributor

@bnsgeyer bnsgeyer left a comment

Choose a reason for hiding this comment

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

Thanks for the testing

@bnsgeyer bnsgeyer merged commit d9dedf3 into ArduPilot:master Dec 14, 2022
@bnsgeyer
Copy link
Contributor

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Copter WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants