Release 2.2 is coming

This forum is dedicated to software development related to MultiWii.
It is not the right place to submit a setup problem.
Software download
Sebbi
Posts: 478
Joined: Sun Jul 08, 2012 1:08 am
Location: Germany
Contact:

Re: Release 2.2 is coming

Post by Sebbi »

So ... the shifting is the cause of this error? Because it is an unsigned value that is being shifted and the µC doesn't care for the 2-compliment (i.e. signed) format then? If so, this error was really introduced with r1343 and other "small optimisations" in that update should be tested if they broke anything.

User avatar
aBUGSworstnightmare
Posts: 115
Joined: Mon Jun 27, 2011 8:31 pm
Location: Munich, Germany

Re: Release 2.2 is coming

Post by aBUGSworstnightmare »

Sebbi wrote:...Debusal introduced these changes as "small optimisations" in r1343 according to git-blame


I wouldn't call it a 'small optimization'! It's a major one regarding code execution speed!

Take AVRStudio and a JTAG-ICE, write a small program which uses multiplication and division and bit shift operations (you need to use the same operants for the benchmarking!). Watch the CPU clock counter for each instruction, compare the values and you know what I'm talking about.

aBUGSworstnightmare

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

The signed value when shifted, removes the Sign. So the answer is, Cast it first, then shift.

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Hi aBUGSworstnightmare,
this is not a cast! 'x >> 3' is a simple bitshift-right operation. Here's a little example:

thank you, I understand about bitshift used as a shortcut for mul/div with power of 2s.
The casts I was referring to was the ones copterrichie introduced like this: int8_t() separately to the high- and lowbytes.

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

I just did a very quick test of this mod and it works: http://youtu.be/mj3jFr1YRjE

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Sebbi wrote:So ... the shifting is the cause of this error? Because it is an unsigned value that is being shifted and the µC doesn't care for the 2-compliment (i.e. signed) format then? If so, this error was really introduced with r1343 and other "small optimisations" in that update should be tested if they broke anything.

from what I understand, using the shift instead of proper division gives an error of '1' for negative values - I have no idea whether that matters for flight computation, but I doubt it. So if speed matters, then probably good to do.

The casts for the separate high and lowbytes before or-ing them together, like C. suggested, make things worse, sorry.

It seems simply or-ing the high and low byte together in the correct order into a 16bit signed variable does everything required, because the internal representation for int16 is twos complement as well. So once the bits are in their proper place, then all is well.

bottom line: '>>3" instead of '/8' intorduced error of '1' for negative values, apart from that the old and actual code handling the mpu data is correct.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

Stars112 wrote:
Stars112 wrote:
IS the SERIAL3W LCD in Work with the 32u4?
In one of the first DEVS (1317) it dosent work.
It should work, with serial signal on PIN 4 according to the code.


Thanks Alex for your answer.
But... The Pin4 is the D+ Pin from USB (Serial0). And in most configuration connect with USB.
Can we use the Serial1 from the 32U4 at Pin 20-RX/21-TX?


This is how it is coded for a promicro:
#define PINMODE_LCD DDRD |= (1<<2);
which is according to the pro micro spec D0, RX PIN.
You can choose any other PIN, you just have to traduce:
#define PINMODE_LCD DDRD |= (1<<2);
#define LCDPIN_OFF PORTD &= ~1;
#define LCDPIN_ON PORTD |= 1;

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

copterrichie wrote:
copterrichie wrote:There appears to be a bug in the MPU6050 code that introduces an error on the Pitch axis, details are posted here: viewtopic.php?f=17&t=2934&start=10


Here is the fix, 2's Complement.

Code: Select all

void ACC_getADC () {
  i2c_getSixRawADC(MPU6050_ADDRESS, 0x3B);
  ACC_ORIENTATION( ((int8_t(rawADC[0])<<8) | int8_t(rawADC[1]))/8 ,
                   ((int8_t(rawADC[2])<<8) | int8_t(rawADC[3]))/8 ,
                   ((int8_t(rawADC[4])<<8) | int8_t(rawADC[5]))/8 );
  ACC_Common();
}


