-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Add generic quad airframes for 250, 450, and 650 size quadrotors and set PWM_OUT in rc.mc_defaults. #13074
Conversation
From a user perspective I like this idea in theory, but I think the question is if the tuning is actually going to be valid across all the other variables (battery size, motors, ESCs, props, etc) for the same frame. With that in mind maybe some vaguely defined generic small, medium, large sizes as a rough starting point, but not get carried away? |
c78a23c
to
b7a279a
Compare
bce7b10
to
4fb0b31
Compare
4fb0b31
to
0be4002
Compare
0be4002
to
6675194
Compare
I've reduced the number of generic quad airframes previously added down to 250, 450, and 650 and moved the contents of Let me know if you have any other suggestions I could attend to with this PR. Thanks! |
fa609cf
to
cc0a149
Compare
cc0a149
to
363ba33
Compare
363ba33
to
2dc0967
Compare
6c91353
to
a4fd8c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changes are ok but I don't have a strong opinion either way.
if [ $AUTOCNF = yes ] | ||
then | ||
param set MC_ROLL_P 6.5 | ||
param set MC_ROLLRATE_P 0.05 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is exremely low. Did you test this?
|
||
param set MC_PITCH_P 6.5 | ||
param set MC_PITCHRATE_P 0.05 | ||
param set MC_PITCHRATE_I 0.05 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep all I gains default (only where set higher than the default (0.2), you can keep it)
if [ $AUTOCNF = yes ] | ||
then | ||
param set MC_ROLL_P 7.0 | ||
param set MC_ROLLRATE_P 0.15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all default, can you drop them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @dagar 's original intent here was basically a placeholder. I don't have a 450 airframe to know if the default values are appropriate. Do we know of any other good candidate 450 sized airframes aside from the flame wheel that might need different parameter values?
For now, I'll add a commit that drops these values and also drops the default values from 4011_dji_f450
. Thanks @bkueng !
# enable high-rate logging profile (helps with tuning) | ||
param set SDLOG_PROFILE 19 | ||
|
||
# use thrust curve factor (instead of TPA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# use thrust curve factor (instead of TPA) | |
# use thrust curve factor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks!
@@ -1,32 +1,32 @@ | |||
#!/bin/sh | |||
# | |||
# @name Generic 250 Racer | |||
# @name Generic Quadcopter (250mm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is specifically for racers, can you keep it in the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks!
Move set PWM_OUT 1234 to rc.mc_defaults for multicopter airframe files that call that script. Move contents of 4050_generic_250 to 4250_generic_quad_250 and delete 4050_generic_250.
a4fd8c0
to
5d4afe4
Compare
param set MC_YAWRATE_P 0.3 | ||
param set MC_YAWRATE_I 0.1 | ||
param set MC_YAWRATE_D 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For explanation of these deletions, all of these are all default parameter values.
Closing this; it can be revisited in the future. Thanks for your reviews! |
Describe problem solved by the proposed pull request
This PR adds a range of generic quadrotor airframes to advance PR #9849.
Additional context
See PR #9849 and PR #12506.
@dagar , would you let me know if you'd like to see anything different in this PR or if it is no longer of interest? Thanks!