lkaino / Triflight

Triflight flight controller firmware for tricopters
http://rcexplorer.se
GNU General Public License v3.0
50 stars 19 forks source link

Servo reversing not supported #11

Closed lkaino closed 7 years ago

lkaino commented 8 years ago

Tricopter mixer logic doesn't support servo reversing.

lkaino commented 8 years ago

Check http://rcexplorer.se/forums/topic/debugging-the-tricopter-mini-racer/page/45/#post-25013 and the following post.

pqueiros commented 8 years ago

@lkaino not sure you have seen. http://rcexplorer.se/forums/topic/debugging-the-tricopter-mini-racer/page/46/#post-25032 If it looks good I will make a pull request. See also

http://rcexplorer.se/forums/topic/debugging-the-tricopter-mini-racer/page/46/#post-25050

lkaino commented 8 years ago

Sorry, missed that.

Looks fine to me, but would need to check more carefully how the reversing works.

Could you add a simple check to call the toggle function only if the servo is reversed.

Would need to be flight tested with a servo that requires reversing.

PProvost commented 8 years ago

I'm pretty new to Triflight (just finished my first tricopter in over a year--pre-Cleanflight days), but since I'm one who needs servo reversing to work, perhaps someone can explain to me why just setting the "Direction and rate" to "-100%" in the Servos tab isn't the right answer? From my very initial testing (just a quick hover) it seems to be right. Am I missing something?

Bengt-M commented 8 years ago

I guess this needs some expert advice from @lkaino but a short look suggests that reversing is partly, but not fully, handled. servoMixer in mixer.c seems to handle reversing but then it calls triServoMixer which doesn't handle it. I guess that means it can work when the yaw is almost 0 but may behave strange when giving more command. Such as changing pitch at a yaw command.

@pqueiros , adding more checks may not be the best way. It's probably better to move the reverse correction to just before pwmWriteServo in pwm_output.c (or close to that), but I don't know enough to see if there may be side effects. Where I come from, such knowledge about the external specifics should stay far away from the core logic.

pqueiros commented 8 years ago

@Bengt-M I agree to the extend that the mixer does not need to know about reverse details. Nevertheless this was the least intrusive way I could think in 1h :).

@PProvost without checking the code I think "Direction and rate" to "-100%" will not affect the servo tricopter_mixer and servo estimator and the thrust compensation (and all the other good stuff introduced in Triflight) would most likely be done in the wrong direction.

lkaino commented 8 years ago

The challenging part here is that tricopter mixer takes as an input the output from the normal servo mixer. Tricopter mixer assumes that it is always working with the normal servo direction values. The problem is that normal servo mixer applies the reversing before passing the value to tricopter mixer.

IMO currently correct fix would be to not reverse the direction before passing to tricopter mixer. The direction needs to be reversed only after tricopter mixer. I think the output needs to be mirrored around 1500, but I'm not completely sure, this part messes with my head a bit, would need to test with a reversed servo to be sure :D.

The correct way to implement the tricopter servo mixer would be to pass the PID output [-1000,1000] directly to it, without passing it through the normal mixer first. Then the reversing can be applied after mixing. This needs to be done at some point.

pqueiros commented 8 years ago

@lkaino sounds like a good plan! I also noticed that the tricopter mixer spends its life translating the angle reference/servo position from the absolute value to the relative value in relation to the middle position, almost all the functions do something like this. The current mixer also works with values around the middle position and only at the end translates them to absolute values. If tricopter servo uses directly the PIDs or the servo command relative to the middle position all this mapping can be thrown away. At least this is my current understanding.

Bengt-M commented 8 years ago

There is a lot of legacy in this code base from several sources so it's a pretty amazing result after all, but I am missing some of the engineering practices I'm used to (from building naval fire control).

As said above, external specifics like this should be handled very close to the output port. So the old mixer is also wrong in my view. Neither of them need to know.

Along the same practices, it would be nice someday with just one representation for angle values but changing that would have big impact. Anyway, I agree that passing through with no converting sounds good.

lkaino commented 8 years ago

This might be easier to implement now without the normal servo mixer in picture. Maybe just need to reverse the output of tricopter mixer.

lkaino commented 8 years ago

Please test the new alpha release where this is fixed: 0.5 alpha 1

Ga1j1n commented 8 years ago

Can't test as sparky target hex disables VCP when flashed, worked on beta 3, funny thing same thing happened when a friend built a binary with an attempted servo reverse fix for me from the current code base a few days ago.

Servo reversing now seems to work in Cleanflight 1.13.0 rc2

lkaino commented 8 years ago

Okay I anticipated that with the new compiler version. I'll take a fix from CF and compile a new one.

Ga1j1n commented 8 years ago

Fantastic, as soon as it's ready I'll flash it

lkaino commented 8 years ago

Okay, uploaded new hexes to the same release. Named ALPHA2.

Ga1j1n commented 8 years ago

Quick bench test, VCP fixed, Input from TX is now reversed (was correct before) tail correction against yaw still the same direction (i.e the wrong way sending it into a spin) will properly test fly in a minute

lkaino commented 8 years ago

You reversed the servo using thw smix command? 2.6.2016 20.34 "Ga1j1n" notifications@github.com kirjoitti:

Quick bench test, VCP fixed, Input from TX is now reversed (was correct before) tail correction against yaw still the same direction (i.e the wrong way sending it into a spin) will properly test fly in a minute

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/lkaino/Triflight/issues/11#issuecomment-223364353, or mute the thread https://github.com/notifications/unsubscribe/AGJU8Huwq0vtnv2cQx-NjbddpaFNOUPVks5qHxQMgaJpZM4HKj1c .

Ga1j1n commented 8 years ago

I applied smix reverse 5 2 r (reversed input) then I applied smix reverse 5 2 n (correct input)

Ga1j1n commented 8 years ago

Scratch that, just reapplied and test flew, PERFECT, thanks!

I'll take it out for a full flight test tomorrow, put a few packs through it, I can say it has a very locked in feel

lkaino commented 8 years ago

Okay good! I will merge this into the master soon then. I didn't flight test the correct way yet :)

zsoltm commented 8 years ago

It looks like servo reversing is forgotten after doing an unarmed tail tune. I had to issue smix reverse 5 2 r again, after I realised my tri is spinning on the floor because of that.