This code doesn't work. Please look in the GUI.
ACC range should range [-512;+512] for each axis without shaking. You will see it's not the case at all.

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Alexinparis wrote:
This code doesn't work. Please look in the GUI.
ACC range should range [-512;+512] for each axis without shaking. You will see it's not the case at all.


Ok, no one can't claim I did not try but my VTOL sure flies much better with NO wild pitching.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

Hamburger wrote:Is it about the casts or about replacing the '>>3' with '/8'?
Anyone to confirm this fix? I would like to copy this and other tidbits into the dev tree.
Would not the same have to be applied for the gyro?

rawADC is uint8_t; accADC from the ACC_ORIENTATION macro is int16_t. I do not understand why either the casts or the division should make a difference - but then I am not a bitbanging developer. Anyone care to explain, please?


http://arduino.cc/en/Reference/Bitshift
When you shift x right by y bits (x >> y), and the highest bit in x is a 1, the behavior depends on the exact data type of x. If x is of type int, the highest bit is the sign bit, determining whether x is negative or not, as we have discussed above. In that case, the sign bit is copied into lower bits, for esoteric historical reasons:


This only trade off is the one you mentioned: a shift of one for negative values:
-234/8 = -29
-234>>3 = -30
but we can live this it

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

AndrejLV wrote:What is a source of constant drift if the bare board are placed on a table with disconnected motors? Arm the "motors", add throttle and we can observe slow constant decreasing of output magnitude on Rear L motor in time. In one two minutes it diverse with another motors ~100 sometimes.

- motors not connected, so it not linked to vibration
- it not depends on board orientation, so it not the problem of earth rotation.
- it not depends on rc sticks centre shift, DEADBAND 100 not eliminate this problem
- stable temperature and enviroment ..
- it present in all my boards, Crius and Nanowii, all with mpu6050.

Is it native sensor drift or software processing?


It's normal and explained here:
http://www.multiwii.com/faq#The_motor_o ... l_RC_input

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

Sebbi wrote:Whether it is 2-compliment or not doesn't matter in this case. Why? Because there is a bias calculated as the zero-value and subtracted from every acc measurement. The original problem (pitch moves when roll is greater than x degrees) comes from the atan functions calculating the actual degrees which do not work correctly .... which is a known bug, but nobody seems to care.

Code: Select all

  angle[ROLL]  =  _atan2(EstG.V.X , EstG.V.Z) ;
  angle[PITCH] =  _atan2(EstG.V.Y , EstG.V.Z) ;


This causes the problem ...

This code was updated as I cared... please read again the code
But the angle roll is still disturbed is pitch reaches 90 or -90.
question: what is roll when pitch is 90 deg ?

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

copterrichie wrote:
Sebbi wrote:Whether it is 2-compliment or not doesn't matter in this case. Why? Because there is a bias calculated as the zero-value and subtracted from every acc measurement. The original problem (pitch moves when roll is greater than x degrees) comes from the atan functions calculating the actual degrees which do not work correctly .... which is a known bug, but nobody seems to care.

Code: Select all

  angle[ROLL]  =  _atan2(EstG.V.X , EstG.V.Z) ;
  angle[PITCH] =  _atan2(EstG.V.Y , EstG.V.Z) ;


This causes the problem ...


I think it is a compounded problem that has existed for sometime now.


Here is my interpretation of the problem "Quadcopter tilts in wind in level mode":
Before the last rev1338, there was a huge LPF applied on ACC before the calcul of the angle.
I suppose this huge LPF was the cause of the tilt if the copter is too long inclined in a position.
There is now (in pre2.2) a small LPF which is used also of ALT code, and the complementary filter GYRO/ACC is a little bit increased.
I have some positive feedbacks and the "tilt effect" seems to be much more acceptable.

klaudius666
Posts: 14
Joined: Thu Apr 05, 2012 5:59 pm
Contact:

