Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPI object pointer inside library [SPI.setX (x = MISO, MOSI, SCLK, SSEL)] does not work. MCU hangs in _spi->begin() #2597

Closed
dzalf opened this issue Dec 9, 2024 · 3 comments
Labels
invalid This doesn't seem right

Comments

@dzalf
Copy link

dzalf commented Dec 9, 2024

Hi all

I will try to be as succinct as possible. Please bear with me.

In short. I am developing a custom library for the ADS79xx family of ADCs to use the library as a cross-platform one. So far, I have successfully tested it for ESP32, however, the implementation for STM32 fails when invoking the SPI set_Pin_ group:

    _mySPI->setMISO(_misoPin);
    _mySPI->setMOSI(_mosiPin);
    _mySPI->setSCLK(_sclkPin);
    _mySPI->setSSEL(_csPin);

but works when externally declaring the SPI object as:

SPIClass spi(MOSI_PIN, MISO_PIN, SCLK_PIN);

and passing it to the library.

EDIT: I forgot to mention that I am working on a custom board definition for the WeAct Mini Core STM32G474CEU which has no official support in platformio, however, I have started working on my own files. So far, they have worked fine (I2C works well, ADCs have some inaccuracies and it is still subject to thorough evaluation). I have started a support request here, however, I have not received any response yet.

My library goes along the following lines:

ADS79xx.h (skimmed version)

#ifndef ADC_H
#define ADC_H

#include <Arduino.h>
#include <stdint.h>
#include <SPI.h>
#include <HardwareSerial.h>

// Number channels in the ADC
// 12 for ADS7952, 16 for ADS7953
// Use 16 to always have array space for channel data
#define ADC_CHANNELS 16

// Reference voltage (with doubler enabled)
#define ADC_EXT_VREF 2.5000f
#define ADC_VREF 5.0000f

// #define DEBUG
#define DBGLN(x) (Serial.println(x));

// SPI Speed
#define ADS79xx_SPI_MAX_SPEED 20000000 // To achieve max throughput of 1 Msps
#define ADS79xx_SPI_NORMAL_SPEED 10000000

// ADC type
class ADS79xx
{

public:

    ADS79xx(SPIClass *spi = &SPI, uint8_t miso = 9, uint8_t mosi = 11, uint8_t sclk = 7, uint8_t cs = 5);

    void init();
    uint16_t read_channel(uint8_t channel);
    static void setSPIspeed(ADS79xx *adc, uint32_t speed);
    inline uint32_t getSPIspeed() { return _SPIspeed; };

protected:
    // Define some SPI pins
    uint8_t _misoPin;
    uint8_t _mosiPin;
    uint8_t _sclkPin;
    uint8_t _csPin; 

    HardwareSerial *_dbgPrt = nullptr; // For devices with native Serial (CDC) this changes
    SPIClass *_mySPI = nullptr;
    SPISettings _spi_settings;

    uint32_t _SPIspeed = ADS79xx_SPI_MAX_SPEED;
};

#endif // ADC_H

and the corresponding ADS79xx.cpp file: (skimmed version too)

#include "ADS79xx.h"

ADS79xx::ADS79xx(SPIClass *spi, uint8_t miso, uint8_t mosi, uint8_t sclk, uint8_t cs)
{
    _mySPI = spi;
    _misoPin = miso;
    _mosiPin = mosi;
    _sclkPin = sclk;
    _csPin = cs;
}

void ADS79xx::init()
{

#if defined(DEBUG)
    DBGLN("Initialising ADC...");
#endif

    // Set GPIO port for ADC CS
    pinMode(_csPin, OUTPUT);    // init_cs(adc->cs->pin, adc->cs->ddr);
    digitalWrite(_csPin, HIGH); // set_cs_high(adc->cs->pin, adc->cs->port);

    // Initialize SPI
    /*
    #if defined(ESP32) // Works fine by passing  &SPI

        DBGLN("Configuring SPI for ESP32...");
        _mySPI->begin(_sclkPin, _misoPin, _mosiPin, _csPin); // init_spi();

    #elif defined(STM32)
    */

    DBGLN("Configuring SPI Pins for STM32...");

    // These lines were commented to make the code work as described in the text below
   
    //_mySPI->setMISO(_misoPin);
    //_mySPI->setMOSI(_mosiPin);
    //_mySPI->setSCLK(_sclkPin);
    //_mySPI->setSSEL(_csPin);

    DBGLN("Beginning SPI for STM32...");

    _mySPI->begin();  // on STM32 the MCU hangs here if set{MISO, MOSI, SCLK, SSEL}(pin) are invoked

    // #endif

    DBGLN("Setting up SPI for STM32...");

    _spi_settings = SPISettings(_SPIspeed, MSBFIRST, SPI_MODE0);

#if defined(DEBUG)
    DBGLN("SPI Configured...");
#endif
}

