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

Tradheli- make input manager stabilize collective parameters percent #12860

Open
wants to merge 3 commits into
base: master
from

Conversation

@bnsgeyer
Copy link
Contributor

bnsgeyer commented Nov 20, 2019

In an effort to make parameters units more understandable to users, the stabilize collective parameters are changed from d% to percent. The resolution of these parameters in d% is not needed. the change was functionally checked. A parameter update was added and checked. Also a pre arm check was added to ensure values don't exceed 100.

@bnsgeyer bnsgeyer added the TradHeli label Nov 20, 2019
@bnsgeyer bnsgeyer requested a review from rmackay9 Nov 20, 2019
@bnsgeyer bnsgeyer self-assigned this Nov 20, 2019
// returns false if STAB_COL_1 is outside of range
if ((_heli_stab_col_max < 0) || (_heli_stab_col_max > 100)){
if (display_msg) {
gcs().send_text(MAV_SEVERITY_CRITICAL, "PreArm: IM_STAB_COL_4 out of range");

This comment has been minimized.

Copy link
@tridge

tridge Dec 3, 2019

Contributor

these need to be new names

Copy link
Contributor

rmackay9 left a comment

LGTM

@@ -119,4 +122,42 @@ float AC_InputManager_Heli::get_pilot_desired_collective(int16_t control_in)
return collective_out;
}

// parameter_check - check if input manager specific parameters are sensible
bool AC_InputManager_Heli::parameter_check(bool display_msg) const

This comment has been minimized.

@CraigElder CraigElder removed the DevCallTopic label Dec 3, 2019
@bnsgeyer bnsgeyer force-pushed the bnsgeyer:pr-stab-coll-percent branch 3 times, most recently from 508bf92 to e0058cb Dec 9, 2019
@bnsgeyer

This comment has been minimized.

Copy link
Contributor Author

bnsgeyer commented Dec 10, 2019

@rmackay9 and @peterbarker I corrected the parameter names in the messages and also conformed the pre-arm check function to what Peter suggested. I believe this is ready for merging.

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Dec 12, 2019

@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Dec 12, 2019

@bnsgeyer bnsgeyer force-pushed the bnsgeyer:pr-stab-coll-percent branch from e0058cb to 83544b6 Dec 12, 2019
@bnsgeyer

This comment has been minimized.

Copy link
Contributor Author

bnsgeyer commented Dec 12, 2019

@peterbarker thanks for the input. I think I got them all.

@bnsgeyer

This comment has been minimized.

Copy link
Contributor Author

bnsgeyer commented Dec 12, 2019

@peterbarker I change my mind regarding the names of the parameters. To this point our users are accustom to the current names with the numbers at the end. I think I will change them back to having the numbers at the end as it also keeps them in order when you look at them in the full parameter list.

@bnsgeyer bnsgeyer force-pushed the bnsgeyer:pr-stab-coll-percent branch from 83544b6 to 13a1b9e Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.