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

Servo mixing under Flying Wing mode not working on latest release. (2015-08-27) #296

Open
Nandox7 opened this issue Sep 6, 2015 · 25 comments

Comments

@Nandox7
Copy link
Contributor

Nandox7 commented Sep 6, 2015

Hey,

Tried to update to the latest FW release (2015-08-27) and the servo mixing in Fling Wing Mode doesn't work.
Reverting to the previous one, makes it work. (2015-07-11)

Checking latest commits this one may be the culprit: ba9c0d2
I'll be running a few more tests with it.

@Nandox7 Nandox7 changed the title Flywing Servo mising not working on latest release Servo mixing under Flying Wing mode not working on latest release Sep 6, 2015
@Nandox7 Nandox7 changed the title Servo mixing under Flying Wing mode not working on latest release Servo mixing under Flying Wing mode not working on latest release. (2015-08-27) Sep 6, 2015
@trollcop
Copy link
Member

trollcop commented Sep 6, 2015

I'll be looking forward a patch that fixes it ;)
On Sep 6, 2015 23:37, "Nandox7" notifications@github.com wrote:

Hey,

Tried to update to the latest FW release (2015-08-27) and the servo mixing
in Fling Wing Mode doesn't work.
Reverting to the previous one, makes it work. (2015-07-11)

Checking latest commits this one may be the culprit: ba9c0d2
ba9c0d2
I'll be running a few more tests with it.


Reply to this email directly or view it on GitHub
#296.

@EmilsPa
Copy link
Contributor

EmilsPa commented Sep 8, 2015

Non of the Servo models work any more.
Planes and TRI copter
The servo Tab don't contain any data.

@trollcop
Copy link
Member

trollcop commented Sep 8, 2015

cool
On Sep 9, 2015 01:05, "EmilsPa" notifications@github.com wrote:

Non of the Servo models work any more.
Planes and TRI copter
The servo Tab don't contain any data.


Reply to this email directly or view it on GitHub
#296 (comment)
.

@readerror67
Copy link
Contributor

Can't we just roll back the stuff lazd commited ?

@trollcop
Copy link
Member

trollcop commented Sep 8, 2015

how about getting him to fix it instead?
On Sep 9, 2015 01:17, "readerror67" notifications@github.com wrote:

Can't we just roll back the stuff lazd commited ?


Reply to this email directly or view it on GitHub
#296 (comment)
.

@readerror67
Copy link
Contributor

Hes over on cleanflight now?

@Nandox7
Copy link
Contributor Author

Nandox7 commented Sep 8, 2015

I'm on Holiday's shouldn't be looking into code but...I want it to fly my
wing. :)

Yesterday I checked it a bit. Seems the mixer is a bit messed.
I got it just half working by changing the custom servo mixer to load only
when the new feature is enabled.
I'd get elevator mix but not ailerons so there is more to check in there.

I could be wrong but seems the mixer rules are overriding itself.
On 8 Sep 2015 17:22, "readerror67" notifications@github.com wrote:

Hes over on cleanflight now?


Reply to this email directly or view it on GitHub
#296 (comment)
.

@schugabe
Copy link
Contributor

schugabe commented Sep 8, 2015

if (core.useServo)

The condition in this line is wrong!
It should be mcfg.mixerConfiguration == MULTITYPE_CUSTOM_PLANE

@schugabe
Copy link
Contributor

schugabe commented Sep 8, 2015

Actually it's not that easy any more. It should load the custom mixer if we have a MULTITYPE_CUSTOM_PLANE or the feature SERVO_MIXER is active and the current model type does not have a default servo mixer.

Or should the feature SERVO_MIXER always activate the custom servo mixer chain?

if (mcfg.mixerConfiguration == MULTITYPE_CUSTOM_PLANE || feature(FEATURE_SERVO_MIXER))
            loadCustomServoMixer();

@Nandox7
Copy link
Contributor Author

Nandox7 commented Sep 8, 2015

@schugabe that was my first change as well. But doing that only makes the elevator mix work the ailerons still don't work.

What I did was to leave the current check and add and extra one for the feature.

if (core.useServo & feature(FEATURE_SERVO_MIXER))
         loadCustomServoMixer();

@dustin
Copy link

dustin commented Sep 9, 2015

@lazd Any input? Your change seems to have negatively impacted people.

@trollcop
Copy link
Member

trollcop commented Sep 9, 2015

left a turd at front door and ran away?
On Sep 9, 2015 01:21, "readerror67" notifications@github.com wrote:

Hes over on cleanflight now?


Reply to this email directly or view it on GitHub
#296 (comment)
.

@lazd
Copy link
Contributor

lazd commented Sep 9, 2015

I don't see why it wouldn't work, as it checked useServo for the mixer type and ran the servo mixer if it was on. Maybe @schugabe and @Nandox7 were unable to setup their servo mixes correctly using the smix command.

@lazd
Copy link
Contributor

lazd commented Sep 9, 2015

if (core.useServo)

The condition in this line is wrong!
It should be mcfg.mixerConfiguration == MULTITYPE_CUSTOM_PLANE