...

my main code is the following:

main.cpp (skimmed version)

#include <Arduino.h>
#include <SPI.h>
#include <string.h>
#include "ADS79xx.h"

#define ESP32_BOARD 0
#define STM32_BOARD 1
#define BOARD_TYPE STM32_BOARD

// ADC and SPI configuration
const uint8_t NUM_CHANNELS = 16;   // Number of channels per ADC
const uint8_t TOTAL_CHANNELS = 32; // Total number of channels (two ADCs)

// SPI Pins
#if (BOARD_TYPE == ESP32_BOARD) // Works fine

const int MISO_PIN = 11;
const int MOSI_PIN = 9;
const int SCLK_PIN = 7;

// Chip Select Pins for two ADCs
const int CS_H_PIN = 5; // High ADC (Channels 16-31)
const int CS_L_PIN = 3; // Low ADC (Channels 0-15)

#elif (BOARD_TYPE == STM32_BOARD) // some issues

static const int MISO_PIN = PB4; // PB4  // 14 || PA6
static const int MOSI_PIN = PB5; // PB5  // 15 || PA7
static const int SCLK_PIN = PB3; // PB3  // 13  || PA5

// Chip Select Pins for two ADCs
const int CS_H_PIN = PB2; // PB6 High ADC (Channels 16-31)
const int CS_L_PIN = PB1; // PB7 Low ADC (Channels 0-15)

#endif

//* Objects

// This is the only way to prevent the MCU from hanging
SPIClass spi(MOSI_PIN, MISO_PIN, SCLK_PIN);

// Create two instances of the ADS79xx ADCs
ADS79xx adcH(&spi, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_H_PIN);
ADS79xx adcL(&spi, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_L_PIN);

// Array to hold the ADC instances, reordered for correct mapping
ADS79xx ADCs[2] = {adcL, adcH}; // adcL handles CH0-15, adcH handles CH16-31

void setup()
{
    // Start serial communication for debugging
    Serial.begin(2000000);
    delay(3000); // Allow time for initialization

    // Debug startup message
    Serial.println(__FILE__);
    Serial.println(F("*** ADS7953 16-CH 12-bit ADC Test ***"));

    Serial.println(F("Initialising ADC group..."));
    
    initialize_adcs();

    Serial.println(F("...done!"));

}

void loop()
{

    //* Plot raw and converted channel values
    raw_and_volts_printer(); // defined elsewhere

}

// Initialize both ADCs in the array
void initialize_adcs()
{
    
    for (uint8_t idx = 0; idx < 2; idx++)
    {
        ADCs[idx].init();
    }
    delay(1000); // Ensure ADCs are fully initialized
}

My code uses two ADCs to read 32 channels in total, however, the issues occur when setting up the SPI object pointer inside the library.

If I use the SPI object as follows:

// Create two instances of the ADS79xx ADCs
ADS79xx adcH(&SPI, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_H_PIN);
ADS79xx adcL(&SPI, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_L_PIN);

// Array to hold the ADC instances, reordered for correct mapping
ADS79xx ADCs[2] = {adcL, adcH}; // adcL handles CH0-15, adcH handles CH16-31

and I delegate the initialization of the pins to the ADS79xx::init() method:

inside ADS79xx.cpp init() method:

ADS79xx::init(){
...

    DBGLN("Configuring SPI Pins for STM32...");

    _mySPI->setMISO(_misoPin);
    _mySPI->setMOSI(_mosiPin);
   _mySPI->setSCLK(_sclkPin);
    _mySPI->setSSEL(_csPin);

    DBGLN("Beggining SPI for STM32...");

    _mySPI->begin();

...
}

The MCU hangs

In contrast, if I pre-declare the pins from the SPI object by using this approach (based on info found here and in other Forums):

// This is the only way to prevent the MCU from hanging
SPIClass spi(MOSI_PIN, MISO_PIN, SCLK_PIN);

then pass by reference to the objects:

// Create two instances of the ADS79xx ADCs
ADS79xx adcH(&spi, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_H_PIN);
ADS79xx adcL(&spi, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_L_PIN);

