espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
12.96k stars 7.29k forks source link

Multiple SPIClass objects cause failure in version 3.0.2 #9939

Open oxothnk423 opened 1 week ago

oxothnk423 commented 1 week ago

Board

custom PCB

Device Description

ESP32-S3 FN8 with three SPI devices: ICM 20948 9-axis IMU MS5611-01BA03 barometric pressure sensor 192x64 monochrome LCD

Hardware Configuration

ESP32-S3 FN8 with three SPI devices connected to default VSPI (FSPI) pins (MOSI, MISO, CLK) and separate chip select pins.

Version

latest master (checkout manually)

IDE Name

Arduino IDE 2.3.2

Operating System

Windows 10

Flash frequency

QIO 80Mhz

PSRAM enabled

no

Upload speed

921600

Description

In ESP32 version 2.0.14, I was able to initialize each SPI device separately with its own SPIClass, even though they're all pointing to the same bus (VSPI in this case). This allows for unique settings (such as chip select pin) per spi device / class. In setup(), each device's class would then need a call to SPIClass->begin(). In version 3.0.2, it seems repeat calls to spi->begin() for the same spi bus creates a conflict or failure, such that the code hangs (with no error thrown) when performing a call to spi->transfer().

A simplified version of the code is below, which works on 2.0.14 and DOES NOT work on 3.0.2. The specific conflict I believe is the second call to spi->begin for the second device. If this line (imu_vspi->begin(...);) is commented out, the code compiles and runs without hanging (even though the SPI communication isn't as expected since the second device is now not set up properly).

There are ways to work around this if I was only using exclusively my own code, but some popular libraries (for example I'm using u8g2 for the LCD) internally handle their own SPI device setup. This makes it nearly impossible to avoid repeat calls to spi->begin on the same bus. (in other words, I can set up a single SPIClass to manage my other devices, or I can initialize the u8g2 library, but not both).

Is there a better way to approach multiple SPI devices on the same bus? I'd love to learn I'm just doing something wrong... I'm happy to take advice on alternative approaches. But this approach worked great in V2.x, so I'm surprised to see it failing here in v3.x when there were no listed breaking changes in the SPI API's (as far as I saw).

Sketch

#include <SPI.h>

#define SPI_SS_IMU       10
#define SPI_MOSI         11
#define SPI_CLK          12
#define SPI_MISO         13  
#define SPI_SS_BARO      18

#if CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
#define VSPI FSPI
#endif

static const int spiClk = 1000000;  // 1 MHz

//uninitialised pointers to SPI objects
SPIClass * baro_vspi = NULL;
SPIClass * imu_vspi = NULL;

void setup() {
  delay(1000);
  Serial.begin(115200);
  delay(1000);
  Serial.println("Starting Setup");

  baro_vspi = new SPIClass(VSPI);
  baro_vspi->begin(SPI_CLK, SPI_MISO, SPI_MOSI, SPI_SS_BARO);
  pinMode(baro_vspi->pinSS(), OUTPUT);
  digitalWrite(baro_vspi->pinSS(), HIGH);

  imu_vspi = new SPIClass(VSPI);
  imu_vspi->begin(SPI_CLK, SPI_MISO, SPI_MOSI, SPI_SS_IMU);   // comment this line out to make it "work"
  pinMode(imu_vspi->pinSS(), OUTPUT);
  digitalWrite(imu_vspi->pinSS(), HIGH);
}

void loop() {
  spi_Command(baro_vspi, 0b01010101);  // junk data to illustrate usage
  delay(100);
  spi_Command(imu_vspi, 0b01010101);  // junk data to illustrate usage
  delay(100);
}

void spi_Command(SPIClass *spi, byte data) {

  spi->beginTransaction(SPISettings(spiClk, MSBFIRST, SPI_MODE0));    
  digitalWrite(spi->pinSS(), LOW);                                    
  spi->transfer(data);                // code hangs here, doesn't return
  digitalWrite(spi->pinSS(), HIGH);                                    
  spi->endTransaction();                                                
}

Debug Message

no visible errors.  As mentioned above, the code just hangs on the third to last line (it never returns from spi->transfer).

Other Steps to Reproduce

As mentioned, this has worked correctly on 2.0.14. I am able to initialize 3 separate SPIClasses, all pointing to VSPI, and each with their own spi->begin() call, and also initialize a fourth u8g2 library spi object.... and SPI transfers worked flawlessly among all of them.

Now in v3.0.2, I only seem to be able to make a single spi->begin call, which thus enables only one SPIClass, not multiple.

I have checked existing issues, online documentation and the Troubleshooting Guide

P-R-O-C-H-Y commented 6 days ago

Hi @oxothnk423, can you set Core debug level to "Verbose", and send the debug message? Maybe then we will have more info of what is happening.

Also can you test if it will work with this slightly changed code?

#include <SPI.h>

#define SPI_SS_IMU       10
#define SPI_MOSI         11
#define SPI_CLK          12
#define SPI_MISO         13  
#define SPI_SS_BARO      18

#if CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
#define VSPI FSPI
#endif

static const int spiClk = 1000000;  // 1 MHz

//uninitialised pointers to SPI objects
SPIClass * vspi = NULL;

void setup() {
  delay(1000);
  Serial.begin(115200);
  delay(1000);
  Serial.println("Starting Setup");

  vspi = new SPIClass(VSPI);
  vspi->begin(SPI_CLK, SPI_MISO, SPI_MOSI);

  pinMode(SPI_SS_BARO, OUTPUT);
  digitalWrite(SPI_SS_BARO, HIGH);

  pinMode(SPI_SS_IMU, OUTPUT);
  digitalWrite(SPI_SS_IMU, HIGH);
}

void loop() {
  spi_Command(vspi, 0b01010101, SPI_SS_BARO);  // junk data to illustrate usage
  delay(100);
  spi_Command(vspi, 0b01010101, SPI_SS_IMU);  // junk data to illustrate usage
  delay(100);
}

void spi_Command(SPIClass *spi, byte data, uint8_t ss_pin) {

  spi->beginTransaction(SPISettings(spiClk, MSBFIRST, SPI_MODE0));    
  digitalWrite(SPI_SS_BARO, LOW);                                    
  spi->transfer(data);                // code hangs here, doesn't return
  digitalWrite(SPI_SS_IMU, HIGH);                                    
  spi->endTransaction();                                                
}
me-no-dev commented 6 days ago

why use two instances of the same bus and not one? Try what @P-R-O-C-H-Y suggested and use one instance of the SPI bus.

oxothnk423 commented 6 days ago

Hi @me-no-dev and @P-R-O-C-H-Y, thanks for the replies! All help appreciated!

Regarding the comment and code suggestion to use just one Class instance, yes, this does work. And if I only needed two manually controlled SPI devices, I would do exactly that. But as mentioned in the original post, I'm also using library-managed SPI devices, where the library is also calling spi->begin. If I create and initialize a single SPI object myself for some devices, but the library is ALSO calling spi->begin, I get a conflict. Add I'm likely going to add in a second library for yet another SPI device, so there may be up to three calls to spi->begin(). This is the conflict I'm trying to resolve. As mentioned, it worked fine in V2.x, but started failing with v3.x.

For an abundance of clarity: My own single SPI Class (your suggested changes): works on 2.x and 3.x My two SPI Classes (my simple example in original post): works on 2.x, does not work on 3.x (fails on spi->transfer) My own single SPI Class, PLUS a library object that is also calling spi->begin(): works on 2.x, does not work on 3.x (fails on spi->transfer)

I'm hoping my original code above is the simplest form to diagnose this error, but I can post longer code with the specific libraries if it's more helpful to see "the real thing".

(and I'd be super happy to learn I'm trying to initialize SPI objects wrong (manually and via library)... so please correct me anywhere I may be mistaken. I'm just surprised the SPI communication was working so great in v2.x and now not in 3.x)

Here are the requested debug messages:

When I run the original [does not work] version with core debug set to "error", I get this message:

[ 2110][E][esp32-hal-cpu.c:121] addApbChangeCallback(): duplicate func=0x42004d18 arg=0x3fc93204

Here is the full verbose output below.. Thanks for any help or insight you can provide!

(not sure why it formats so big and bold..?)

[   116][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Dein[  1618][I][esp32-hal-periman.c:141] perimanSetPinBus(): Pin 19 already has type USB_DM (45) with bus 0x3fc95fb0
[  1619][I][esp32-hal-periman.c:141] perimanSetPinBus(): Pin 20 already has type USB_DP (46) with bus 0x3fc95fb0
Starting Setup
[  2627][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_SCK (34) successfully set to 0x42004d30
[  2628][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_MISO (35) successfully set to 0x42004c58
[  2640][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_MOSI (36) successfully set to 0x42004b80
[  2652][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_SS (37) successfully set to 0x42004a6c
[  2664][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2675][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type GPIO (1) with bus 0xd
[  2685][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type SPI_MASTER_SCK (34) with bus 0x1
[  2695][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2706][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 13 successfully set to type GPIO (1) with bus 0xe
[  2716][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 13 successfully set to type SPI_MASTER_MISO (35) with bus 0x1
[  2727][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2738][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 11 successfully set to type GPIO (1) with bus 0xc
[  2747][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 11 successfully set to type SPI_MASTER_MOSI (36) with bus 0x1
[  2758][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2769][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 18 successfully set to type GPIO (1) with bus 0x13
[  2779][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_SCK (34) successfully set to 0x42004d30
[  2791][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_MISO (35) successfully set to 0x42004c58
[  2803][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_MOSI (36) successfully set to 0x42004b80
[  2815][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type SPI_MASTER_SS (37) successfully set to 0x42004a6c
[  2826][E][esp32-hal-cpu.c:121] addApbChangeCallback(): duplicate func=0x420051fc arg=0x3fc93204
[  2835][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type INIT (0) with bus 0x0
[  2845][D][esp32-hal-spi.c:185] spiDetachBus(): Stopping SPI bus
[  2851][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 13 successfully set to type INIT (0) with bus 0x0
[  2860][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 11 successfully set to type INIT (0) with bus 0x0
[  2870][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type INIT (0) with bus 0x0
[  2880][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2891][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type GPIO (1) with bus 0xd
[  2900][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 12 successfully set to type SPI_MASTER_SCK (34) with bus 0x1
[  2911][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2922][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 13 successfully set to type GPIO (1) with bus 0xe
[  2932][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 13 successfully set to type SPI_MASTER_MISO (35) with bus 0x1
[  2942][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2953][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 11 successfully set to type GPIO (1) with bus 0xc
[  2963][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 11 successfully set to type SPI_MASTER_MOSI (36) with bus 0x1
[  2974][V][esp32-hal-periman.c:235] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x42028588
[  2985][V][esp32-hal-periman.c:160] perimanSetPinBus(): Pin 10 successfully set to type GPIO (1) with bus 0xb
=========== After Setup Start ============
INTERNAL Memory Info:
------------------------------------------
  Total Size        :   398196 B ( 388.9 KB)
  Free Bytes        :   365068 B ( 356.5 KB)
  Allocated Bytes   :    28032 B (  27.4 KB)
  Minimum Free Bytes:   359832 B ( 351.4 KB)
  Largest Free Block:   327668 B ( 320.0 KB)
------------------------------------------
GPIO Info:
------------------------------------------
  GPIO : BUS_TYPE[bus/unit][chan]
  --------------------------------------  
    10 : GPIO
    11 : SPI_MASTER_MOSI[0]
    12 : SPI_MASTER_SCK[0]
    13 : SPI_MASTER_MISO[0]
    18 : GPIO
    19 : USB_DM
    20 : USB_DP
    43 : UART_TX[0]
    44 : UART_RX[0]
============ After Setup End =============
me-no-dev commented 5 days ago

@P-R-O-C-H-Y will have a look why multiple begins would cause an issue, but in general we will fix it and you should use one instance of the class and call multiple begins, that will not cause issues. Using separate classes talking to the same hardware is a bad idea and for that to work, many things will need to change.

oxothnk423 commented 5 days ago

Hi again, Ok, learning more about Arduino's handling of SPI, I added a new test-case (#4) to the previously tried 3 cases:

1) My own single SPI Class (your suggested changes): works on 2.x and 3.x 2) My two SPI Classes (my simple example in original post): works on 2.x, does not work on 3.x (fails on spi->transfer) 3) My own single SPI Class, PLUS a library object that is also calling spi->begin(): works on 2.x, does not work on 3.x (fails on spi->transfer) 4) [new] Arduino's default static SPIClass instance, PLUS a (u8g2) library object that is also calling SPI.begin(). This works on 2.x and 3.x.

So the issue does appear to be specifically when two SPIClass instances are present. In case 3 above, I was initializing my own new instance, and the u8g2 library was initializing the default static instance provided in SPI.h, resulting in two separate class instances to the same SPI bus.

When only one class instance is present (case #4 above), you're correct @me-no-dev that multiple SPI.begin() calls works fine. (which is great, because each new library I use for a different device calls begin() on the default Arduino static SPIClass instance, so there wouldn't be an easy way around multiple begin() calls).

So to summarize: V3.x -- mutiple calls to begin() works fine as long as there's only a single SPIClass instance. V2.x -- apparently you can have multiple SPIClass instances and call begin() on all of them and it works just fine! Because it worked so well, I thought this was an acceptable way to do it! No wonder I didn't see anyone else having this issue in the 3.0 migration (haha).

Thanks very much for your attention and helpful comments.

enjoyneering commented 1 day ago

In this case, do you need to correct the example for the library?