Looking at the dataflash logs, it appears that the I term in the speed controller does not get reset when you switch modes - it basically drifts to one extreme of IMAX or the other. This seems to either give you a slow or excessively fast initial response.
To me, it seems like switching into AUTO, GUIDED, STEERING or ACRO should reset the I term since the target speed will have changed, so any I term is no longer valid.
You can see in the plot that the I term persists through mode changes, giving it an extra kick as soon as it goes into Guided or Auto again.
Just to be a “me too”: I have seen this behavior with my automated lawn mower (https://www.youtube.com/channel/UCkIJQFyZdzLKh7AR-m3t2Bw). If I have it in manual for a while, when I switch to auto, it takes off at a high speed. It corrects in about 2 seconds or so, so it isn’t too bad, but a little scary until I see it slow down to normal speed. I suspected it was something like what you found but I had not analyzed it.
Aha, I think there is a missing reset_I() call here in AR_AttitudeControl. feel free to PR a fix or I can have a go at it on Monday.
Re the I-term build-up to IMAX while in Auto, Guided, etc, could you post the full dataflash log? A random guess is that the cruise-speed and cruise-throttle aren’t set quite right.
// reset input filter to value received
if (_flags._reset_filter) {
_flags._reset_filter = false;
_input = input;
_derivative = 0.0f;
}
Should this also reset the integrator to 0, rather than having to explicitly call “reset_I()” - surely if you’re restarting the whole PID controller, I should also go to zero.
Don’t worry about the I build up in that case - it was something I threw together quickly in SITL to illustrate my point.
calling reset_I() as part of reset_filter() seems reasonable. When I did a quick search for “reset_filter()” it’s only used in Rover and as far as I can tell it should always be followed by a reset_I(). It looks like we’re missing the reset_I() in the balance bot code (AR_AttitudeControl::get_throttle_out_from_pitch) as well.
I’d probably want to check with Leonard Hall before making that change but functionally it seems correct.
I’ve fixed this in master by calling reset-I in the speed controller… so it’s very possible that the other solution involving adding reset-I to reset_filter is a better way to go but the effect is the same and I can make this change immediately.
We could push this into an official release of Rover-3.4… we’re getting close to starting beta testing of Rover-3.5 though… anyway… if people are really keen… we could patch the official release…