Re: Release 2.2 is coming

Post by klaudius666 »

Hi

Please, Just for my curiosity about arduino compilator optim

with uint8 toto
if i write :

a=toto/8

does the compiler optimize and do unsigned shifting of 3 as 8 is a power of two ?

same question for int8, it does signed shift ?

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

aBUGSworstnightmare wrote:
Sebbi wrote:...Debusal introduced these changes as "small optimisations" in r1343 according to git-blame


I wouldn't call it a 'small optimization'! It's a major one regarding code execution speed!

Take AVRStudio and a JTAG-ICE, write a small program which uses multiplication and division and bit shift operations (you need to use the same operants for the benchmarking!). Watch the CPU clock counter for each instruction, compare the values and you know what I'm talking about.

aBUGSworstnightmare

Thank you for understanding this point :)
taking each operation individually, the optimization is effectively huge.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

copterrichie wrote:
Alexinparis wrote:
This code doesn't work. Please look in the GUI.
ACC range should range [-512;+512] for each axis without shaking. You will see it's not the case at all.


Ok, no one can't claim I did not try but my VTOL sure flies much better with NO wild pitching.

Did you test both code with pre2.2 ?
The code you suggested isn't correct and degrades the acc values, but the angle can still be extracted from those values.
That may explain it still works.

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Alexinparis wrote:Did you test both code with pre2.2 ?
The code you suggested isn't correct and degrades the acc values, but the angle can still be extracted from those values.
That may explain it still works.


Alex, we had this discussion before with the MMA7455, I have been using that code for a very long time. Yes, I tested the modifications to R1277 and Pre2.2. Same situation. This Bug does not appear all the time, depending upon the calibration offset and Angle of the copter will cause an extreme shift, like going from 127 to 128. with two's complement, that is one extreme to the other, Positive 127 to a negative -128.

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Thanks for the consideration, I will just copy and paste for future versions. No need to implement this change on my behalf.

Sebbi
Posts: 478
Joined: Sun Jul 08, 2012 1:08 am
Location: Germany
Contact:

Re: Release 2.2 is coming

Post by Sebbi »

Alexinparis wrote:
Hamburger wrote:Is it about the casts or about replacing the '>>3' with '/8'?
Anyone to confirm this fix? I would like to copy this and other tidbits into the dev tree.
Would not the same have to be applied for the gyro?

rawADC is uint8_t; accADC from the ACC_ORIENTATION macro is int16_t. I do not understand why either the casts or the division should make a difference - but then I am not a bitbanging developer. Anyone care to explain, please?


http://arduino.cc/en/Reference/Bitshift
When you shift x right by y bits (x >> y), and the highest bit in x is a 1, the behavior depends on the exact data type of x. If x is of type int, the highest bit is the sign bit, determining whether x is negative or not, as we have discussed above. In that case, the sign bit is copied into lower bits, for esoteric historical reasons:


This only trade off is the one you mentioned: a shift of one for negative values:
-234/8 = -29
-234>>3 = -30
but we can live this it


But ... assume the MPU is returning values between -4096 and 4096. Since rawADC is uint8_t (accADC is int16_t) i am not sure what would be inside those variables. Since the documentation says it's 2-complements -4096 would be 11110000 and 00000000. Shifted to the right by 3 this results in 00011110 00000000 which is no longer a negative number when cast/copied to accADC (it is 7680). Am I wrong? If rawADC would have been a signed variable the shifting would result in 11111110 00000000 which is -512. I am sure something is wrong with my assumption here ... should confirm with code ;-)

Alexinparis wrote:
Sebbi wrote:Whether it is 2-compliment or not doesn't matter in this case. Why? Because there is a bias calculated as the zero-value and subtracted from every acc measurement. The original problem (pitch moves when roll is greater than x degrees) comes from the atan functions calculating the actual degrees which do not work correctly .... which is a known bug, but nobody seems to care.

