ButterFlight / butterflight

GNU General Public License v3.0
106 stars 26 forks source link

Butterflight 3.5.1 [ Release Candidate ] #102

Closed kidBrazil closed 6 years ago

kidBrazil commented 6 years ago

This branch represents our Release Candidate features that we want to get tested.

adrianmiriuta commented 6 years ago

@rs2k @orneryd

I think this is not correct accelerationTrims->raw[X] = (a[X] + halfCycles) / CALIBRATING_ACC_CYCLES; it should be accelerationTrims->raw[X] = a[X] / CALIBRATING_ACC_CYCLES;

It is only a positive bias to a integer division ... It does not make any sense to me. Moving calibration code to float would make more sense, I think.

adrianmiriuta commented 6 years ago

@kidBrazil @rs2k

Is a HEX available for 3.5.2 RC for HELIOSPRING ?

adrianmiriuta commented 6 years ago

@kidBrazil

sorry found the comment

why ? Revert "Merge pull request #112 from adrianmiriuta/Butterflight-RC-351"

kidBrazil commented 6 years ago

@adrianmiriuta - I merged your branch in first, then Ornery's and it caused a conflict. Turns out Ornery had made the same fix you did. So I reverted, merged his in first then tried to merge your fix but was no longer necessary.

Nothing you did wrong, just my fuck up with the merge order.

adrianmiriuta commented 6 years ago

@robert-b I think you do not know what you are talking about !

quaternionNormalize restores the length of the quaternion to 1.0f so that it is a valid rotation quaternion ! initialization is not needed ! this would be a continuity break !

robert-b commented 6 years ago

:-) maybe the callee of the callee. doing any calculation with a quaternion which can't be normalized is really smart. i should really stop here ...

robert-b commented 6 years ago

a unit quaternion represents a rotation in 3D space. so far ok? any other (i.e.: non-unit) quaternion does not represent a rotation. and you are going to do calculations with a non-unit quaternion... you know what a reverse function is in math? silly it is.

adrianmiriuta commented 6 years ago

@robert-b I think you saw that code in Inav, there are a lot of problematic implementations on the NET! It is a wrong implementation from "digitalentity" ... your interpretation is wrong. please study quaternions.

adrianmiriuta commented 6 years ago

@robert-b

lets put it another way You have a algo that handles addition you put in 1+1 he answers 3 you do not believe it ... and initialize it to 0 besides that both answers are wrong ... it is bad programming practice to paper over errors in algorithms if there is a error it should be observable so that it can be fixed, not painted over so that it cannot be seen !

don't you think ?

robert-b commented 6 years ago

i have not touched the sensor fusion from inav - i didn't even look at it lately. but i did it two years ago and complained - do not know what has been changed. before i fly inav i will do so. again - if a quaternion can't be normalized then don't do anything with it. bad data can come from anywhere.

robert-b commented 6 years ago

check: https://github.com/librepilot/LibrePilot/blob/next/flight/modules/Attitude/attitude.c at the end a non-unit quaternion will be corrected. you can do anything with the quaternion inbetween.

but at the very end it has to be a unit quaternion. if it is not a valid description then you crash.

robert-b commented 6 years ago

more than 20 minutes and no answer ... look at applyAccError()

when is it impossible to apply the correction? just thinking.

adrianmiriuta commented 6 years ago

@robert-b I see ... "Librepilot" implementation is also not desirable !

We do normalize because trough standard taylor series integration (that is not a valid quaternion operation) we get small deviations of the quaternion length so modulus(quaternion) != 1 trough normalization we calculate a tangent to the S3 hypersphere, and bring it back to a unit quaternion (and introduce small error on all axes...) normal integral error is about 1+- 0.0000001 .(depends on the Acc and Gyro cycle)

If a modulus(quaternion) is less than 0.999 (like the IMUF code at the moment) Than the algo should really be looked at ...
(not painted over)

The quad will fall out of the sky even when you init the Attitude quaternion to roll,pitch,yaw = 0 It will take several seconds ,(and many valid acc readings) to converge from RPY=0 to the real position. Meanwhile you will crash !

robert-b commented 6 years ago

the comment on libre pilot is accepted. openpilot and all derivatives have a big blunder.

but first we clear up applyAccError() :-) joshua bardwell: today you can learn something.

adrianmiriuta commented 6 years ago

@robert-b

look at applyAccError()

when is it impossible to apply the correction? just thinking.

what do you mean ?

robert-b commented 6 years ago

when will the calculation fail. this is the question.

adrianmiriuta commented 6 years ago

@robert-b what do you mean with fail ?

robert-b commented 6 years ago

a non unit quaternion. will happen during a free fall scenario. in this case you shouldn't do anything with the quaternion. in this situation you can forget any classic sensor fusion. the situation is just not defined. once you have a nan somewhere you won't get rid of it.

adrianmiriuta commented 6 years ago

@robert-b Yes , in a case where the sensor fusion fails and you will get a highly degraded or NAN quaternion. than you can "set debug_mode=IMU" and find out ... and hopefully correct the algo! There is no advantage in initializing it at that moment.

robert-b commented 6 years ago

i refuse to fly this piece of sw. from time to time i create my own problems. that's enough. your piece of sw - your work!

adrianmiriuta commented 6 years ago

@robert-b well ... as you wish.

I fly with this for about 4 months, i never had modulus(quaternion) != 1

orneryd commented 6 years ago

closing in favor of the odin branch.