Coding error, can anyone confirm?
I tried to understand Arduplane's code. In calc_nav_roll (Attitude.pde) there's a command:
nav_roll = g.pidNavRoll.get_pid(bearing_error, dTnav, nav_gain_scaler);I understand that dTnav is the time interval between calls of PID controller. This command is executed each time calc_nav_roll is called so the time interval between calc_nav_roll's calls also dTnav. dTnav is updated in medium_loop at the frequency of about 10 Hz, so dTnav is about 100 ms. Looking at the main loop, the fast_loop is called at the frequency of about 50 Hz, the time interval between calls is delta_ms_fast_loop which is about 20 ms. The fast_loop calls update_current_flight_mode() that means the time interval between calls also 20 ms. In most cases of update_current_flight_mode() it calls cal_nav_roll, so the time interval between calls is only 20 ms. There is a big difference between 20 ms and 100 ms. In my understanding in get_pid() calls instead of dTnav it should be delta_ms_fast_loop. Is this a coding error? Can anyone confirm or clarify this?
Replies
Another thing I don't understand is that when looking at calc_nav_pitch(). There is a command
nav_pitch = -g.pidNavPitchAirspeed.get_pid(airspeed_error, dTnav); (2 parameters call)
However, get_pid definition is
PID::get_pid(int32_t error, uint16_t dt, float scaler) (3 parameters definition)
How can it be like this? Can anyone explain me?
yep, you're right. Jon Challinger noticed this recently as well. I've been thinking about how to fix it.
It's easy to just fix the deltat of course, but that will change the meaning of the I and D terms in everyones existing configurations (ie. the HDG2RLL_I and HDG2RLL_D values), scaling them by a factor of 5x. That could change how well a plane flies by quite a lot. I'm always reluctant to break existing tuning, but in this case we may have to, with a warning in the release notes.
I'm also thinking about how to ensure this sort of bug doesn't happen again. I think the best way would be to make the PID object compute the deltat itself, rather than passing it in.
Cheers, Tridge