the MCU works normally. Here, I am feeding 1.21V and 5.00V to Channels 0 and 31, respectively:

image

Since I need to make this library work for other platforms, removing the option to pass the pins is far from ideal as the ESP32 implementation works fine (_mySPI->begin({pins} section) and thus I need to ask you for a solution to this.

Why the pin setter methods are not working but when declaring the SPI object in advance works fine?

Finally, if you allow me to ask a slightly off-topic question. What are the "standard" preprocessor directives to compile for different boards? Say ESP32 and STM32

I have tried with #if defined(STM32), however, that part of the code gets ignored.

Please advise

Cheers

🍻

@fpistm
Copy link
Member

fpistm commented Dec 9, 2024

As a first guess :

    _mySPI->setMISO(_misoPin);
    _mySPI->setMOSI(_mosiPin);
    _mySPI->setSCLK(_sclkPin);
    _mySPI->setSSEL(_csPin);

If you set a SSEL to the SPI class then you want the CS pin managed by the hardware and not by the software (your application).
In that case you have to provide a _cspin linked to the SPI instance.
If it is not, then the begin simply fail and you should see a debug message if the log are enabled.... and I think it is the case as you don't want the hardware to drive the CS pin.
Here the possible value:

#ifdef HAL_SPI_MODULE_ENABLED
WEAK const PinMap PinMap_SPI_SSEL[] = {
{PA_4, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
{PA_4_ALT1, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
{PA_15, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
{PA_15_ALT1, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
{PB_12, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
{PF_0, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
{NC, NP, 0}
};
#endif

So in your case:

static const int MISO_PIN = PB4; //  --> SPI1
static const int MOSI_PIN = PB5; // --> SPI1
static const int SCLK_PIN = PB3; // --> SPI1

// Chip Select Pins for two ADCs
const int CS_H_PIN = PB2; // --> not link to an SPI
const int CS_L_PIN = PB1; // --> not link to an SPI

So simply do not call this:
_mySPI->setSSEL(_csPin);
else you fall in this trap, preventing the SPI to preoperly been init:

if (spi_data == NP || spi_cntl == NP || obj->spi == NP) {
core_debug("ERROR: SPI pins mismatch\n");
return;

@fpistm fpistm added the waiting feedback Further information is required label Dec 9, 2024
@dzalf
Copy link
Author

dzalf commented Dec 9, 2024

Dear @fpistm

Aha! You got it.

So, I commented the line:

_mySPI-->setSSEL(_cs_Pin);

then, I passed a simple SPI object (with no instance)

// Create two instances of the ADS79xx ADCs
ADS79xx adcH(&SPI, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_H_PIN);
ADS79xx adcL(&SPI, MISO_PIN, MOSI_PIN, SCLK_PIN, CS_L_PIN);

and the code works as intended!

image

I believe that my brain was trying to mirror the same implementation from ESP32 where you can pass the CS pin without any issues but your explanation makes a lot of sense.

In my case, I want the library to take control over the CS pin since I am using two objects.

Now, regarding the comments you made "not link to an SPI" did you mean that these are not Chip Select pins by default at the hardware level? Or do you simply mean "do not link them to an SPI object"?

Finally, any comments regarding my last question on the preprocessor directives for compiling for STM2 or ESP32?

Thanks for your prompt answer!

🍻

P.S. It would be awesome if you could have a look at the support request link I posted above in the platformio repo for adding support for my board.

@fpistm
Copy link
Member

fpistm commented Dec 9, 2024

Now, regarding the comments you made "not link to an SPI" did you mean that these are not Chip Select pins by default at the hardware level? Or do you simply mean "do not link them to an SPI object"?

No, just he pin you set as CS is not link to a SPI peripheral (SPIx) that's all.

Finally, any comments regarding my last question on the preprocessor directives for compiling for STM2 or ESP32?

Official one is ARDUINO_ARCH_STM32.
It is Arduino specification:
https://arduino.github.io/arduino-cli/1.1/library-specification/#working-with-multiple-architectures

P.S. It would be awesome if you could have a look at the support request link I posted above in the platformio repo for adding support for my board.

Unfortunately, I do not support/use PIO. PIO integrate this core but we support only the use in Arduino ecosystem (IDE, cli).

@fpistm fpistm closed this as completed Dec 9, 2024
@fpistm fpistm added invalid This doesn't seem right and removed waiting feedback Further information is required labels Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
Development

No branches or pull requests

2 participants