Code: Select all

  angle[ROLL]  =  _atan2(EstG.V.X , EstG.V.Z) ;
  angle[PITCH] =  _atan2(EstG.V.Y , EstG.V.Z) ;


This causes the problem ...

This code was updated as I cared... please read again the code
But the angle roll is still disturbed is pitch reaches 90 or -90.
question: what is roll when pitch is 90 deg ?


When turning each axis individually the above works. But observe what happens if you tilt both axis equally. The GUI will show 90 degrees pitch and 90 degrees roll when the copter stands on its side (well on one of the arms if X configuration), which are not the correct values in this situation.

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

maybe a little experiment helps - you would have to check my implementation of a hand crafted twos_complement_to_binary :

Code: Select all

// experiment - try to find example values, for which the two implementations
// of acc codes would produce different results
void setup() {
   int n = 0;
   static int16_t r1, r2;
   Serial.begin(115200);
   for (uint8_t i=0; i< 255; i++) { // emulate rawADC[2] - the high byte from mpu6050
      for (uint8_t j=0; j<255; j++) { // emulate rawADC[3] - the low byte from mpu6050
         r1 = ((i<<8) | j) >>3;
         r2 = twos_complement_to_binary(i, j) >>3;
         if (r1 != r2) {
            Serial.print(i, DEC); Serial.print(' '); Serial.print(j, DEC); Serial.print(" : ");
            Serial.print(i, BIN); Serial.print(' '); Serial.print(j, BIN); Serial.print(" -:- ");
            Serial.print(r1, DEC); Serial.print(' '); Serial.print(r2, DEC); Serial.print(" : ");
            Serial.print(r1, BIN); Serial.print(' '); Serial.print(r2, BIN);
            Serial.print("\n");
            n++;
         }
      }
   }
   Serial.print("\n== end == count of different results : "); Serial.print(n);
   Serial.print("\n");
}

void loop() {
}

int16_t twos_complement_to_binary(uint8_t hi, uint8_t lo) {
   uint16_t t = (hi<<8) | lo; // combine to 16bit sized twos complement
   if (hi & (1<<7) ) {
      // highest bit set, so negative number
      return -((uint16_t)1 + ~t);
   } else {
      // positive number; nothing to be done
      return (int16_t)t;
   }
}

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Hamburger, I am reviewing your code and I don't believe it works that way. Will see if I can modify it to the way I think it does work.

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Hamburger, I was unable to modify your script to illustrate the casting issue however, I believe this very simply script does.

Code: Select all

void setup() {                
  Serial.begin(115200);
  Serial.println(uint8_t(127), BIN);
  Serial.println(int8_t(127),BIN);
  Serial.println(uint8_t(128), BIN);
  Serial.println(int8_t(128),BIN);
}

// the loop routine runs over and over again forever:
void loop() {
 
}


Screen output.

1111111
1111111
10000000
11111111111111111111111110000000

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Added one more line for clarity.

Code: Select all

void setup() {                
  Serial.begin(115200);
  Serial.println(uint8_t(127), BIN);
  Serial.println(int8_t(127),BIN);
  Serial.println(uint8_t(128), BIN);
  Serial.println(int8_t(-127), BIN); // <-Added
  Serial.println(int8_t(128),BIN);
 
}

// the loop routine runs over and over again forever:
void loop() {
 
}


1111111
1111111
10000000
11111111111111111111111110000001
11111111111111111111111110000000


And because each Accelerator even from the same manufacture has a different calibration offset, this problem will not happen to every situation.
Last edited by copterrichie on Wed Feb 27, 2013 6:23 pm, edited 1 time in total.

postaL
Posts: 36
Joined: Thu Oct 04, 2012 10:08 pm

Re: Release 2.2 is coming

Post by postaL »

copterrichie wrote:Added one more line for clarity.

Code: Select all

void setup() {                
  Serial.begin(115200);
  Serial.println(uint8_t(127), BIN);
  Serial.println(int8_t(127),BIN);
  Serial.println(uint8_t(128), BIN);
  Serial.println(int8_t(-127), BIN); // <-Added
  Serial.println(int8_t(128),BIN);
 
}

