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

This forum is dedicated to software development related to MultiWii.
It is not the right place to submit a setup problem.
Software download
Tommie
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

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

Post by Tommie »

jevermeister wrote: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
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

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

Post by 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?

User avatar
jevermeister
Posts: 708
Joined: Wed Jul 20, 2011 8:56 am
Contact:

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

Post by 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
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

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

Post by Tommie »

jevermeister wrote: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#)

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

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

Post by EOSBandi »

deleted
Last edited by EOSBandi on Thu Jul 05, 2012 10:54 pm, edited 1 time in total.

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

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

Post by EOSBandi »

jevermeister wrote: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....

User avatar
jevermeister
Posts: 708
Joined: Wed Jul 20, 2011 8:56 am
Contact:

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

Post by jevermeister »

EOSBandi wrote:
jevermeister wrote: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

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

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

Post by EOSBandi »

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

Tommie
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

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

Post by Tommie »

jevermeister wrote: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?

User avatar
jevermeister
Posts: 708
Joined: Wed Jul 20, 2011 8:56 am
Contact:

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

Post by 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
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

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

Post by Tommie »

jevermeister wrote: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 :-)

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

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

Post by 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
GUI -> SAVE

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

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

Post by Alexinparis »

kipkool wrote: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
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

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

Post by Alexinparis »

jevermeister wrote: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
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

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

Post by Alexinparis »

Hamburger wrote: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.

User avatar
kos
Posts: 286
Joined: Thu Feb 16, 2012 4:51 am
Location: Fr

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

Post by kos »

thank for pointing my copy/past habit

solved ;)

kipkool
Posts: 30
Joined: Mon Jun 25, 2012 9:21 am

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

Post by kipkool »

Alexinparis wrote:
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
Image
Image

User avatar
jevermeister
Posts: 708
Joined: Wed Jul 20, 2011 8:56 am
Contact:

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

Post by 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
Posts: 332
Joined: Sun Apr 10, 2011 6:26 pm

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

Post by mahowik »

Alexinparis wrote: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


Alexinparis wrote: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

User avatar
shikra
Posts: 783
Joined: Wed Mar 30, 2011 7:58 pm

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

Post by 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

User avatar
shikra
Posts: 783
Joined: Wed Mar 30, 2011 7:58 pm

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

Post by 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
Posts: 332
Joined: Sun Apr 10, 2011 6:26 pm

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

Post by mahowik »

shikra wrote: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
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

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

Post by Alexinparis »

kipkool wrote:
Alexinparis wrote:
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
Image
Image


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

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

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

Post by Alexinparis »

jevermeister wrote: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
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

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

Post by Alexinparis »

mahowik wrote: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.

Post Reply