teemuatlut / TMC2130Stepper

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

Dereferences the return value of portOutputRegister #18

Closed tobbelobb closed 6 years ago

tobbelobb commented 6 years ago

I haven't tested this on hardware, and I'm not an expert on digital I/O, I just noticed a compile warning:

SW_SPI.cpp:43:21: warning: invalid conversion from 'volatile uint8_t* {aka volatile unsigned char*}' to 'uint8_t {aka unsigned char}' [-fpermissive]
       miso_register = portOutputRegister(getPort(miso_pin));

The mentioned type conversion is indeed taking place in my build. See documentation for portOutputRegister().

Comparing to Arduino's source code for digitalWrite(), it seems like dereferencing the output of portOutputRegister() is what we want to do.

We could also change types of mosi_register, miso_register, and sck_register to volatile uint8_t*, and do the dereferencing of them in writeMOSI_H, writeMOSI_L, writeSCK_H, writeSCK_L, and readMISO instead, I guess.

teemuatlut commented 6 years ago

Thanks. I'll do the testing tomorrow and sure enough it compiles, but I might be more comfortable if we went with changing the variable type as then it would match the Arduino digitalWrite implementation.

tobbelobb commented 6 years ago

Ok. Were tests successful?

teemuatlut commented 6 years ago

I seem to recall that I did quickly test it, but the changes should be now in the v2.1.4 release.

tobbelobb commented 6 years ago

Looks good! Closing.