teemuatlut / TMC2130Stepper

Arduino library for Trinamic TMC2130 Stepper driver
MIT License
159 stars 50 forks source link

Usage of _pinDIR, _pinSTEP, _pinEN #19

Closed alexsomesan closed 6 years ago

alexsomesan commented 6 years ago

It seems, by searching the repository, that these attributes are not used anywhere. Yet the constructor expects them as arguments which then get stored in the corresponding attributes.

If indeed unnecessary can these be removed and the constructor simplified?

Thank you!

teemuatlut commented 6 years ago

At one point the idea was to have a step generating function included but I didn't include it in the end. However, if I now remove the EN/STEP/DIR arguments from the constructor it would from 4 to 1 argument, while the new SW SPI constructor would go from 7 to 4. This would cause confusion when updating the library since the 4 argument constructor still exists but its function is different. I might add a single argument constructor and leave the current one as deprecated.

They won't be included in the TMCStepper library that at some point will supercede this one.

alexsomesan commented 6 years ago

I was suspecting that was the story. Totally agree that just plain removing them would be a breaking change and cause unnecessary confusion. I think the only reasonable option now is to add a single argument constructor and leave the current one as deprecated, like you suggested. That would make my life a bit easier, but you know best if the change is worth it or not.

Thanks for clarifying the background though!

teemuatlut commented 6 years ago

https://github.com/teemuatlut/TMC2130Stepper/commit/4db5d2f62915cb2a03acd95812c36109ecb43101 Add a single argument constructor

alexsomesan commented 6 years ago

Awesome! Thanks a lot for adding the simple constructor.