// the loop routine runs over and over again forever:
void loop() {
 
}


1111111
1111111
10000000
11111111111111111111111110000001
11111111111111111111111110000000



theCLAN is watching you...

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Good, I love an audience. :mrgreen:

Sebbi
Posts: 478
Joined: Sun Jul 08, 2012 1:08 am
Location: Germany
Contact:

Re: Release 2.2 is coming

Post by Sebbi »

I tested around and

Code: Select all

  ACC_ORIENTATION( ((rawADC[0]<<8) | rawADC[1])>>3 ,
                   ((rawADC[2]<<8) | rawADC[3])>>3 ,
                   ((rawADC[4]<<8) | rawADC[5])>>3 );


looks like the correct way to do it. It works since ACC_ORIENTATION takes an int16_t argument so the bitshifting <<8 works and the 2-complements fit naturally in this data type. ">>3" also works. Confirmed that on my NanoWii (MPU6050) board.

Now about the roll/pitch angle. It looks like this has been fixed in a recent release (wasn't following development for a long time now)? Pitch seems to be ok (values between -90° and +90°) and roll also behaves like expected. What I am not sure about is the fact that the roll angle is used in algorithms (level/horizon) since this is not an angle measured in earth frame (as compared to heading and pitch). But anyways ... as long as copter/airplanes aren't flying with pitch values near 90° everything seems ok ;-)

copterrichie
Posts: 2261
Joined: Sat Feb 19, 2011 8:30 pm

Re: Release 2.2 is coming

Post by copterrichie »

Sebbi wrote:I tested around and

Code: Select all

  ACC_ORIENTATION( ((rawADC[0]<<8) | rawADC[1])>>3 ,
                   ((rawADC[2]<<8) | rawADC[3])>>3 ,
                   ((rawADC[4]<<8) | rawADC[5])>>3 );


looks like the correct way to do it. It works since ACC_ORIENTATION takes an int16_t argument so the bitshifting <<8 works and the 2-complements fit naturally in this data type. ">>3" also works. Confirmed that on my NanoWii (MPU6050) board.

Now about the roll/pitch angle. It looks like this has been fixed in a recent release (wasn't following development for a long time now)? Pitch seems to be ok (values between -90° and +90°) and roll also behaves like expected. What I am not sure about is the fact that the roll angle is used in algorithms (level/horizon) since this is not an angle measured in earth frame (as compared to heading and pitch). But anyways ... as long as copter/airplanes aren't flying with pitch values near 90° everything seems ok ;-)


Bingo, this is what I MISSED. Sorry

static int16_t gyroADC[3],accADC[3],accSmooth[3],magADC[3];

User avatar
mbrak
Posts: 136
Joined: Sat Dec 03, 2011 8:08 pm
Location: Germany, Lemgo

Re: Release 2.2 is coming

Post by mbrak »

Hi

i thought i had fixed the problem with telemetry display (page7) on my OLED. a error appeared with the code i posted.

after some pm with hamburger he finaly fixed the problem. thanks a lot hamburger!!

gps_ok.JPG
(29.08 KiB) Not downloaded yet


i am happy with this :)

br michael

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Will checkin later the change

Woppit
Posts: 22
Joined: Tue Jul 17, 2012 11:47 pm

Re: Release 2.2 is coming

Post by Woppit »

Hi all, just tried to fine tune the voltage display using the following

#define VBATSCALE 131 // (*) change this value if readed Battery voltage is different than real voltage

is this the right thing to adjust? if so it does not make any difference to my voltage reading, went from 131 to 100 it stayed the same!
did I do the right thing? as this is the first time I have used this function, or is there something wrong?

thanks
Shane

Woppit
Posts: 22
Joined: Tue Jul 17, 2012 11:47 pm

Re: Release 2.2 is coming

Post by Woppit »

DOH! ok its my fault!

all sorted

