On Wed, Feb 25, 2015 at 02:48:49PM +0000, Javi Merino wrote: > Hi Eduardo, > > On Tue, Feb 24, 2015 at 06:21:26PM +0000, Eduardo Valentin wrote: > > On Wed, Jan 28, 2015 at 05:00:35PM +0000, Javi Merino wrote: > > > + > > > +k_d > > > +--- > > > + > > > +`k_d` configures the PID loop's derivative term constant. It's > > > +recommended to leave it as the default: 0. > > > + > > > > I know we are considering K_d = 0. However, ... > > > > <yet another big cut> > > > > > + /* > > > + * Calculate the derivative term > > > + * > > > + * We do err - prev_err, so with a positive k_d, a decreasing > > > + * error (i.e. driving closer to the line) results in less > > > + * power being applied, slowing down the controller) > > > + */ > > > + d = mul_frac(tz->tzp->k_d, err - params->prev_err); > > > > > > ... Shouldn't the above d component consider the rate of changes over time > > of the error? > > > > I would expect you should do: > > d = k_d * (dE / dt) > > > > or > > > > d = K_d * ((err - params->prev_err) / sampling_period) > > > > in plain C: > > > > + d = mul_frac(tz->tzp->k_d, err - params->prev_err); > > + d /= tz->passive_polling; /* might require fixed point division */ > > Could do. To be honest, both k_d and passive_polling are constants so
Yes, I agree that they are constants. But if you deploy the thermal zone with different sampling period on different devices, then the behavior will change. > I don't think you get anything by doing this other than the added > complexity of the fixed point division. As you said, the default k_d > is 0, so I'm not strongly against it. OK. Then I would prefer to add the division, as it makes the code aligned to the concept. > > Cheers, > Javi
signature.asc
Description: Digital signature
