-
Notifications
You must be signed in to change notification settings - Fork 1.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
INAV Helicopter Support #9271
base: master
Are you sure you want to change the base?
INAV Helicopter Support #9271
Conversation
merge changes from iNavFlight/inav master
pull iNavFlight/inav master
merge upstream release 6.1.0
Merge recent fixes from iNavFlight/inav master branch
…exible handling of collective pitch and gyro gain channels
…r testing, compile additions conditionally to not waste memory on regular aircraft targets
…+ gyro gain channel is not used to switch modes. Also, apply rxrange to channels past non-aux-channel.
…detection from multirotor navigation
…first 8 bit before)
…TPUT_MAPPING_FULL endpoint. Keep MSP2_INAV_OUTPUT_MAPPING as is to ensure backwards compatibility.
…ranslate to collective-pitch letters) if target is variable pitch.
…e as a comment for discussion about airmode/I-term handling on helicopters.
…compensation not properly applied to rcCommand[COLLECTIVE]. Thus, NAV-ALTHOLD now working on collective-pitch aircraft.
src/main/fc/rc_controls.h
Outdated
AUX12 | ||
AUX12, | ||
COLLECTIVE = AUX3, //woga65: for heli-like aircraft | ||
GYRO_GAIN = AUX4 //... |
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.
Do we need this?
It probably would be better to integrate into the existing functionality:
https://github.com/iNavFlight/inav/blob/master/docs/Inflight%20Adjustments.md
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.
Do we need this?
It probably would be better to integrate into the existing functionality ...
Years ago I have tried this (inflight adjusments) approach. At first I encountered the difficulty that I could make adjustments rather in discrete steps than in absolute values. Then Michael Keller (I was still on Betaflight) told me how to use RC-Adjustment to map an RC-channel directly to the PID gain which was an undocumented feature. Sadly this did not work with INAV so I decided for myself that it is not worth the effort.
Of course one could use in-flight Adjustments to switch PID profiles according to the flight mode / headspeed, which leads to the need to have the Configurator handy on the flying field. To me this feels like a slow, cumbersome and not very flexible process.
src/main/fc/rc_controls.h
Outdated
@@ -37,12 +37,15 @@ typedef enum rc_alias { | |||
AUX9, | |||
AUX10, | |||
AUX11, | |||
AUX12 | |||
AUX12, | |||
COLLECTIVE = AUX3, //woga65: for heli-like aircraft |
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.
Isn't AUX2 a more common channel for collective? This matches defaults for both Spektrum and Futaba.
At least historically, futaba radios would use channel 5 for tail gain, and channel 6 (aux2) for collective.
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.
Isn't AUX2 a more common channel for collective? This matches defaults for both Spektrum and Futaba.
At least historically, futaba radios would use channel 5 for tail gain, and channel 6 (aux2) for collective.
Just a matter of personal preference, will change it to whatever is agreed on. I was using CH5 for arming and CH6 to switch navigation modes from the beginning, so I opted for CH7 onwards to do additional stuff.
Btw @MrD-RC mentioned that CH5 could make some difficulties on ELRS. I cannot confirm this because I do not use ELRS yet.
// to indicate the headspeed to the FC. For this, AUX3 is mapped | ||
// to the throttle RC-channel. The headspeed is used to determine, | ||
// which amount of collective pitch is needed to hover. woga65: | ||
#if defined(USE_VARIABLE_PITCH) |
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 to me fells a bit like the wrong approach.
The first 4 channels can be remmaped, yet, they don't required special handling here.
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 to me fells a bit like the wrong approach.
The first 4 channels can be remmaped, yet, they don't required special handling here.
I am not overly happy with and overly scared by my solution, this is why I am asking about your thoughts.
What happens is, that if AUX3 (which is already occupied by COLLECTIVE) is used for mode switching, it is rerouted to the THROTTLE channel which represents the headspeed which represents the flight mode.
So basically I am using the throttle RC-channel to indicate the flight mode to the FC without wasting an additional AUX channel.
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 will try to look at the code that handles the mapping of the first 4 channels, and see if we can easily adapt it to also map cyclic.
It is probably possible to omit the cyclic channel from any mode selection, as we don't offer ch1-4 as options there either.
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.
Just to keep our thoughts sorted.
There are three channel orders to have in mind.
- The channel order in the code (AERT or AERT56CG)
- The default channel order shown to the user (AETR or AECR56TG for Futaba, TAER or CAER56TG for Spektrum)
- The user defined mapping (user thinks he maps from AETR to whatever, but really maps from AERT to whatever)
So from the coder's perspective we talk about CH7 which is COLLECTIVE which looks like THROTTLE from the user's perspective (given the current state).
Note that the user sees T at the sventh position from the left which is CH7 from the user's perspective which might lend itself to be used to switch headspeeds / flight modes. This is why I routed CH7 (which is COLLECTIVE in the code) to the throttle channel.
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 you are mixing C with T in your comment above. Channel 3 is always throttle on futaba. Pitch/Collective is controlled by channel 6 in both futaba and spektrum. (I fly my helis with futaba radios)
So futaba woule be:
AETRC12346
And Spektrum
TAERC12346
Channel 5 would usually be connected to the tail gyro, but is probably irrelevant to this discussion.
The main issue is that stick commands rely on knowing the stick positions, but In helis, the throttle channel is no longer operated directly by stick position.
It is operated through a mix that changes the values of throttle channel and collective channel based on stick position.
That is what is creating the issue, correct?
The proposed solution is to use the collective channel to detect stick commands, as that is more likely to be set to a linear curve.
I don't think we should use channel 7 as collective by default, though, since that is not the stablished convention.
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.
just to make it clearer:
- I am ok using collective for stick commands. It just need to be documented somewhere for the users, when inevitably someone complains stick commands don't work for helis :)
- I think collective should be aux2 (channel 6)
- I am ok removing collective channel from mode selection
- I am ok with having collective configurable, but we are probably ok with it hard coded to channel 6.
@@ -78,7 +78,12 @@ typedef enum { | |||
BOXUSER4 = 49, | |||
BOXCHANGEMISSION = 50, | |||
BOXBEEPERMUTE = 51, | |||
BOXMULTIFUNCTION = 52, | |||
#if defined(USE_VARIABLE_PITCH) |
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.
Do we need to handle flight modes here?
Or should we have modes to switch governor profiles instead?
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.
flight modes also tend to imply handling pitch curves, btw. That is probably better left for the radio
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.
Do we need to handle flight modes here?
Or should we have modes to switch governor profiles instead?
For the moment I am using this to switch hover settings (nav_hc_hover_collective_normal
, nav_hc_hover_collective_idleup_1
, nav_hc_hover_collective_idleup_2
) as well as the target RPM for the governor, if governor type SET is configured (available governor types are OFF
| SIMPLE
| SET
).
Curves are handled by the radio.
#else | ||
// woga65: cut throttle, hold collective (kind of auto gyro) | ||
FAILSAFE_CHANNEL_NEUTRAL, // THROTTLE (cut trottle regardless until sufficiently tested) | ||
FAILSAFE_CHANNEL_HOLD, // AUX1 (NOT USED / ALWAYS HOLD) |
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.
Handling the collective channel mapping like the first 4 channels could skip having aux1 and aux2 here.
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.
Handling the collective channel mapping like the first 4 channels could skip having aux1 and aux2 here.
This would be true if COLLECTIVE + GYRO_GAIN were CH5 + CH6. Then, CONTROL_CHANNEL_COUNT would be 6 and nothing to skip at all.
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 am thinking along the lines of CONTROL_CHANNEL_COUNT=5, and gyro gain being handled as any adjustment in inav. Need to look into the code to see how feasible it would be.
INAV already has a way to adjust yaw gains in flight.
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.
Had a quick look,but haven't really tested it.
But maybe something like this:
+++ b/src/main/rx/rx.c
@@ -65,7 +65,7 @@
#include "rx/mavlink.h"
#include "rx/sim.h"
-const char rcChannelLetters[] = "AERT";
+const char rcChannelLetters[] = "AERTC";
static uint16_t rssi = 0; // range: [0;1023]
static timeUs_t lastMspRssiUpdateUs = 0;
@@ -527,7 +527,12 @@ void parseRcChannels(const char *input)
for (const char *c = input; *c; c++) {
const char *s = strchr(rcChannelLetters, *c);
if (s && (s < rcChannelLetters + MAX_MAPPABLE_RX_INPUTS))
- rxConfigMutable()->rcmap[s - rcChannelLetters] = c - input;
+ if(*s == 'C')
+ {
+ rxConfigMutable()->cyclic = c - input;
+ } else {
+ rxConfigMutable()->rcmap[s - rcChannelLetters] = c - input;
+ }
}
}
diff --git a/src/main/rx/rx.h b/src/main/rx/rx.h
index 4d5e2b135..881a22d91 100644
--- a/src/main/rx/rx.h
+++ b/src/main/rx/rx.h
@@ -107,6 +107,7 @@ PG_DECLARE_ARRAY(rxChannelRangeConfig_t, NON_AUX_CHANNEL_COUNT, rxChannelRangeCo
typedef struct rxConfig_s {
uint8_t receiverType; // RC receiver type (rxReceiverType_e enum)
uint8_t rcmap[MAX_MAPPABLE_RX_INPUTS]; // mapping of radio channels to internal RPYTA+ order
+ uint8_t cyclic;
uint8_t serialrx_provider; // Type of UART-based receiver (rxSerialReceiverType_e enum). Only used if receiverType is RX_TYPE_SERIAL
uint8_t serialrx_inverted; // Flip the default inversion of the protocol - e.g. sbus (Futaba, FrSKY) is inverted if this is false, uninverted if it's true. Support for uninverted OpenLRS (and modified FrSKY) receivers.
uint8_t halfDuplex; // allow rx to operate in half duplex mode. From tristate_e.
And when you are checking if the channel is in range, to activate a mode, you can always return false for the channel marked as cyclic, if you are in helicopter mode.
A helicopter has 5 main controls, instead of the traditional 4 of plane. the 5 is the pitch of the blades. On traditional systems, without a fc this is handled on the radio with a mix with different curves for pitch and throttle, driven by the throttle stick. To not have the extra channel for helis would probably need to bring all this extra helicopter specific stuf into inav. + different curves for different flight modes The extra channel mapping for cyclic should probably be done in a similar way as it is handled for the 4 main channels, with a default value for channel 6 for cyclic. That is the default value for cyclic in helicopter setups, and channel 5 is problematic with elrs. |
…ernor types available: SIMPLE governor + SET governor.
As it comes to me, I am absolutely fine with helicopters requiring at least six RC-channels plus probably extra channels for arming and fancy stuff. I would rather lose an RC-channel than having to deal with a software that tries to be helpful while at the same time making life more complicated.
Out of curiosity and because I am preparing to move to ELRS. What kind of problems occur with ELRS and CH5? |
Channel 5 in elrs is 1 bit. Either on, or off and should be high when "armed", so not really useable for gyro gain. If you use an elrs rx outputting sbus or pwm, you can remap the channels and send another channel value as channel 5, but you are still supposed to fly with channel 5 set high. My concern with the gyro gain is more to follow what is already there. I am not opposed to improving the existing functionality either. Plenty of gains that may benefit from being able to adjust more easily. |
Merge recent changes from the master branch
Switch default channel order
Full 32 bit timer usage flags with timer ID over MSP.
I am still waiting for a stack small enough to test this on my m1. Ali vendor sent me just the esc, instead of the stack I ordered. :/ |
Intended for use with Heli-Quads, Chinook style Twin-Rotor-Helis and regular Helicopters.
Corresponding pull request for the Configurator: iNavFlight/inav-configurator
INAV Configurator 6.1.0 for Variable Pitch Aircraft live preview: INAV Configurator 6.1.0_vari-pitch
*_VP - target files are variable pitch while regular target files should behave like regular INAV. The *_VP - target files can also handle regular aircraft, albeit they are intended for use with Collective-Pitch aircraft. The *_VP - targets offer extended RC-Channel remapping to account for the COLLECTIVE + GYRO_GAIN RC-channels.
Latest changes
Notes
pid_scaling_factor_...
which allow for PID scaling between 0 and 1 or 2. For the D-gain it is advised to set the scaling factors (for roll/pitch/yaw) to values between 0.1 and 0.05 so you have a finer grained resolution and can use Derivative values between 1 and 15 or 1 and 30 in the PID tuning tab of the configuratornav_hc_hover_collective_normal
+nav_hc_hover_collective_idleup_1
+nav_hc_hover_collective_idleup_2
with the throttle (= flight-mode) switch. The values stored in thenav_hc_hover_collective_xxx
CLI variables are used as a starting point for NAV-ALTHOLD.hc_rotor_spoolup_time
(10 = default, 0 = soft spool-up disabled). Active on the ground, disabled in the air to allow for bailing out of an auto rotation.nav_hc_althold_collective_type
(STICK | MID_STICK | HOVER).