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

Apply the same maximum control rates (1.0) in CLI and MSP #658

Merged
merged 1 commit into from Mar 23, 2015

Conversation

thenickdude
Copy link
Contributor

Previously it was possible to set roll/pitch rate > 1.0 using MSP, but not using the CLI. Roll/pitch rate > 1.0 is meaningless.

@tricopterY
Copy link
Contributor

The pid_tuning.html for class="rate-tpa", currently has max="2.55" for roll/pitch and yaw. These should be limited to max="1.0". It helps the user to appreciate the limits. Not sure if it's necessary to have extra insurance in firmware code.

Previously it was possible to set roll/pitch rate > 1.0 using MSP, but
not using the CLI. Roll/pitch rate > 1.0 is meaningless.

TPA is also limited to 1.0.
@thenickdude
Copy link
Contributor Author

I figure that the Configurator is not the only consumer of the MSP API so it would be better to cover our bases. I've added the limit to TPA as well now.

@hydra
Copy link
Contributor

hydra commented Mar 23, 2015

I was under the assumption that some people use > 1 rates. Can you ask/check on the rcgroups thread?

@tricopterY
Copy link
Contributor

If you examine annexCode() in mw.c L#170, you do not expect to end up with -ve values for prop2, when you are calculating with a datum of 100 i.e rates are meant to be between 0 and 100%, which translates to limits of 0 to 1.0.
Note casting to (uint16_t) at L#204
prop1 = (uint16_t)prop1 * prop2 / 100;
We can end up with two's complement if prop2 is -ve, i.e something large.

@thenickdude
Copy link
Contributor Author

Roll/pitch rates >1.0 end up applying a negative factor of the PID correction (i.e. it makes the craft actively unstable), nobody would be using those.

Yaw rate >1.0 does make sense but the CLI has always limited that to max 1.0 so I doubt many are using it. I'll ask on RCGroups.

@hydra
Copy link
Contributor

hydra commented Mar 23, 2015

It was RC rate that I was thinking of, couldn't remember last night.

hydra added a commit that referenced this pull request Mar 23, 2015
Apply the same maximum control rates (1.0) in CLI and MSP
@hydra hydra merged commit 35abdb8 into cleanflight:master Mar 23, 2015
@thenickdude thenickdude deleted the rate-limits branch March 23, 2015 08:41
blckmn added a commit to blckmn/cleanflight that referenced this pull request Jul 2, 2016
Fixed build issue for F1, and added dfu CLI command (for restart in D…
martinbudden pushed a commit to martinbudden/cleanflight that referenced this pull request Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants