Let's try to release 2.1: first try based on r949

[Tommie]

ps.: Tommie....

Why are you summoning me?

The f. bitfield works exactly like the variables before, the original code was:

Code: Select all

   //===================== GPS fix notification handling =====================
   #if GPS
   if ((GPSModeHome || GPSModeHold) && !GPS_fix){    //if no fix and gps funtion is activated: do warning beeps.

So what changed?

[Tommie]

It took me around five seconds to find the appropiate lines of code:

Code: Select all

      static uint8_t GPSNavReset = 1;
      if (f.GPS_FIX && GPS_numSat >= 5 ) {
        if (!rcOptions[BOXGPSHOME] && !rcOptions[BOXGPSHOLD] )
          {    //Both boxes are unselected
            if (GPSNavReset == 0 ) {
               GPSNavReset = 1;
               GPS_I2C_command(I2C_GPS_COMMAND_STOP_NAV,0);
            }
          }
        if (rcOptions[BOXGPSHOME]) {
         if (!f.GPS_HOME_MODE)  {
            f.GPS_HOME_MODE = 1;
            GPSNavReset = 0;
            GPS_I2C_command(I2C_GPS_COMMAND_START_NAV,0);        //waypoint zero
          }
        } else {
          f.GPS_HOME_MODE = 0;
        }
        if (rcOptions[BOXGPSHOLD]) {
          if (!f.GPS_HOLD_MODE & !f.GPS_HOME_MODE) {
            f.GPS_HOLD_MODE = 1;
            GPSNavReset = 0;
            GPS_I2C_command(I2C_GPS_COMMAND_POSHOLD,0);
          }
        } else {
          f.GPS_HOLD_MODE = 0;
        }
      }

So the actual flag variables won't change unless you have a valid GPS fix; they indicate a _state_. rcOptions indicates the intention to enter that mode. after reviewing my repository, this has always been the case, so I don't understand the confusion and in what way I am supposed to be responsible?

[jevermeister]

The bitfied variables are only set if I have a GPS Fix:

Code: Select all

 if (f.GPS_FIX && GPS_numSat >= 5 ) {
        if (rcOptions[BOXGPSHOME]) {
          if (!f.GPS_HOME_MODE)  {
            f.GPS_HOME_MODE = 1;
            GPS_set_next_wp(&GPS_home[LAT],&GPS_home[LON]);
            nav_mode    = NAV_MODE_WP;
          }
        } else {
          f.GPS_HOME_MODE = 0;
        }...

So what sense does it make to check if the bits are set and the fix not? This would never be the case.


This was not tested but commited.

So back to:

Code: Select all

  #if GPS
  if ((rcOptions[BOXGPSHOME] || rcOptions[BOXGPSHOLD]) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

Otherwise it won't work,


OK?

Regards,
Nils

[Tommie]

The bitfied variables are only set if I have a GPS Fix:

Yes, that is what I just said.

So what sense does it make to check if the bits are set and the fix not? This would never be the case.

I don't know? I'm not the one who wrote that code.

This was not tested but commited.

Probably.

So back to:

Code: Select all

  #if GPS
  if ((rcOptions[BOXGPSHOME] || rcOptions[BOXGPSHOLD]) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

This is not back. It changes the semantics of the code. I already pointed out that the code has always been implemented the way you consider faulty - that might be the case, but the semantics have always been that way, even before the changes implementing the bitfield members (f.*).

(https://code.google.com/p/multiwii/sour ... uzzer.ino#)

[EOSBandi]

deleted

[EOSBandi]

And another one:

in buzzer.ino:

Code: Select all

  //===================== GPS fix notification handling =====================
  #if GPS
  if ((f.GPS_HOME_MODE || f.GPS_HOLD_MODE) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

will not work, because f.GPS_HOME_MODE and f.GPS_HOLD_MODE are never set if f.GPS_FIX is 0.

I don't get these f.**** variables at all..

I will revert to this:

Code: Select all

  //===================== GPS fix notification handling =====================
  #if GPS
  if ((rcOptions[BOXGPSHOME] || rcOptions[BOXGPSHOLD]) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

It is only to indicate if I want to use GPS and have no fix yet.

Nils

ps.: Tommie....

Nils, the flags are set by the box options and they are used to prevent entering poshold or rth mode without fix, and indicates that the necessry initialisations are done.
For warning you can use the box options, but the poshold and rth logic is based on these flags...

Take it like this : the box options are indicating that we are trying to enter into poshold or rth and the flags are indicating that we were ENTERED into that mode.
So buzzer.ino GPS warning was faulty from the start but it has nothing to do with the flags....

[jevermeister]

And another one:

in buzzer.ino:

Code: Select all

  //===================== GPS fix notification handling =====================
  #if GPS
  if ((f.GPS_HOME_MODE || f.GPS_HOLD_MODE) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

will not work, because f.GPS_HOME_MODE and f.GPS_HOLD_MODE are never set if f.GPS_FIX is 0.

I don't get these f.**** variables at all..

I will revert to this:

Code: Select all

  //===================== GPS fix notification handling =====================
  #if GPS
  if ((rcOptions[BOXGPSHOME] || rcOptions[BOXGPSHOLD]) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;    
  }else{
    warn_noGPSfix = 0;
  }
  #endif

It is only to indicate if I want to use GPS and have no fix yet.

Nils

ps.: Tommie....

Nils, the flags are set by the box options and they are used to prevent entering poshold or rth mode without fix, and indicates that the necessry initialisations are done.
For warning you can use the box options, but the poshold and rth logic is based on these flags...

Take it like this : the box options are indicating that we are trying to enter into poshold or rth and the flags are indicating that we were ENTERED into that mode.
So buzzer.ino GPS warning was faulty from the start but it has nothing to do with the flags....

Andras, Tommie,
I know that but someone changed the code in buzzer.ino to check the bitfield not the boxes.
Just wanted to let you guys know that I fixed this error. And: TESTED IT!

Nils

[EOSBandi]

Great... thank you ! I never checked the buzzer code, since I don't use it on my copters... But good to know, it's working now.

[Tommie]

Andras, Tommie,
I know that but someone changed the code in buzzer.ino to check the bitfield not the boxes.

No. The code never checked the boxes. I posted the relevant lines above.

But through the magic of version control, I can trace back the history of this code (the command is aptly named 'git blame'):

Up until your patch, the code looked like this:

Code: Select all

  //===================== GPS fix notification handling =====================
  #if GPS
  if ((f.GPS_HOME_MODE || f.GPS_HOLD_MODE) && !f.GPS_FIX){    //if no fix and gps funtion is activated: do warning beeps.
    warn_noGPSfix = 1;
  }else{
    warn_noGPSfix = 0;
  }
  #endif

The state of the GPS code is checked, not the TX switch position. The code has been commited by dubusal@gmail.com in r925.

Before that, I (stefan@pico.ruhr.de) commited a version using the get_flag macros (r875):

Code: Select all

if ((get_flag(FLAG_GPS_HOME_MODE) || get_flag(FLAG_GPS_HOLD_MODE)) && !get_flag(FLAG_GPS_FIX)){ 

It is equivalent to the code above.

How did it look before that?
Well, someone called nils.tholen@gmail.com submitted this code in r620:

Code: Select all

if ((GPSModeHome || GPSModeHold) && !GPS_fix){    //if no fix and gps funtion is activated: do warning beeps.

It is equivalent to the code above.

Before that, Buzzer.ino/Buzzer.de was still called buzzer.pde; the code originates in r615, commited by nils.tholen@gmail.com. Again, equivalent to all successor implementations.

So please point out at which point in time this code used the boxes instead of the GPS flag variables?

[jevermeister]

What are we doing here?


Before:
multiwii.ino:

Code: Select all

 if ( (GPSModeHome == 0 && GPSModeHold == 0) || (GPS_fix_home == 0) ) {

buzzer.ino:

Code: Select all

  if ((GPSModeHome || GPSModeHold) && !GPS_fix){    //if no fix and gps funtion is activated: do warning beeps.

after:
multiwii.ino:

Code: Select all

     if (f.GPS_FIX && GPS_numSat >= 5 ) {
        if (rcOptions[BOXGPSHOME]) {
          if (!f.GPS_HOME_MODE)  {
            f.GPS_HOME_MODE = 1;
            GPS_set_next_wp(&GPS_home[LAT],&GPS_home[LON]);
            nav_mode    = NAV_MODE_WP;
          }
        } else {
          f.GPS_HOME_MODE = 0;
        }
        if (rcOptions[BOXGPSHOLD]) {
          if (!f.GPS_HOLD_MODE) {
            f.GPS_HOLD_MODE = 1;
            GPS_hold[LAT] = GPS_coord[LAT];
            GPS_hold[LON] = GPS_coord[LON];
            GPS_set_next_wp(&GPS_hold[LAT],&GPS_hold[LON]);
            nav_mode = NAV_MODE_POSHOLD;
          }
        } else {
          f.GPS_HOLD_MODE = 0;
        }
      }

buzzer.ino:

Code: Select all

  if ((f.GPS_HOME_MODE || f.GPS_HOLD_MODE) && !GPS_fix){    //if no fix and gps funtion is activated: do warning beeps.

I think the problem is pretty obvious. The flags where set before the change despite the status of GPS fix, after the change the flags were only set if a fix is ok, so the warning is pointless.

I don't want to know who did this, it is not important, it is fixed - that is important.

Fracking cool isn't it?

Have a good night everybody - Mythbusters is on.

Nils

[Tommie]

What are we doing here?

That's what I've been asking you from the beginning.

Before:

Before what?

I think the problem is pretty obvious. The flags where set before the change despite the status of GPS fix, after the change the flags were only set if a fix is ok, so the warning is pointless.

Yes, and this has been the case since 2012-05-25, r814. It had nothing to do with the bitflags implemented later.

I don't want to know who did this, it is not important, it is fixed - that is important.

I agree, however I'd be even more delighted if the facts would get checked before starting the finger pointing.

Have fun watching Adam and Jamie blowing stuff up

[Hamburger]

error : GUI - cannot SAVE config: the dialog box is equal to the LOAD dialog box - cannot enter file name to save to.
This is what the SAVE dialog looks like:

GUI -> SAVE

[Alexinparis]

Alexinparis said:

Hi,
I think there is not only one possible VTAIL config.
For instance 40deg inclination is one possible setup, but we can found other inclinations and other arm distances.
So the best we could do is to document more the current implementation, and give some hints to modify the code to cope with other VTAIL setups.

I'm agree with you, the Sin(MotorAngle)x100 as angle is a variable, this could be written in comments, but current implementation is wrong anyway.
Torque Yaw is in opposition to the yaw inducted by push force of the 40° motor angle.

That's why I reversed quad like motor rotation, like they did in OpenPilot for Vtail. Now Front Left is CCW and Front Right is CW

Ok, I understand your view because in your setup the prop turns in the opposite direction.
The one currently in multiwii is based on the QUADX propeller rotation direction. This is the logic behind.
There is no wrong or right setup. OpenPilot implementation is just different.

[Alexinparis]

Hi,
@Alex: I just took the time to browse the code and found you already fixed the buzzer defines for toggle beep - no wrok for me

I just did the setup for my configuration and stumbled upon something that bugged me for a long time:

Why is

Code: Select all

// ******************
// rc functions
// ******************
#define MINCHECK 1160
#define MAXCHECK 1850

situated in mutliwii.ino?
Wouldn't it be better if we place it in config.h? I have to adjust these variables because my TX has a more narrow band. It is ahrd to find and easy to miss.


Nils

ps.: I will test _shared tomorrow: Looking forward

The standard for every TX should be able to output for each RC chan [1000-2000], that's why it's not in the conf part.
It's also a guarantee to have a good stick resolution, which wouldn't be the case with a poor range.
Maybe we can move those param in config.h, but with some warnings.

[Alexinparis]

error : GUI - cannot SAVE config: the dialog box is equal to the LOAD dialog box - cannot enter file name to save to.
This is what the SAVE dialog looks like:

GUI-save.jpeg

on win XP, it's the same dialog box, but we can enter a name to save the file.

[kos]

thank for pointing my copy/past habit

solved

[kipkool]

Ok, I understand your view because in your setup the prop turns in the opposite direction.
The one currently in multiwii is based on the QUADX propeller rotation direction. This is the logic behind.
There is no wrong or right setup. OpenPilot implementation is just different.

I changed prop turns direction to solve the Yaw inneficiency problem.
It's a bit long to explain, i've cleared it here with a scheme : http://www.rcgroups.com/forums/showpost ... tcount=805

[jevermeister]

Alex,
there is no beep after writing params from Conf to EEPROM.
But if I understand the code correctly it has to beep. It is using blinkLED like Gyrocalibration and AccCalibratien etc. And they all buzz.

Nils

[mahowik]

Hi,

I understand the coeff are not correct in the original version,
but something seems wrong here because the weight of ROLL factor for 4&5 motors shouldn't be the same as the others.

Hi Alex,

I have got a couple of good feedbacks for these new mixes on russian forum:
http://forum.rcdesign.ru/f123/thread258 ... ost3476241
http://forum.rcdesign.ru/f123/thread258 ... ost3477676

Actually the logic is simple for mixes - it should take into account angle to calculate the power of each motor.

And it should take into account two points:
1) be symmetric in power for roll and pitch
2) each motor should have the same power (as much as possible) in diff with others motors to have linear load

So mixes for HEX6 & HEX6X are here :

Code: Select all

#ifdef HEX6
    motor[0] = PIDMIX(-9/10,+4/5,+1); //REAR_R
    motor[1] = PIDMIX(-9/10,-4/5,-1); //FRONT_R
    motor[2] = PIDMIX(+9/10,+4/5,+1); //REAR_L
    motor[3] = PIDMIX(+9/10,-4/5,-1); //FRONT_L
    motor[4] = PIDMIX(+0  ,-4/5,+1); //FRONT
    motor[5] = PIDMIX(+0  ,+4/5,-1); //REAR
  #endif
  #ifdef HEX6X
   motor[0] = PIDMIX(-4/5,+9/10,+1); //REAR_R 
   motor[1] = PIDMIX(-4/5,-9/10,+1); //FRONT_R 
   motor[2] = PIDMIX(+4/5,+9/10,-1); //REAR_L 
   motor[3] = PIDMIX(+4/5,-9/10,-1); //FRONT_L 
   motor[4] = PIDMIX(-4/5 ,+0 ,-1); //RIGHT 
   motor[5] = PIDMIX(+4/5 ,+0 ,+1); //LEFT
  #endif

About the THR expo functions:
If I set MAXTRHOTTLE to 1900,
When RC throttle channel reaches 2000, we have the right value: rcCommand[THROTTLE] = 1900
(checked with debug[0]) whatever the value of MID or EXPO.
please check again your conf.

mid value is in the range:
from the stick point of view: [MINCHECK;2000]
from the throttle command point of view: [MINTHROTLE ; MAXTHROTTLE]

I will add a small cursor in the GUI so that you can see where the throttle is in the graph, and correlate easier the hover point.
The logic is a little bit different from alexmos code because the expo curve is apply only on the useful part of the range.

"small cursor" is a good idea to see/find the hover point!
About range from TX: Yes, I had ~1920 from receiver as max value. And as for now with Turnigy 9x (er9x firmware) it's not a problem to increase the range for me, e.g. to 1000..2000, but for some cheapest TX like "Hobby King 2.4Ghz 6Ch Tx" (i started from that one) it's not possible to set this range even with 125%, i.e. only about 1080-1920...

thx-
Alex

[shikra]

Thats intersting Alex - I said same to someone yesterday that I thought the model was slightly wrong for Hex - not symettrical as you said. I'll have a look at yours tomorrow. I'm sure its good - nice to have real world test feedback too!

I had first flight with hexa this week. Could sense something not quite right.... kinda 90% there

[shikra]

I just flew with Alex Hex6+
I wish I had put it on a switch for comparison, but can say it works fine. And subjectively it felt better flying. Last time it it just didn't feel quite right..... I didn't have that feeling this time.

[mahowik]

I just flew with Alex Hex6+
I wish I had put it on a switch for comparison, but can say it works fine. And subjectively it felt better flying. Last time it it just didn't feel quite right..... I didn't have that feeling this time.

Shikra, thanks for the quick response!

It seems we have found right mixes for hex

[Alexinparis]

Ok, I understand your view because in your setup the prop turns in the opposite direction.
The one currently in multiwii is based on the QUADX propeller rotation direction. This is the logic behind.
There is no wrong or right setup. OpenPilot implementation is just different.

I changed prop turns direction to solve the Yaw inneficiency problem.
It's a bit long to explain, i've cleared it here with a scheme : http://www.rcgroups.com/forums/showpost ... tcount=805

ok, I think I understand.
I will update the VTAIL mix accordingly.

[Alexinparis]

Alex,
there is no beep after writing params from Conf to EEPROM.
But if I understand the code correctly it has to beep. It is using blinkLED like Gyrocalibration and AccCalibratien etc. And they all buzz.

Nils

It's normal: in writeParam, there is a toggle to avoid blinkLED.
this toggle is not activated after an EEPROM write.

if (b == 1) blinkLED(15,20,1);

[Alexinparis]

So mixes for HEX6 & HEX6X are here :

Code: Select all

#ifdef HEX6
    motor[0] = PIDMIX(-9/10,+4/5,+1); //REAR_R
    motor[1] = PIDMIX(-9/10,-4/5,-1); //FRONT_R
    motor[2] = PIDMIX(+9/10,+4/5,+1); //REAR_L
    motor[3] = PIDMIX(+9/10,-4/5,-1); //FRONT_L
    motor[4] = PIDMIX(+0  ,-4/5,+1); //FRONT
    motor[5] = PIDMIX(+0  ,+4/5,-1); //REAR
  #endif
  #ifdef HEX6X
   motor[0] = PIDMIX(-4/5,+9/10,+1); //REAR_R 
   motor[1] = PIDMIX(-4/5,-9/10,+1); //FRONT_R 
   motor[2] = PIDMIX(+4/5,+9/10,-1); //REAR_L 
   motor[3] = PIDMIX(+4/5,-9/10,-1); //FRONT_L 
   motor[4] = PIDMIX(-4/5 ,+0 ,-1); //RIGHT 
   motor[5] = PIDMIX(+4/5 ,+0 ,+1); //LEFT
  #endif

I still think there is an error:
motor[3] = PIDMIX(+9/10,-4/5,-1); //FRONT_L
motor[4] = PIDMIX(+0 ,-4/5,+1); //FRONT
FRONT_L and FRONT shouldn't have the same PITCH coeff.
confirmed also here: http://code.google.com/p/ardupirates/wiki/motormixing

I think the current code is closer to the right mix, except for ROLL&PITCH coeff that should be different.

"small cursor" is a good idea to see/find the hover point!
About range from TX: Yes, I had ~1920 from receiver as max value. And as for now with Turnigy 9x (er9x firmware) it's not a problem to increase the range for me, e.g. to 1000..2000, but for some cheapest TX like "Hobby King 2.4Ghz 6Ch Tx" (i started from that one) it's not possible to set this range even with 125%, i.e. only about 1080-1920...

[/quote]
So, your problem is independent with the throttle expo implementation.
1920 throttle stick value can't give MAXTHROTTLE value with the current code or with the old one.