search

cvra / pid

A PID controller implementation
68 stars 27 forks source link

Initial code review #1

Closed antoinealb closed 10 years ago

antoinealb commented 10 years ago

Please review :)

froj commented 10 years ago

Looks good. We need to add a limit for the integrator and a function to reset the integrator.

Stapelzeiger commented 10 years ago

Why did you choose to use malloc? (btw you didn't check it's return value)

Stapelzeiger commented 10 years ago

What do you think about adding a control frequency/period parameter. Then we could have PID parameters which are independent of control frequency.

antoinealb commented 10 years ago

I used malloc because I think having private structure members promotes encapsulation, which is good.

I will open several issues to discuss each of the above points.