@schugabe No, that would defeat the purpose of #289, titled "Make servoMixer work for types other than CUSTOM_PLANE." As MULTITYPE_CUSTOM_PLANE defines useServo = 1, we still use the mixer for that multitype.

What if we moved the call to loadCustomServoMixer() before the setup of multitype-specific rules, then we wouldn't be overriding rules set by the multitype. Thoughts?

@lazd
Copy link
Contributor

lazd commented Sep 9, 2015

@schugabe @Nandox7 with the release you were having problems with previously, try setting up your servo mixer rules differently. The mapping for MULTITYPE_FLYING_WING seems to be target channel 3 -> servo 0 and target channel 4 -> servo 1.

# Control servo output 3 with channel 1 (Aileron)
smix 1 3 1 100 0 0 100 0

# Control servo output 4 with channel 2 (Elevator)
smix 2 4 2 100 0 0 100 0

@lazd
Copy link
Contributor

lazd commented Sep 9, 2015

I could be wrong but seems the mixer rules are overriding itself.

@schugabe that might be the case. Given that we have a number of different ways to setup servos, it seems my assumption that the servo mixer could run for any multitype that uses servos was wrong -- mutlitypes have their own, hardcoded servo mixer rules that are getting blown away by the custom servo mixer, and they never actually setup servo mixer rules using smix.

Ideally, when a given multitype is selected, the default rules would be added to the custom servo mixer for that type (smix style). Instead, we hardcode them completely separate from the custom servomixer, hence the situation my PR got us into.

After my PR, you would have to manually setup custom servo mixer rules to map your elevator and ailerons to stick inputs, which is obviously a breaking change. I didn't realize this at the time, but it makes sense now.

@Nandox7
Copy link
Contributor Author

Nandox7 commented Sep 9, 2015

@lazd I thought so.
Because I never had to set any mix when using the Flying Wing type. It makes the mix automatically and I just have to reverse the servo movements in the UI in case one of the channels is reversed (aileron/elevator).

Not sure what's the ideal solution here, but moving into a pure smix style could be one.
When setting a mode it would automatically insert the rules using smix.
This will break the UI i believe?

@schugabe
Copy link
Contributor

The way I implemented the servo mixing had a reason: clean up the hard coded mess. Custom servo mixing was more or less a nice by product because the implementation was flexible enough to support it as well.

My main problem with the suggested "load default rules to smix storage in all cases" is that there are no API/events that would allow to implement this without touching several different places where the actual model is set. Many different use cases must be considered. It results in a more complex implementation without any real benefit.

I think that most of the people just want to select a model and are done => therefore the load default rules based on model type during initialization is the default case and imho should stay the default case.

Just out of curiosity: What models do need the feature servo mixer?

@lazd
Copy link
Contributor

lazd commented Sep 10, 2015

@schugabe I wanted to use the servo mixer to map an AUX switch to a servo output for a QUADX. This is insanely easy to do on the CC3D with OpenPilot, and I was surprised that it was impossible when I moved to Naze32 + Baseflight. Baseflight is full of hardcoded features, such as support for gimbals that sits behind a feature, which enables servos for QUADX with a hardcoded calculation, but gives zero flexibility when it comes to configuring or overriding anything. I added the SERVO_MIXER feature in hopes of making the flexibility of the servo mixer available everywhere, but it seems there are just too many hardcoded features we have to work around for this to be both flexible and easy.

@Nandox7
Copy link
Contributor Author

Nandox7 commented Nov 3, 2015

I was away for some time but picking on this again.
Is this dead? I haven't seen any commit related to it, so I'd imagine the issue is still here?

@trollcop
Copy link
Member

trollcop commented Nov 3, 2015

Should be still there, I haven't seen anyone doing anything with this.
feel free to test/fix/pr :)

On Wed, Nov 4, 2015 at 12:47 AM, Nandox7 notifications@github.com wrote:

I was away for some time but picking on this again.
Is this dead? I haven't seen any commit related to it, so I'd imagine the
issue is still here?


Reply to this email directly or view it on GitHub
#296 (comment)
.

@lazd
Copy link
Contributor

lazd commented Nov 3, 2015

@Nandox7 the commits were rolled back, so the issue is fixed. 436f926

That said, it's my opinion that a unified way of handling servo mixing that removes the hardcoded features and brings everything together under smix would be a great step forward.

@trollcop
Copy link
Member

trollcop commented Nov 4, 2015

patches welcome, especially if they're not hacks

On Wed, Nov 4, 2015 at 1:38 AM, Larry Davis notifications@github.com
wrote:

@Nandox7 https://github.com/Nandox7 the commits were rolled back, so
the issue is fixed. 436f926
436f926

That said, it's my opinion that a unified way of handling servo mixing
that removes the hardcoded features and brings everything together under
smix would be a great step forward.


Reply to this email directly or view it on GitHub
#296 (comment)
.

@tracernz
Copy link
Contributor

tracernz commented Nov 4, 2015

@lazd rolling the commits back doesn't mean it's not welcome, just that it needs a little rework to iron out the issues. ;-)

@EmilsPa
Copy link
Contributor

EmilsPa commented Nov 4, 2015

Could you maybe roll back on the Hex file to?
The current stable in Gui don't work on any models with servos!..

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

No branches or pull requests

8 participants