nRF24 / RF24

OSI Layer 2 driver for nRF24L01 on Arduino & Raspberry Pi/Linux Devices
https://nrf24.github.io/RF24
GNU General Public License v2.0
2.21k stars 1.02k forks source link

[Question] How to use ESP32 HSPI #722

Closed szerwi closed 3 years ago

szerwi commented 3 years ago

Hi,

Is it possible to use ESP32's HSPI instead of VSPI to communicate with nRF24L01? I would like to use pins 12, 13, 14 insead of 19, 23, 18.

TMRh20 commented 3 years ago

It should be possible but not currently supported. Requires a few modifications.

On Jan 25, 2021, at 3:46 PM, Piotr G notifications@github.com wrote:

 Hi,

Is it possible to use ESP32's VSPI instead of HSPI to communicate with nRF24L01? I would like to use pins 12, 13, 14 insead of 19, 23, 18.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

szerwi commented 3 years ago

I found the solution. I simply added SPI.begin(14, 12, 13, 15); before radio.begin();. Unfortunately, I have no idea if SPI.begin changed VSPI to HSPI or did it just change VSPI's pins? May it cause any problems or can I use it without any worry?

2bndy5 commented 3 years ago

Remember that the library instantiates the default SPI bus on this line. Not sure if that helps, but you could check if the radio is working by seeing what radio.begin() returns. Checking the configuration with radio.printDetails() is another more finite debugging option.

I've been toying with the idea of making the begin() take in a reference to an SPI object... Haven't reached any conclusion due to all the platforms that this library supports.

mikechy2005 commented 3 years ago

I did small modification that can use HSPI. Create a SPIClass pointer in sketch(SPIClass* hspi = new SPIClass(HSPI); Then pass it to RF24 constructer(RF24(hspi, cePin, csnPin);)

RF24.h Constructor RF24(SPIClass spi, ...):ptrSPI(spi), ... Private Add SPIClass ptrSPI = NULL;

RF24.cpp Replace all _SPI. With ptrSPI->

may be some code not exactly correct, but almost. Because I type the above drafty. I can get it works. Hope can help you.

szerwi commented 3 years ago

Is the modification in the library really neccessary? My solution (adding SPI.begin(14, 12, 13, 15); before radio.begin()) works well too :)

2bndy5 commented 3 years ago

@mikechy2005 is this code on your fork or is it private? Your modifications are exactly what I was thinking. TBH, I think this idea could use a separate issue/discussion.

@szerwi its possible your solution works without modifying the library because of the SPI implementation for the esp32 core (namely treating the SPI object as a singleton).

mikechy2005 commented 3 years ago

Is the modification in the library really neccessary? My solution (adding SPI.begin(14, 12, 13, 15); before radio.begin()) works well too :)

Obversely, your code also ok. I think yours could apply to use one SPI bus only. In my project, I use default VSPI for high speed(27mhz) for LCD which nRF24l01 not support. I want to separate SPI bus for coding easier.

In my way, you could choose VSPI/HSPI in sketch level. Depends on your projects.:)

mikechy2005 commented 3 years ago

@mikechy2005 is this code on your fork or is it private? Your modifications are exactly what I was thinking. TBH, I think this idea could use a separate issue/discussion.

@szerwi its possible your solution works without modifying the library because of the SPI implementation for the esp32 core (namely treating the SPI object as a singleton).

Just private , I did it in quick and dirty way to coding my project. Haha. Because I used SPI pointer in my other library, so modify this too and share to here:)

ex-caliper commented 3 years ago

@szerwi from your earlier comment, "I found the solution. I simply added SPI.begin(14, 12, 13, 15);" I am assuming these numbers are MOSI, MISO, SCK and SS arguments. I have tried relating this to the documentation and I am unfamiliar with the ESP32, so please could you put them in order. i.e. which one of the four does 14 represent.

Many thanks..

szerwi commented 3 years ago

@ex-caliper have a look at SPI.cpp: https://github.com/espressif/arduino-esp32/blob/6b0114366baf986c155e8173ab7c22bc0c5fcedc/libraries/SPI/src/SPI.cpp#L37 The correct order is: SCK, MISO, MOSI, SS

