Airsim: Inconsistent implementation with comment in PidController.hpp

Created on 1 Oct 2019 · 5Comments · Source: microsoft/AirSim

See below code:
https://github.com/microsoft/AirSim/blob/991810e9e00fe3395498888d8789a98e538608b2/AirLib/include/vehicles/multirotor/firmwares/simple_flight/firmware/PidController.hpp#L99-L107

The comments say that goal would be canceled out and dInput only matters.
I guess dInput is dMeasured, i.e, error_der = (measured_ - last_measured_) / dt
but the code above is calculating difference between goal_ and last_goal_.
I think the comments would be right.
Is this a bug? or Do I miss something?

Source

sehee382

😕1

All 5 comments

This also confuses me.

48cfu on 10 Oct 2019

👍1

But nobody answers me );

seheevic on 10 Oct 2019

OPTION1: I would actually do something like this, according to the definition of the derivative action

if (dt > min_dt_) { 
     integrator->update(dt, error, last_time_); 

     float error_der = (error - last_error_) / dt; 
     dterm = error_der * config_.kd; 
     last_error_ = error; 
 }

OPTION2: instead of explicitly eliminating the "derivative kick" (the current code as quoted by @sehee382 appears to be wrong anyway)

if (dt > min_dt_) { 
     integrator->update(dt, error, last_time_); 

     //To eliminate "derivative kick", we assume goal was approximately 
     //constant between successive calls. dE = dGoal - dMeasured = -dMeasured 
     float error_der = - (measured - last_measured_) / dt; 
     dterm = error_der * config_.kd; 
     last_measured_ = measured_; 
 }

Can I fix this issue by implementing OPTION1? Or is there a reason to implement OPTION2?

48cfu on 10 Oct 2019

👀1 👍1

Just a suggestion ( I don't have knowledge about this nor have I done a deep-dive into the controller code)
You could open a PR, maybe the maintainers could then have a look