Possible Bug in GPS?

[PatrikE]

I found a possible Bug in GPS.

In get_D function

Code: Select all

.
.
.
    // update state
    pid->last_input = input;
    pid->lastderivative    = pid->derivative;
    // add in derivative component
    return pid_param->kD * pid->derivative;
  }

get_D is called one time per axis in the navmodes.
But they will share pid->last_input & pid->lastderivative ....

Each axis should have a own set of "Last state" parameters.
I think this can cause problems in the calculations.

[pm1]

yes,
bug was introduced when removing c++ classes (The code for the i2c board ist still ok). Second thing is, in calc_poshold the P and I term are dependend on rate_error, the D term is dependend on error. This is in fact a second P term of velocity (rate). Try to pull the copter by hand from waypoint in PH mode several meters and then let it go. Then you know, what I mean . Remove D-term completely and limit P-term to 50 cm/s, then it works brilliant (at least for me).

[dramida]

was this bug solved?

[pm1]

No, it is not solved. I often don't know, if it's simple a bug or a feature. I think multiwii flies with a bit of magic . Fixing a bug might change too much. For example, if you fix the cycle time of baro, mag, gps, sonar, the new altitude hold will work somewhat different. If you correct the gps pids, the copter might react differently. If you correct the bug in mag initialization, mag might work differently...

[timecop]

lol...

at least it "works" in the worthless (i2c) gps implementation...

[pm1]

lol...
at least it "works" in the worthless (i2c) gps implementation...

Why worthless, I even would have to use it on naze32, since I haven't an rx with sum signal.

I had the i2c board working in one day, now on an mega2560 it costs me about 2 days to find out, that the serial interfce for gps was close unintentionally in the init code, if the initialization was not fast enough (another sample of some magic code).

[dramida]

If bugs are cleared one after the other, there will be no magic after all. I had big ptoblems lately with ublox serial gps.

[pm1]

I received 2 days ago the second ublox-serial (Crius gps from rctimer version 1). It now works great in multiwii and megapirate...

[Alexinparis]

I found a possible Bug in GPS.

In get_D function

Code: Select all

.
.
.
    // update state
    pid->last_input = input;
    pid->lastderivative    = pid->derivative;
    // add in derivative component
    return pid_param->kD * pid->derivative;
  }

get_D is called one time per axis in the navmodes.
But they will share pid->last_input & pid->lastderivative ....

Each axis should have a own set of "Last state" parameters.
I think this can cause problems in the calculations.

I see no bug here.
Where do you see that pid->last_input & pid->lastderivative are shared ??

[PatrikE]

In GPS.ino
Both static void GPS_calc_nav_rate(.... and static void GPS_calc_poshold() call the get_D(error[axis] function.
That's ok because only one of them at the time depending on witch nav_mode is active.

But get_D(error[axis] is called in a for loop!
for (axis=0;axis<2;axis++) {
And if you look at pid->lastderivative & pid->lastderivative It can only one store last reading.
That's where i se the possible "Shared pid->last_input & pid->lastderivative "

Or
Maby it's just me having bad luck thinking...

[timecop]

lol...
at least it "works" in the worthless (i2c) gps implementation...

Why worthless, I even would have to use it on naze32, since I haven't an rx with sum signal.

I had the i2c board working in one day, now on an mega2560 it costs me about 2 days to find out, that the serial interfce for gps was close unintentionally in the init code, if the initialization was not fast enough (another sample of some magic code).

You don't need ppm sum receiver to use GPS for at least a month now.
You just lose those 2 rc channels, so you only get 2 aux instead of 4.
See latest manual. Not saying it works any better there, if there is that pid bug, its probably still there too.

[Mis]

PatrikE:
Look at declaration of params for get_D:
int32_t get_D(int32_t input, float* dt, struct PID_* pid, struct PID_PARAM_* pid_param)
pid is declarated as poiner to structure witch PID_ type.
Then in GPS_calc_poshold() you see get_D call:
d = get_D(error[axis], &dTnav, &poshold_ratePID[axis], &poshold_ratePID_PARAM);
The pid variable of get_D function is assigned to poshold_ratePID[axis] array declarated as " PID poshold_ratePID[2]; ". Each [axis] have separate structure.
In this case the pid->derivative is assigned to "float derivative" field in array, and any iteration of "for (axis=0;axis<2;axis++)" have own "pid->lastderivative" in array.
Maybe you should read some more about pointers

[PatrikE]

So then there's no bug then?

As i said it could be me having bad luck thinking...
Just me not understanding the code...

[Mis]

So then there's no bug then?

IMHO no, but i'm not 100% sure because this code is little screwed up
For sure, need to look at the generated assembly code, but it is more unreadable and need lot of time for understand it.

[dramida]

I think we have a Bug in GPS. Try to yaw the copter in GPS pos hold and it will go all over the sky till you stop yaw-ing. Is this Bug enough?

[pm1]

I think we have a Bug in GPS. Try to yaw the copter in GPS pos hold and it will go all over the sky till you stop yaw-ing. Is this Bug enough?

Sorry, but i cannot reproduce this. My copter stays within 2m with full yaw (if you like, until battery is empty). Maybe because I changed PH code.

[pm1]

So then there's no bug then?

As i said it could be me having bad luck thinking...
Just me not understanding the code...

No, the code is OK. I didn't see, that a pid structure (with last_derative) per axis was declared, and the whole structure is given to calculation

[dramida]

I think we have a Bug in GPS. Try to yaw the copter in GPS pos hold and it will go all over the sky till you stop yaw-ing. Is this Bug enough?

Sorry, but i cannot reproduce this. My copter stays within 2m with full yaw (if you like, until battery is empty). Maybe because I changed PH code.

I am interesting about yaw-ing in PH as i want to shoot some aerial virtual panoramas. Where can i test the latest PH code? I am using R1124 from shared trunk.
Here i found a pice of code that if defined, screws up all gps navi on Criusa AIO board.

Code: Select all

/************************        AP FlightMode        **********************************/
    /* Temporarily Disables GPS_HOLD_MODE to be able to make it possible to adjust the Hold-position when moving the sticks.*/
    //#define AP_MODE 10  // Create a deadspan for GPS.

Thanks.

[pm1]

[quote="dramida"Where can i test the latest PH code? [/quote]

Maybe give this a try, it limits the maximum speed when in poshold mode, and removes the obscure d term

Code: Select all

static void GPS_calc_poshold() {
  int32_t target_speed;
  uint8_t axis;
  
  for (axis=0;axis<2;axis++) {
    target_speed = get_P(error[axis], &posholdPID_PARAM); // calculate desired speed from lon error
    target_speed = constrain(target_speed,-100,100);
    rate_error[axis] = target_speed - actual_speed[axis]; // calc the speed error

    nav[axis]      =
        get_P(rate_error[axis],                                               &poshold_ratePID_PARAM)
       +get_I(rate_error[axis] + error[axis], &dTnav, &poshold_ratePID[axis], &poshold_ratePID_PARAM);
    nav[axis]  = constrain(nav[axis], -NAV_BANK_MAX, NAV_BANK_MAX);       
    navPID[axis].integrator = poshold_ratePID[axis].integrator;
  }
}

I am using these pids.