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.

Sign up for GitHub

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

Jump to bottom

Fix scale application on FW throttle baro compensation #9099

Merged
merged 1 commit into from
Mar 19, 2018
Merged

Fix scale application on FW throttle baro compensation #9099

merged 1 commit into from
Mar 19, 2018

Conversation

sanderux
Copy link
Member

@sanderux sanderux commented Mar 17, 2018 •
edited by AuterionWrikeBot
Loading

The scale factor intended for the baro compensation was applied to the entire throttle range.
Now only applied to the eas2tas factor effect so it can be tuned properly

@dagar
Copy link
Member

dagar commented Mar 17, 2018

I'm not following why this is necessary. Can you break it down further?

@sanderux
Copy link
Member Author

Old situation:
Scale factor 1.1 while flying AMSL would mean:
eas2tas = sqrt(1013.25/1013.25) = 1
cruise_throttle = cruise_throttle * (eas2tas * 1.1) (just added 10% to throttle without any need

@sanderux
Copy link
Member Author

new situation:

scale factor 2 while flying AMSL would mean:
eas2tas = sqrt(1013.25/1013.25) = 1
scale factor = (eas2tas-1) * 2 + 1 = 1
cruise_throttle = cruise_throttle * scale_factor (remains unchanged as it should)

at altitude
eas2tas = sqrt(1013.25/900) = 1.06
scale factor = (eas2tas-1) * 2 + 1 = 1.12
cruise_throttle = cruise_throttle * scale_factor (now it increases as a function of the pressure difference)

@dagar
Copy link
Member

dagar commented Mar 18, 2018

Ok, I see what you mean. Why did you change the lower bound of the constrain?

dagar
dagar reviewed Mar 18, 2018
View reviewed changes
src/modules/fw_pos_control_l1/fw_pos_control_l1_params.c
@@ -98,7 +98,7 @@ PARAM_DEFINE_FLOAT(FW_THR_CRUISE, 0.6f);
* The default value of 0 will disable scaling.
*
* @min 0.0
* @max 2.0
* @max 10.0
Copy link
Member

Choose a reason for hiding this comment

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

Why 10? Even 2 was questionable.

Copy link
Member Author

Choose a reason for hiding this comment

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

As this changes the scale to be applied only on the amount of change the factor required is significantly higher. In our case approximately 3 is required

@sanderux
Copy link
Member Author

I changed the lower bound because i noticed we had instances of flying in 1030 Hpa, when scaling up hard it should probably not lower hard too. Generally makes it safer to tinker with this value.

dagar
dagar reviewed Mar 19, 2018
View reviewed changes
src/modules/fw_pos_control_l1/FixedwingPositionControl.cpp Outdated
@@ -1870,7 +1870,7 @@ FixedwingPositionControl::tecs_update_pitch_throttle(float alt_sp, float airspee
if (PX4_ISFINITE(baro.pressure) && PX4_ISFINITE(_parameters.throttle_alt_scale)) {
// scale throttle as a function of sqrt(p0/p) (~ EAS -> TAS at low speeds and altitudes ignoring temperature)
const float eas2tas = sqrtf(MSL_PRESSURE_MILLIBAR / baro.pressure);
const float scale = constrain(eas2tas * _parameters.throttle_alt_scale, 0.9f, 2.0f);
const float scale = constrain((eas2tas - 1) * _parameters.throttle_alt_scale + 1, 1.0f, 2.0f);
Copy link
Member

Choose a reason for hiding this comment

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

1 -> 1.0f

@sanderux
Copy link
Member Author

@dagar addressed your comment

@dagar
Copy link
Member

dagar commented Mar 19, 2018 •
edited
Loading

To be honest I'm still not sure if this scale factor is entirely correctly, but it seems workable (and safe).

@sanderux sanderux merged commit d87b7ac into master Mar 19, 2018
@sanderux
Copy link
Member Author

Please leave this branch for a few days

@dagar dagar deleted the fw-alt-scl-fix branch March 21, 2018 19:21
@mrpollo mrpollo mentioned this pull request Jun 5, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagar dagar dagar approved these changes

@RomanBapst RomanBapst Awaiting requested review from RomanBapst

Assignees
No one assigned
Labels
Fixed Wing 🛩️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sanderux @dagar