search

laurb9 / StepperDriver

Arduino library for A4988, DRV8825, DRV8834, DRV8880 and generic two-pin (DIR/STEP) stepper motor drivers
MIT License
556 stars 228 forks source link

Use safer & faster min/max functions #120

Closed richievos closed 1 year ago

richievos commented 1 year ago

Two issues here. First, the default min/max that this is dependening on uses min/max as a macro. That means the min call was actually evaluating sqrt and float cast twice, with a minor perf penalty (unless the optimizer catches it).

Second, not all environments have min/max available in the global namespace (particularly platformio's native env). This avoids that implicit dependency. Therefore this fixes:

<path_here>/StepperDriver/src/BasicStepperDriver.cpp:141:25: error: use of undeclared identifier 'min'; did you mean 'fmin'?

The original purpose of this diff was the second issue, but researching other people running into this made me realize the first issue. So I went down a different fix.

Another fix is just use native c++'s std::min and std::max, but that gets into various arduino-cli related compiler complications.

richievos commented 1 year ago

Sorry for the multiple force-pushes, took me a couple tries to figure out how to run this locally and on my own PR fork.

I updated the commits to get the build passing (ran them on my fork https://github.com/richievos/StepperDriver/pull/2), and I also found there's a minor potential perf boost to an alternative solution, so I did that too.

richievos commented 1 year ago

Thank you for this fix. I am curious about the use case for native, I imagine this code runs choppy on a Pi or similar

You're welcome. I'm using native mode to run my unit tests without having to have my ESP32/Arduino/... connected. I am not invoking the StepperDriver code in that setup, but I currently do have it as a library dependency to successfully build my tests.