thanks
Shane

Peter
Posts: 82
Joined: Mon Jun 11, 2012 2:09 pm

Re: Release 2.2 is coming

Post by Peter »

Woppit wrote:Hi all, just tried to fine tune the voltage display using the following

#define VBATSCALE 131 // (*) change this value if readed Battery voltage is different than real voltage

is this the right thing to adjust? if so it does not make any difference to my voltage reading, went from 131 to 100 it stayed the same!
did I do the right thing? as this is the first time I have used this function, or is there something wrong?

thanks
Shane


I had the same problem. You have to reset the values on the GUI.

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Peter wrote:I had the same problem. You have to reset the values on the GUI.

or you can follow the '*' and do as described in the note 1) at top of config.h and tune parameters via LCD/Oled or the serial monitor of arduino IDE.

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Wasted memory from disabled features needs cleaning.

The symptom
Many features can be turned on/off via #define.
But even when turned off, often times the corresponding variables still get used somewhere in the code or in MSP messages without meaningful content - thus waisting dire memory resources.

the cause
This effect can best be seen with MAG, BARO or lately RSSI. (same may apply to other features, I merely picked these to entertain you). The corresponding features are turned on/off via #defines, but some mag, baro, rssi variables are still used.

how to hunt these down:
To find out, configure a bare quad without mag,baro, rssi; then uncomment the variables' declarations for mag*, baro*, rssi* in MultiWii.ino. You will find it breaks the compile, because these variables are used for MSPs and elsewhere, even though no mag,baro, rssi are present.

what to do
all developers please track down such occurrences in the code you have contributed or maintain or otherwise feel comfortable to deal with.

Maybe in the future it would be good to have #ifdef FEATURE_X around the variables' declarations. That way unwanted dependencies and usage become obvious.
Last edited by Hamburger on Thu Feb 28, 2013 2:21 pm, edited 1 time in total.

Woppit
Posts: 22
Joined: Tue Jul 17, 2012 11:47 pm

Re: Release 2.2 is coming

Post by Woppit »

Thanks for the info guys, think it was a case of to late at night and not reading thing through properly!

Thanks for the replies.

Shane

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

Hamburger wrote:Wasted memory from disabled features needs cleaning.

The symptom
Many features can be turned on/off via #define.
But even when turned off, often times the corresponding variables still get used somewhere in the code or in MSP messages without meaningful content - thus waisting dire memory resources.

the cause
This effect can best be seen with MAG, BARO or lately RSSI. (same may apply to other features, I merely picked these to entertain you). The corresponding features are turned on/off via #defines, but some mag, baro, rssi variables are still used.

how to hunt these down:
To find out, configure a bare quad without mag,baro, rssi; then uncomment the variables' declarations for mag*, baro*, rssi* in MultiWii.ino. You will find it breaks the compile, because these variables are used for MSPs and elsewhere, even though no mag,baro, rssi are present.

what to do
all developers please track down such occurrences in the code you have contributed or maintain or otherwise feel comfortable to deal with.

Maybe in the future it would be good to have #ifdef FEATURE_X around the variables' declarations. That way unwanted dependencies and usage become obvious.



I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release

User avatar
dramida
Posts: 473
Joined: Mon Feb 28, 2011 12:58 pm
Location: Bucharest
Contact:

Re: Release 2.2 is coming

Post by dramida »

Here is MWC pre2.2 FPV tricopter with minimOSD and camera gimbal, performing various maneuvers.


http://youtu.be/fbEMrn7NMhU

In a later flight, at about 300 m height, when i engaged Alt Hold, the copter fellt out from the sky with 9m/s ( i had the OSD video recording). The behaviour is reproductible as i did it after.

More after uploading the video.

User avatar
alll
Posts: 220
Joined: Fri Dec 07, 2012 9:53 am

Re: Release 2.2 is coming

Post by alll »

Alexinparis wrote:I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release


no! in modern editors all unused code is automatically hided, easy to read => less mistakes / dependencies errors. But then, you need to make the effort once to rewrite all ".ino" the proper way, structured with headers ad c files...

please ;) ;) ;) :| :| :| , let this be a requirement for next releases, so that the code can better evolute and easier to maintain.

look how setup() is short when #define COPTERTEST 1

manu
Attachments
Screen shot 2013-02-28 at 22.14.33.png
Screen shot 2013-02-28 at 22.04.11.png
Screen shot 2013-02-28 at 22.03.18.png

jy0933
Posts: 180
Joined: Wed Jun 27, 2012 4:24 pm

Re: Release 2.2 is coming

Post by jy0933 »

will be useful if there is a aprox converting factor from old PID to new PID.. so we can have a rough idea what number to shoot for... (and hope the algorithm wont change again in the future since it gets confusing easily)

User avatar
Hamburger
Posts: 2578
Joined: Tue Mar 01, 2011 2:14 pm
Location: air
Contact:

Re: Release 2.2 is coming

Post by Hamburger »

Alexinparis wrote:I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".

agreed. I would assume it being a safe bet to take a m328p or 32u4 with gyro and acc as a basis.
Anyway, if something has to be done it is after 2.2 release

Cleaning up memory waste from improperly capsulated features/code should be part of the consolidation phase prior to release.

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Release 2.2 is coming

Post by EOSBandi »

alll wrote:
Alexinparis wrote:I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release


no! in modern editors all unused code is automatically hided, easy to read => less mistakes / dependencies errors. But then, you need to make the effort once to rewrite all ".ino" the proper way, structured with headers ad c files...

please ;) ;) ;) :| :| :| , let this be a requirement for next releases, so that the code can better evolute and easier to maintain.

look how setup() is short when #define COPTERTEST 1

manu



If you use Visual Studio you will get all benefits of a modern IDE and don't have to rewrite to C and headers.... :D

User avatar
alll
Posts: 220
Joined: Fri Dec 07, 2012 9:53 am

Re: Release 2.2 is coming

Post by alll »

Pleaaase, lets make the effort once, and it will compile in all modern ide's, even in the Arduinio IDE.

Lapino
Posts: 84
Joined: Tue Aug 16, 2011 10:01 am

AW: Release 2.2 is coming

Post by Lapino »

Modern and arduino IDE in one sentence...XD
Sry but nothing compares to visual studio ;)

jy0933
Posts: 180
Joined: Wed Jun 27, 2012 4:24 pm

Re: Release 2.2 is coming

Post by jy0933 »

Alexinparis wrote:
Hamburger wrote:Wasted memory from disabled features needs cleaning.

The symptom
Many features can be turned on/off via #define.
But even when turned off, often times the corresponding variables still get used somewhere in the code or in MSP messages without meaningful content - thus waisting dire memory resources.

the cause
This effect can best be seen with MAG, BARO or lately RSSI. (same may apply to other features, I merely picked these to entertain you). The corresponding features are turned on/off via #defines, but some mag, baro, rssi variables are still used.

how to hunt these down:
To find out, configure a bare quad without mag,baro, rssi; then uncomment the variables' declarations for mag*, baro*, rssi* in MultiWii.ino. You will find it breaks the compile, because these variables are used for MSPs and elsewhere, even though no mag,baro, rssi are present.

what to do
all developers please track down such occurrences in the code you have contributed or maintain or otherwise feel comfortable to deal with.

Maybe in the future it would be good to have #ifdef FEATURE_X around the variables' declarations. That way unwanted dependencies and usage become obvious.



I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release


that sounds like a going back.. every time one thinks of new feature needs to reflash firmware... the so-called optional should be on GUI enable or disable... or find a proper way that people can do the update in GUI. It should be simple and noob-friendly...

rbirdie001
Posts: 178
Joined: Fri Apr 01, 2011 10:32 pm
Location: Czech Republic, Prague

Re: Release 2.2 is coming

Post by rbirdie001 »

