jgromes / RadioLib

Universal wireless communication library for embedded devices
https://jgromes.github.io/RadioLib/
MIT License
1.48k stars 373 forks source link

Custom SPI pins #121

Closed aguglie closed 4 years ago

aguglie commented 4 years ago

Hi Jan,

I was testing out RadioLib on a Adafruit HUZZAH32 – ESP32 Feather and spot a strange issue: the labeled SPI pins are (MOSI: 18, MISO: 19, SCK: 5) which differ from the default ones (MOSI: 23, MISO: 19, SCK: 18).

Unfortunately, all the feather boards/radios agree on this pin mapping.

Meaning that i had to manually patch Module.cpp, specifying the pinout as: _spi->begin(5, 19, 18, SS);.

Have you already faced this sort of issue? I was wondering about adding some parameters to Module's constructor, but I'm not sure about compatibility with other boards: seems that ESP822's SPI::begin is not accepting custom mapping.

Let me know :smile:

jgromes commented 4 years ago

There's already a Module constructor that allows you to change the SPI instance:

https://github.com/jgromes/RadioLib/blob/b7c21ffac65ccb2b32069ab473739a591ae55c2c/src/Module.h#L38-L51

You should be able to create a new SPI class with the correct pins or change the pins of the default SPI prior to calling Module constructor.

aguglie commented 4 years ago

Hi Jan, thanks for you fast reply.

There's no way to change the pins pior calling Module constructor, seems that the only way to change them is via begin(sck, miso, mosi, ss).

I'll extend SPI and wrap begin()to make it call begin(mySck, myMiso, ....).

Thanks.

P.S: Feel free to re-open if you spot a smarter way to do it!

jgromes commented 4 years ago

There's no way to change the pins pior calling Module constructor

Not with the default examples, but there's no reason you can't do something like this (untested with HW but does compile):

#include <RadioLib.h>

SPIClass newSPI(HSPI);

Module* mod;
SX1268* radio;

void setup() {
  newSPI.begin(5, 19, 18, SS);
  mod = new Module(10, 2, 3, 9, newSPI);
  radio = new SX1268(mod);
}

See also https://github.com/espressif/arduino-esp32/blob/master/libraries/SPI/examples/SPI_Multiple_Buses/SPI_Multiple_Buses.ino

jgromes commented 4 years ago

Looking over Module.cpp however, I came across this:

https://github.com/jgromes/RadioLib/blob/b7c21ffac65ccb2b32069ab473739a591ae55c2c/src/Module.cpp#L61

It's not entirely clear whether that will re-initialize SPI on the default pins, this will have to be tested (and preferrably only initialize SPI if the user hasn't done so).

aguglie commented 4 years ago

That double call to begin was the reason why I was proposing the wrapper 😄

I’ll anyway test your solution and check what happens, I think it’ll be disruptive (at least in ESP32) since second begin will overwrite the pinout matrix.

I’ll let you know soon.

Thank you

jgromes commented 4 years ago

I think it should be enough to discard the default SPI arguments in Module constructors and add new constructors that don't have SPIClass and SPISettings args. That way, when user wants to use the default SPI, they can just call the constructor with no SPI arguments.

jgromes commented 4 years ago

@Guglio95 Module::init() will now initialize the SPI interface only if it the default interface is used. Tested on Uno with default SPI and STM32 with non-default SPI and seems to be working, feel free to reopen if there are issues on ESP32.

aguglie commented 4 years ago

Hi Jan,

Thanks, I was about to text you, This will for sure sort out the problem.

I found an easier way for my specific case, I discovered that when using the right platformio's board definition the default pins (miso/mosi/...) are changed according to the labels printed on the board. But, in any case, with your change I'm able to customize the used pin as I wish.

Thanks :smile: