-
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
fw: add takeoff flaps setting #9858
Conversation
Great! This has been on the list for a long time! It makes sense why you did it that way, scaling the scale, otherwise we'd have to start messing with the mixer. I have it currently set in the mixer to limit the max flaps output at 25% of total output (100% = 250us pwm) but with this there might be a good reason to change that to have more granular control. I took a quick look at your changes and like the 3 options and saw the decoupling of flaperons (/airbrakes). I guess the only thing to think about is how and where we would add in the ability for a full wing flap, ie, is it a 4th option or treated as a separate case. I'm of two minds because this is great and very much needed and adding any type of iteration of this related to flaps is still a flap whereas extending the use could be classified as something different although since still a flap (and would in the vast majority of cases only be useful on takeoff), it might make sense to have it grouped here. Potentially getting ahead of myself but looks great to far! |
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.
Makes sense.
msg/vehicle_attitude_setpoint.msg
Outdated
@@ -20,7 +20,7 @@ bool yaw_reset_integral # Reset yaw integral part (navigation logic change) | |||
bool fw_control_yaw # control heading with rudder (used for auto takeoff on runway) | |||
bool disable_mc_yaw_control # control yaw for mc (used for vtol weather-vane mode) | |||
|
|||
bool apply_flaps | |||
int8 apply_flaps # 0 = no flaps, 1 = landing flaps setting, 2 = take-off flaps setting |
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 enum instead of standard int. We could for example use the enum states FLAPS_OFF, FLAPS_LAND, FLAPS_TAKEOFF. Rest of code is fine in my opinion.
Thanks for all these contributions! It's really great to see momentum on the fixed wing side and you're doing a great job in moving the architecture along bit-by-bit. |
Couldn't agree more! A definite thanks for this! |
@LorenzMeier @dagar any ideas why the OSX v4 build is failing? |
Likely a machine issue, restarting. |
Issue: currently no flaps are applied during auto-take-off.
Solution: add take-off specific flaps scale to enable some percentage of full flaps during take-off.
Description: Adds an extra parameter for scaling the given flap allowance for a specific take-off configuration. I.e., the FW_FLAPS_TO_SCL is actually scaling the FW_FLAPS_SCL value -- maybe slightly confusing, but I wasn't sure if there was a better way to add this without people needing to change their current flaps param settings.
Structural changes: I changed the apply flaps bool in uorb vehicle_attitude_setpoint to an int8 with 0=no flaps, 1=landing flaps, and 2=takeoff flaps configs. This way the position controller could pass the different options to the attitude controller. Currently, the flaperon functionality is unaffected by this -- but if any flaperon users want to include flaperons in the take-off, it could also be added with a similar extra param.
HITL logs: see https://review.px4.io/plot_app?log=3a43d493-6575-4908-9db6-54de352bed27 . I noticed the flaperon signal on that log for some reason doesnt show up in the flight review -- maybe I missed an output designation or mapping in the params, but I confirmed that the actuator_control_target0 output for sure got there after converting the logs -- see below:
There are several ways to approach this, the present implementation only one of those. Open to discussion on this. @philipoe @dagar @Antiheavy @ryanjAA