Hi!
I'm unable to compile 2.2 pre if I set in config.h Airplane. I'm getting these errors:
MultiWii.cpp: In function 'void servos2Neutral()':
MultiWii:845: error: 'i' was not declared in this scope
Maybe there is some change in necessary default settings which I missed but at a time I didn't find where is the problem.
Thanks for your works, coders!
Roman

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

dramida wrote:Here is MWC pre2.2 FPV tricopter with minimOSD and camera gimbal, performing various maneuvers.


http://youtu.be/fbEMrn7NMhU

In a later flight, at about 300 m height, when i engaged Alt Hold, the copter fellt out from the sky with 9m/s ( i had the OSD video recording). The behaviour is reproductible as i did it after.

More after uploading the video.


It's maybe an overflow problem somewhere.
Hopefully, it seems to appear only in extreme conditions (300m). we will have to look closer at this.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

alll wrote:
Alexinparis wrote:I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release


no! in modern editors all unused code is automatically hided, easy to read => less mistakes / dependencies errors. But then, you need to make the effort once to rewrite all ".ino" the proper way, structured with headers ad c files...

please ;) ;) ;) :| :| :| , let this be a requirement for next releases, so that the code can better evolute and easier to maintain.

look how setup() is short when #define COPTERTEST 1

manu


This is something that will change for next release, I promise ;)
annex .ino -> .c+.h or .h only
most probably with a structure like this:
http://code.google.com/p/mwc-ng/source/ ... comp%2Fsrc

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

jy0933 wrote:will be useful if there is a aprox converting factor from old PID to new PID.. so we can have a rough idea what number to shoot for

it's just a proportion
for instance: defaut ROLL P was 4.0, and it is now 3.3
you just have to calculate your_old_value x 3.3 / 4.0

(and hope the algorithm wont change again in the future since it gets confusing easily)

multiwii is not a plug and play software. The confusion is here I think.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

jy0933 wrote:
Alexinparis wrote:
Hamburger wrote:Wasted memory from disabled features needs cleaning.

The symptom
Many features can be turned on/off via #define.
But even when turned off, often times the corresponding variables still get used somewhere in the code or in MSP messages without meaningful content - thus waisting dire memory resources.

the cause
This effect can best be seen with MAG, BARO or lately RSSI. (same may apply to other features, I merely picked these to entertain you). The corresponding features are turned on/off via #defines, but some mag, baro, rssi variables are still used.

how to hunt these down:
To find out, configure a bare quad without mag,baro, rssi; then uncomment the variables' declarations for mag*, baro*, rssi* in MultiWii.ino. You will find it breaks the compile, because these variables are used for MSPs and elsewhere, even though no mag,baro, rssi are present.

what to do
all developers please track down such occurrences in the code you have contributed or maintain or otherwise feel comfortable to deal with.

Maybe in the future it would be good to have #ifdef FEATURE_X around the variables' declarations. That way unwanted dependencies and usage become obvious.



I think some of function or sensors should be considered as "standard" (like gyro ar acc even if not used) for multiwii, and some other function should be considered as "optional".
Everything could be defined everything as "optional", and it would result in a very small flash code and probably a little bit efficient.
But I fear the code becomes difficult to read using this strict way.
There are already too much #define possibilities in config.h without a proper documentation.
Anyway, if something has to be done it is after 2.2 release


that sounds like a going back.. every time one thinks of new feature needs to reflash firmware... the so-called optional should be on GUI enable or disable... or find a proper way that people can do the update in GUI. It should be simple and noob-friendly...


This is also another confusion.
multiwii began on a 328p proc, and activating everything at the compilation would just be too big for this proc.
ok, we could follow this way with 32bit proc of mega2560 and remove every #if defined
I think a clean code design could probably conciliate the 2 approaches.

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: Release 2.2 is coming

Post by Alexinparis »

I updated the pre2.2 version today, and wrote a release note (in the first post)
If you fell something is missing (a functionality or a credit), it's not intentional.
don't hesitate to suggest some rephrasing.

Post Reply