PowerBroker2 / ArduPID

PID library for Arduinos with greater accuracy than the legacy Arduino PID library
MIT License
85 stars 23 forks source link

Separated compute logic from timer #15

Closed ipsod closed 8 months ago

ipsod commented 8 months ago

In my application, the control logic needs some things to happen after the timer fires but before compute() is called, so I separated the timer call from the compute logic.

PowerBroker2 commented 8 months ago

Hello, thanks for the PR! Can you explain the issue and the resulting fix in more detail? Thanks!

ipsod commented 8 months ago

Hi! My pleasure - I'm a big fan of this library and your SerialTransfer library.

I'm driving motors. After the timer fires, I first need to grab target positions and then calculate positional errors, and only then run the PID compute(). The compute() function contained both the timer check and the compute logic, and I didn't see any place to insert my grab position and calculate error logic between the two, so I separated them. I only made the smallest change that would let me insert logic where I needed to, and I don't think it should change your library for most uses.

I'm not actually using the updated compute() function at all, in my code - just the doCompute function that I extracted from it. My own code is just like the compute() function, except that I added my application-specific stuff between timer.fire() and doCompute(). Well, that, and, I'm actually running many motors, each with their own PID, but all on the same timer - they're all on a 1khz timer, so there's no need for each to have their own.

ipsod commented 8 months ago

Also, in many instances, one doesn't even want to use a software timer at all. Hardware timers or RTOS timers might be doing the ticking. So, this is just a way to open up the library to more use-cases, hopefully in a non-breaking way.

ipsod commented 8 months ago

This is a side-point, but, I previously needed the double float precision of your library, because I was trying to use setpoint=target position and input=actual position, and now I use input=positional_error, and setpoint=0. Since positional_error has a much smaller range than target_position, I think single precision floats should work fine for me. So, most of what I've said is now hypothetical, because I'm probably going to transition to a 32-bit PID library, for this application.

ipsod commented 8 months ago

QuickPID's docs summarize everything I've said as:

Optional external timer or ISR timing control.

https://www.arduino.cc/reference/en/libraries/quickpid/