mauricio-linhares commented 3 years ago

@mikechy2005 I am working in a project that requires 2 SPI buses like yours. I intend to use the nRF24l01 in the HSPI, but can't manage to make the changes you suggested work. I modified the RF24.h and RF24.cpp files, but it fails to compile. If you could detail it more, it would help a lot.

mikechy2005 commented 3 years ago

@mikechy2005 I am working in a project that requires 2 SPI buses like yours. I intend to use the nRF24l01 in the HSPI, but can't manage to make the changes you suggested work. I modified the RF24.h and RF24.cpp files, but it fails to compile. If you could detail it more, it would help a lot.

@mauricio-linhares First of all, i did it in OOP way.

  1. I remove Line 113-124 and add new 1 line SPIClass* ptrSPI; in RF24.h
  2. Add replace line 180 (constructor) with RF24(SPIClass* spi, uint16_t _cepin, uint16_t _cspin, uint32_t _spispeed = RF24_SPI_SPEED); in RF24.h
  3. Add ptrSPI(spi), before ce_pin(_cepin) in line 424 in RF24.cpp
  4. Find all "_SPI." and replace with "ptrSPI->" in RF24.cpp
  5. Finally create HSPI pointer (SPIClass* _hspi = SPIClass(HSPI); ) in sketch and init RF24 with (_hspi, cePin.......

I create all object in object pointer since i always wrote my project by OOP. You may follow OOP concept to try my code. Hope can help you.

mauricio-linhares commented 3 years ago

@mikechy2005 It is working perfectly now. Much better to have each SPI device its own dedicated bus. I've been smashing my head for several days trying to make this work, luckily I found this issue open. You helped a lot! Thank you!!

gfvalvo commented 3 years ago

I've been toying with the idea of making the begin() take in a reference to an SPI object... Haven't reached any conclusion due to all the platforms that this library supports.

Or a pointer ..... that would be great!!! I was thinking about hacking it in myself.

2bndy5 commented 3 years ago

I'm working on this feature to use a secondary SPI bus by overloading begin()... I think its ready for testing (need testers). The overload-begin branch has the changes. I've also added example source code specific for the ESP32.

makerMcl commented 3 years ago

Just for report/feedback: I tried workaround from @szerwi with SPI.begin() before radio.begin() -> result was kernel panic to illegal write at 0x400d6828: SFE_BMP180::writeBytes(unsigned char*, char) .pio\libdeps\waterSensorCOM\BMP180/SFE_BMP180.cpp:207 This configuration also uses I2C - I did not expect this side effect but there was no other change. After removing that SPI.begin()-hack my sketch runs without kernel panics as before. I will now test that solution in the branch before starting my own fork...

makerMcl commented 3 years ago

I have tested the solution in that branch now, I can confirm it works! Good work, I like the OOP approach (it's my world too). Tested with a ESP-32 DevKit v1, with some interfaces (I2C, Onewire, UART0, AsyncTCP) in parallel, without any problems. Used the pattern from the example but referenced inside a class instance (instead of static/main loop). Do you need more docs?

I would really appreciate it if you can merge that branch so I can get back to get the lib via platformio registry...

szerwi commented 3 years ago

@makerMcl hmm, that's strange. I successfully used nRF24L01 together with SC16IS752 expander over I2C. Here is part of my setup:

void setup()
{
  Serial.begin(9600);

  Wire.begin(4, 16);
  i2cuart.beginA(SC16IS752_DEFAULT_SPEED);

  SPI.begin(14, 12, 13, 15); //HSPI
  radio.begin();
  radio.setChannel(100);
  radio.setDataRate(RF24_250KBPS);
  radio.setPALevel(RF24_PA_MAX);
  radio.openReadingPipe(1, masterPipe);
  radio.openWritingPipe(slavePipe);
  radio.startListening();
  radio.printDetails();
}

Maybe that's because I use I2C on different pins (4 and 16).

@2bndy5 when do you plan to implement it into stable release?

2bndy5 commented 3 years ago

@szerwi my branch needs testing. It covers more than just esp32. I don't own a teensy board that could test my solution.

To be clear, my solution is not the same as @szerwi solution. ~@szerwi your solution breaks features that others may depend on~

@makerMcl thank you so much for validation. If you want you can post you code, so others (not just me) can follow your lead. Although, the example snippet I added the the docs is meant to be as simple and straight forward as possible (to avoid confusion).

Also I think pio has a mechanism to use a specific lib folder as a dependency in your project's platformio.ini. we are aware that v1.3.12 is not showing in pio registry

szerwi commented 3 years ago

@2bndy5 what exactly can my solution break? Currently I am just testing if all components are working for bigger project. I guess I will use your solution in the final code :D

2bndy5 commented 3 years ago

@szerwi I'm sorry. I was confusing your solution with @mikechy2005 solution. @mikechy2005 solution breaks features.

@szerwi solution will work better with my branch (the code @szerwi posted is not identical to the example snippet I added to the docs) because my branch actually allows the user to call SPIClass::begin() separately from radio.begin(_SPI*). Additionally, I think my solution also supports espressif's ArduinoCore's soft SPI implementation.

makerMcl commented 3 years ago

@2bndy5 fyi - Regarding the code I used - it is the same as in your example;

    void init(const byte pinCe, const byte pinCsn)
    {
        hspi = new SPIClass(HSPI); // by default VSPI is used
        hspi->begin();
        radio = new RF24(pinCe, pinCsn);
        if (radio->begin(hspi))
        {
            // AutoACK is on by default
            radio->setChannel(67);
            radio->setPALevel(RF24_PA_MIN);
            radio->enableDynamicPayloads();
            radio->openReadingPipe(0, receiveAddress);
            radio->openWritingPipe(sendAddress);
            radio->startListening();
            ui.logInfo() << "radio initialized (chipConnected=" << radio->isChipConnected() << ") with datarate=" << radio->getDataRate() << ", PAlvl=" << radio->getPALevel() << ", crcLength=" << radio->getCRCLength() << endl;
        }
        else
        {
            ui.logError() << "RF24 radio: chip is not responding" << endl;
        }
    }
2bndy5 commented 3 years ago

@makerMcl good show! What logging lib are you using? (I assume it works on ESP32)

I'm curious about the log lib you're calling to (the ui.log*()) because we're looking to unify the RF24* libs' stdout usage (which currently employs printf()) on all supported platforms.

makerMcl commented 3 years ago

Sorry for the late response. It is something self-built, I have out-factored some common tasks there and for logging it's main benefit is a rolling buffer for log-output via webserver. You can find it on https://github.com/makerMcl/universalUi, sample usage in https://github.com/makerMcl/calibrationServer.

If you are thinking about logging, I can only share my humble experience: for now I found the most simple solution (KISS principle) with macros, see line 90-101 of https://github.com/makerMcl/universalUi/blob/master/hc12tool.h for it. There are some logging frameworks, and also ESP - for my needs it was too many bytes used for program code. For a lib like NRF24 I would assume different requirements.

2bndy5 commented 3 years ago

Thanks for the lead! Yes our RF24Log lib will use a singleton with macros API as well. But, extending it to all possible supported platforms (x86 included), while implementing our own printf() specifiers & log level filtering, for use with all possible output streams... Well, that's what's taking up all the program space. We hope to encompass more use cases than just RF24 libs... LED matrices, file output, multiple output streams, ncurses, etc...

kshitijgarg2609 commented 3 years ago
SPIClass *sp;
RF24 radio(2,15);

void setup()
{
  Serial.begin(115200);
  initRF();
}

void initRF()
{
  sp = new SPIClass(HSPI);
  sp->begin();
  if(radio.begin(sp))
  {
    Serial.println("Radio Started !!!");
    radio.openWritingPipe(addr2);
    radio.openReadingPipe(1, addr1);
    radio.startListening();
  }
  else
  {
    Serial.println("Radio couldn't start !!!");
  }
}

I have been trying to use HSPI of ESP32 but this library is also not working with VSPI of ESP32. Ihave shared the snippet of the code I have been trying.

I connected pins :- nrf esp32
mosi 13
miso 12
sck 14
csn 15
ce 2
2bndy5 commented 3 years ago

@kshitijgarg2609 what exact board are you using (ESP32 is a chip used on several MCU boards)? The fact that VSPI doesn't work for you (assuming you use the normal RF24::begin() without passing a parameter to use VSPI) tells me you may be facing a wiring problem.

Try changing the wires (shortest wires possible) and make sure the connections are tight. Please note that sometimes the MCU boards' have very high impedance on the SPI bus pins used which makes long wires actual corrupt the data going to the radio module (thus begin() will return false).

If finding short wires is a problem, try lowering the SPI frequency by specifying it in the c'tor:

RF24 radio(2, 15, 4000000); // uses 4 MHz instead of default 10 MHz
kshitijgarg2609 commented 3 years ago

I have resolved myself, I was going according to the documentation and trying HSPI on ESP32 CAM, RF24(_SPI) doesn't work but the SPI.begin(SCK,MISO,MOSI,SS); worked.

Thank you

Frenzyritz13 commented 3 years ago

Hey, I have been trying to use SPI.begin to run the HSPI bus with my nrf24l01+ module but I am unabl to do so. I am also using i2c on pins 21 and 22. My module's CE pin is being handled by the MCP27013 GPIO expander Any suggestions on how I should go about troubleshooting this?

2bndy5 commented 3 years ago

@Frenzyritz13 Have you tried following the docs? I only ask because you didn't mention it.

It's a little strange to have a vital pin be controlled by another slave device. This slows down everything you do with the nRF24L01 because you have to do a complete I2C transaction just to start/stop listening and transmit anything. Technically this could be easily achieved in CircuitPython, but not in Arduino land. The uint16_t you pass to the constructor's _ce_pin parameter is used in pinmode() & digitalwrite() calls. I don't see an easy way to disguise the CE pin as an extended device's API.

I'd suggest using inheritance to make adjustments to how the CE pin is controlled, but the ce() is private so no help there. You'd have to modify the RF24 source code to make the ce() function use the MCP23017 API. And don't forget to modify isValid() & the private _init_pins() functions to accommodate for whatever adjustments you make to the ce_pin member (& constructor parameter).

kshitijgarg2609 commented 3 years ago

@Frenzyritz13 I had also tried HSPI but later on I used VSPI with variable pins. For using different pins of VSPI you have to call SPI.begin(sck_pin,miso_pin,mosi_pin,ss_pin) before RF24.begin(). HSPI is needed when you are using some other SPI device along with rf24 device. One unique feature of esp32 is that pins of i2c and spi can be mapped to any pins.

https://github.com/espressif/arduino-esp32/blob/master/libraries/SPI/src/SPI.h this link contains the class and functions of SPI ESP32.

2bndy5 commented 3 years ago

Just to clarify, mapping the SPI bus or I2C bus to any other pins is specific to the ESP32 Arduino core (not the hardware). Technically the Arduino core invokes bit-banged (software-driven) solutions when manually mapping SPI/I2C pins to GPIO pins that weren't designed for hardware-driven serialized buses. The core's docs neglect to adequately warn users that these bit-banged solutions are much slower than using the default hardware capable pins.

kshitijgarg2609 commented 3 years ago

@2bndy5 can you send me that docs or link where it is written that pin mapping of esp32 spi pins makes communication slow. Because I have experimented esp32 cam streaming over rf24 which has less pins and vspi is occupied by the sd card reader, so I just have to map spi pins, now on the receiever side I have used esp8266 and rf24 which receives data frame from rf24 of esp32 and combines packet as the mtu size is just 32 bytes, and sends over to wifi.

The result I found is that I achieved 5-6 fps on the resolution of QQVGA which is just enough and not that much real time.

kshitijgarg2609 commented 3 years ago

image I am facing this issue that SPIClass pointer or function I am passing in image gets compiled but doesn't work.

2bndy5 commented 3 years ago

@kshitijgarg2609 The docs are seriously lacking about the SPI interface, but I managed to find this table that hints to my clarification. The SPI source relies heavily on espressif's ESP32 SDK.

gets compiled but doesn't work.

This very vague. How is it not working? Please don't post pictures of code; I can't copy-n-paste adjustments if its a picture.

kshitijgarg2609 commented 3 years ago

@2bndy5The pictures of code is from specific functions of .h files given on https://github.com/nRF24/RF24/blob/master/RF24.h and https://github.com/espressif/arduino-esp32/blob/master/libraries/SPI/src/SPI.h

SPIClass *sp;
RF24 radio(2,15);

void setup()
{
  Serial.begin(115200);
  initRF();
}

void initRF()
{
  sp = new SPIClass(HSPI);
  sp->begin();
  if(radio.begin(sp))
  {
    Serial.println("Radio Started !!!");
    radio.openWritingPipe(addr2);
    radio.openReadingPipe(1, addr1);
    radio.startListening();
  }
  else
  {
    Serial.println("Radio couldn't start !!!");
  }
}

This was the code posted earlier that rf24 is not working on this SPIClass(HSPI). But this library is working great on VSPI. Sorry for taking much of your time.

2bndy5 commented 3 years ago

I don't see anything wrong with this code. The only difference I see is that you passed the SPI_SS pin to the RF24 c'tor's CSN parameter (this is not wrong). Although, the RF24 lib assumes it can handle the CSN pin independently from the other SPI pins.

It is common for SPI slave device drivers to handle the CSN pin independently. In fact the SPI_SS pin is usually still manipulated by the Arduino core while the actual CSN pin is manipulated by the RF24 lib. This better suites the cross-platform compatibility we hold dearly.

Frenzyritz13 commented 3 years ago

@Frenzyritz13 I had also tried HSPI but later on I used VSPI with variable pins. For using different pins of VSPI you have to call SPI.begin(sck_pin,miso_pin,mosi_pin,ss_pin) before RF24.begin(). HSPI is needed when you are using some other SPI device along with rf24 device. One unique feature of esp32 is that pins of i2c and spi can be mapped to any pins.

https://github.com/espressif/arduino-esp32/blob/master/libraries/SPI/src/SPI.h this link contains the class and functions of SPI ESP32.

Hey, this makes sense, I will try this.

Frenzyritz13 commented 3 years ago

@Frenzyritz13 I had also tried HSPI but later on I used VSPI with variable pins. For using different pins of VSPI you have to call SPI.begin(sck_pin,miso_pin,mosi_pin,ss_pin) before RF24.begin(). HSPI is needed when you are using some other SPI device along with rf24 device. One unique feature of esp32 is that pins of i2c and spi can be mapped to any pins. https://github.com/espressif/arduino-esp32/blob/master/libraries/SPI/src/SPI.h this link contains the class and functions of SPI ESP32.

Hey, this makes sense, I will try this.

Update: I tried remapping the VSPI pins but couldn't do so. I used HSPI and it seemed to start the communication between my 2 esps. Here is the code I ended up using:

SPIClass * hspi = NULL; //Before void setup

//In void setup==>

 hspi = new SPIClass(HSPI); // by default VSPI is used
  hspi->begin();
  if (!radio.begin(hspi)) {
    Serial.println(F("radio hardware is not responding!!"));
    while (1) {} // hold in infinite loop
  }

This code runs successfully with the MCP23017 expander. I called the adafruit library in the RF24 library itself. Here is the modification I have made to the library.==>

If anyone has better ways of going about doing this, do le me know.

#include <Adafruit_MCP23X17.h>
Adafruit_MCP23X17 mcp;

void RF24::ce(bool level)
{
    bool checkBegin;

    //Allow for 3-pin use on ATTiny
    checkBegin = mcp.begin_I2C();

    if (ce_pin != csn_pin)
    {
        if (!checkBegin)
        {
            digitalWrite(ce_pin, level);
            Serial.println("Non-expander mode/Error in starting I2C");
        }
        else if (checkBegin)
        {
            mcp.pinMode(4, OUTPUT);
            mcp.digitalWrite(4, level);
        }
    }
}
2bndy5 commented 3 years ago

If that's all the modification that was needed to make the MCP23017 work, then you might be able to reduce the compile size with a new macro defined in _RF24config.h

#else //Everything else
    #include <Arduino.h>

    // This macro assumes that MCP23017::begin() was called by the application prior to RF24::begin().
    #define USE_MCP23XXX_AS_CE mcp // the global MCP23017 object declared in main application's code space

Then alter the ce() function in RF24.cpp

void RF24::ce(bool level)
{
#if defined USE_MCP23XXX_AS_CE
    USE_MCP23XXX_AS_CE.digitalWrite(ce_pin, level);
#else // !defined(USE_MCP23XXX_AS_CE)
    if (ce_pin != csn_pin)
    {
        digitalWrite(ce_pin, level);
    }
#endif // !defined(USE_MCP23XXX_AS_CE)
}

Lastly, I still think _init_pins() function (in RF24.cpp) should be altered to set pinmode() for a pin number that's designated for a MCP23017 (modification is toward the bottom and would only apply to Arduino platforms).

// This modification assumes you passed the MCP23017 pin number to the RF24 constructor's _ce_pin parameter

bool RF24::_init_pins()
{
    if (!isValid()) {
        // didn't specify the CSN & CE pins to c'tor nor begin()
        return false;
    }

    #if defined (RF24_LINUX)

        #if defined (MRAA)
    GPIO();
    gpio.begin(ce_pin, csn_pin);
        #endif

    pinMode(ce_pin, OUTPUT);
    ce(LOW);
    delay(100);

    #elif defined (LITTLEWIRE)
    pinMode(csn_pin, OUTPUT);
    csn(HIGH);

    #elif defined (XMEGA_D3)
    if (ce_pin != csn_pin) {
        pinMode(ce_pin, OUTPUT);
    };
    ce(LOW);
    csn(HIGH);
    delay(200);

    #else // using an Arduino platform

    // Initialize pins
    if (ce_pin != csn_pin) {
        #if !defined (USE_MCP23XXX_AS_CE)
        pinMode(ce_pin, OUTPUT);
        #endif // !defined (USE_MCP23XXX_AS_CE)
        pinMode(csn_pin, OUTPUT);
    }

        #if defined (USE_MCP23XXX_AS_CE)
    USE_MCP23XXX_AS_CE.pinMode(ce_pin, OUTPUT);
        #endif // defined(USE_MCP23XXX_AS_CE)

    ce(LOW);
    csn(HIGH);

        #if defined (__ARDUINO_X86__)
    delay(100);
        #endif
    #endif // !defined(XMEGA_D3) && !defined(LITTLEWIRE) && !defined(RF24_LINUX)

    return true; // assuming pins are connected properly
}

Please note that my code suggestions are contribution compatible. So, if you find that this solution works, then you can make a Pull Request to add the new featured macro for others to benefit from your hard work/frustration.

Frenzyritz13 commented 3 years ago

If that's all the modification that was needed to make the MCP23017 work, then you might be able to reduce the compile size with a new macro defined in _RF24config.h

EDITED! please see above updated post

Then alter the ce() function in RF24.cpp

EDITED! please see above updated post

Lastly, I still think _init_pins() function (in RF24.cpp) should be altered to set pinmode() for a pin number that's designated for a MCP23017 (modification is toward the bottom and would only apply to Arduino platforms).

EDITED! please see above updated post

Please note that my code suggestions are contribution compatible. So, if you find that this solution works, then you can make a Pull Request to add the new featured macro for others to benefit from your hard work/frustration.

This looks good Brendan! Will test it and get back to you. Also, I am going to be using 2 I2Cs and an Expander based interrupt, I am expecting some issues there as well, will keep this thread updated.

2bndy5 commented 3 years ago

@Frenzyritz13 I just modified my post (found/fixed an error in the logic). Please be sure to use the updated snippet about _ini_pins()

This looks good Brendon! Will test it and get back to you. Also, I am going to be using 2 I2Cs and an Expander based interrupt, I am expecting some issues there as well, will keep this thread updated.

This deserves a separate thread/issue because we are no longer discussing just the ESP32's SPI bus.