-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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
Plane: Differential spoilers support, elevon offset #2935
Plane: Differential spoilers support, elevon offset #2935
Conversation
I have checked out the code, rebased on master and built the code and tested and its not working for me. I will try to determine why. |
Grant, did you rebase master? Did the original code work?
|
I had rebased master (see my comment above). |
MIXING_OFFSET is a generic parameter name. Do we think it should change to something a bit more specific? I don't have any great ideas - what about - WING_MIXING_OFFSET? Hmmm, I don't really like that either. Thoughts? |
Note the tabbing in the files isn't great. A lot of the comments are offset from the code. Would be nice if that could be fixed. |
I called the new parameter MIXING_OFFSET to match up with the existing parameter MIXING_GAIN. --ET |
ch2 += abs(channel_rudder->servo_out); | ||
ch4 -= abs(channel_rudder->servo_out); | ||
} | ||
//get rudder value and multiply by DSPOILR_RUD_RATE/100: |
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 code block doesn't need to be here as its only called for the old ELEVON_MIXING configuration which we don't really want to change as its legacy code.
Hi ET. First thanks for submitting this PR. I know its been a long road for you as you have had to rebase a couple of time and resubmit the PR. We are nearly there! |
Sure, I should be able to work on it over the next few days. --ET |
That's great ET - thanks! I look forward to testing the new code. |
7f1a2fe
to
67636ad
Compare
I've updated it to use 'set_servo_out()', which supports the RC REV/MIN/MAX/TRIM settings. Added constraints to keep the generated values in the 900-2100 range, which matches what is in the existing elevon-mixing code. Reverted the legacy 'ELEVON_MIXING 1' code back to original (except removed tab chars). Setting the RC MIN/MAX values for the differential-spoiler channels to 900/2100 should yield matching outputs on the spoilers vs. the elevons. Here is an updated set of sample parameters for testing: DSPOILR_RUD_RATE,75 --ET |
Just a note that we really prefer whitespace changes as a separate commit. I know it seem's overly fussy but it makes our job much easier when we have to review loads of commits at release time. |
@gmorph Point taken on the whitespace commits. Do you want me to do an additional commit on this PR that reverses that bit of whitespace mod? (It just affects about 5 lines of code.) In general I see this PR as ready. --ET |
What's the status of this? @gmorph ? Has it been flown? |
Looks great, however wouldnt a general purpose HAL mixer be a better approach? regards |
1f85735
to
7f1a2fe
Compare
7f1a2fe
to
070f066
Compare
Rebased to master 2016-01-23 |
9b240b8
to
d32a5df
Compare
Oops, I think I accidentally swapped in the older version when I rebased. It should have the updated version now, and I rebased it to the current master (2016-02-11). --ET |
It will be great when it goes in. Been a long time! Thanks for taking the time @gmorph to get this through! |
} | ||
//change elevon 1 & 2 positions; constrain min/max: | ||
channel_roll->radio_out = constrain_int16(ch1,900,2100); | ||
channel_pitch->radio_out = constrain_int16(ch2,900,2100); |
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.
Rather then the hardcoded values for max and min PWM of 900 and 2100 its better to use the max and min for that channel. So
channel_roll->radio_min, channel_roll->radio_max
channel_pitch->radio_min, channel_pitch->radio_max
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.
As I mentioned in my Nov 29th comment, the constraints to keep the generated values in the 900-2100 range match what is in the existing elevon-mixing code (see line 698 with the comment "scale for a 1500 center and 900..2100 range, symmetric"). The channel-min/max values are applied later in the 'set_servo_out()' calls, yes?
b64b4aa
to
8699b5e
Compare
I've processed modified sections through 'astyle' (to conform comment indentations, etc), and rebased to current master 2016-02-20. --ET |
Could this possibly Be used for Differential thrust in twin engine Wing |
See #1861 for twin motor plane |
Fixed differential spoilers support, added elevon offset See PR ArduPilot#2935
8699b5e
to
c3e6a6e
Compare
Rebased to current master (2016-04-26) and changes put into single commit. @gmorph As far as I can tell this is ready for merging. Please let me know if something else needs to be done with it on my end. --ET |
@ethomas997 you've been a champ keeping up on this PR with the rebases. Sorry it's taking so long. @tridge Let's try and get this into v3.6.0 if we can. |
@ethomas997 are you able to do a rebase soon? Perhaps also a wiki entry? We'd like to get this merged ASAP |
Fixed differential spoilers support, added elevon offset See PR ArduPilot#2935
I have rebased on master, made the necessary fixes and tested it again on my plane on the ground. It works correctly. I submitted a new PR for it #4283 but you will still get the commit credit Thomas. Thanks for your patience. |
Fixed differential spoilers support, added elevon offset See PR #2935
merged, thanks! (7d82424) |
There is a report of someone having trouble getting their elevons to work correctly with plane 3.6 but that they work fine with plane 3.5 so I'm wondering if this is related? http://discuss.ardupilot.org/t/3-6-elevons-mixing-gain-mixing-offset-cant-get-proper-throw/9648 |
Possibly. Should be able to see with the other guys params.
|
I got here by following a different post asking about what i was looking for as well, but i don't seem to see any relation. i have differential spoilers and crowing one side at a time with yaw working on a fx-79 buffalo flying wing, now how do i also get both sides to crow for landing flaps? |
This is a resubmit of PR #2137 , applied to ArduPlane v3.4.1dev as of 2015-10-03.
Fixes ArduPlane differential spoilers when ELEVON_MIXING=0 and ELEVON_OUTPUT>0 (mixing done in flight controller). All flight modes are supported. This functionality is used on flying wings with "split elevons" using two servos on each side. In v3.2-v3.4.0 of ArduPlane, differential spoilers are non-operational (see issue #1032). This patch fixes them so they operate as described in the ArduPlane docs: http://plane.ardupilot.com/wiki/differential-spoilers
Added parameter: DSPOILR_RUD_RATE "Differential spoilers rudder rate"
Sets the amount of deflection that the rudder output will apply to the differential spoilers, as a percentage. The default value of 100 results in full rudder applying full deflection. A value of 0 will result in the differential spoilers exactly following the elevons (no rudder effect).
Added parameter: MIXING_OFFSET
The offset for the Vtail and elevon output mixers, as a percentage. This can be used in combination with MIXING_GAIN to configure how the control surfaces respond to input. The response to aileron or elevator input can be increased by setting this parameter to a positive or negative value. A common usage is to enter a positive value to increase the aileron response of the elevons of a flying wing. The default value of zero will leave the aileron-input response equal to the elevator-input response.
Notes: The new parameter DSPOILR_RUD_RATE allows the rudder effect to be "toned down." Otherwise it can be too easy to apply too much rudder-spoiler control and stall the wing. The new parameter MIXING_OFFSET allows the elevon mixing on a wing to be tuned so that the "aileron" effect is greater than the "elevator" effect. Flying wings are usually pitch-sensitive while being difficult to roll, so this tuning is desirable.
I have successfully tested this patch (on APM2) in a 47" Popwing, in Manual, Stabilized, and GPS-guided flight modes. I've flown it (on APM2) with the patch applied to ArduPlane v3.3.0. I've bench tested the patch, applied to ArduPlane v3.4.0, on Pixhawk.
Source and hex files are also posted here: http://www.etheli.com/APM/ArduPlane_etMod_DiffSpoilersFix
Here is sample set of parameters for testing the new functionality:
DSPOILR_RUD_RATE,75
ELEVON_OUTPUT,2
MIXING_OFFSET,75
RC5_FUNCTION,16
RC5_MAX,1900
RC5_MIN,1100
RC5_REV,1
RC6_FUNCTION,17
RC6_MAX,1900
RC6_MIN,1100
RC6_REV,1
The four servos are plugged into RC1, RC2, RC5 and RC6. One thing to watch out for is that the RC5_MIN, RC5_MAX, RC6_MIN and RC6_MAX parameters can be modified when a radio calibration is performed. If this happens, they should be restored to values like the ones above.
--ET