SPI Multibyte read seems to drop the first two bytes

[mdroberts1243]

Hi.

I'm having a lot of trouble getting a multi-byte read working using the SPI. I've connected an ADXL345 accelerometer to an ESP32-S2-Solo-2 module.

When I read the x, y and z accelerometer settings using MB I don't seem to get the x value. The read_buffer locations for x contain the y value and the y locations contain the z value and the z locations contain 0.

To prove to myself the chip is working I read the six registers directly using a single-byte read and get the expected values. It is only my Multibyte (MB) function that seems to be having trouble.

The main.c code looks like this:

Code: Select all

static const char TAG[] = "main";

/* The (initially blank) structures we need for the 
 * ESP-IDF API calls.  These are retained.
 */
spi_device_handle_t adxl;    // this adxl is used by the API calls, it's global for convenience
static spi_bus_config_t buscfg; 
static spi_device_interface_config_t devcfg;

void app_main(void)
{
    /* Get pointers to the configuration data structures */
    spi_bus_config_t *buscfg_ptr = &buscfg;
    spi_device_interface_config_t *devcfg_ptr = &devcfg;

    /* Initialize the ADXL345 and check it is there */
    if (adxl345_init(adxl, buscfg_ptr, devcfg_ptr) != 0) {
        ESP_LOGE(TAG, "Initialization of the device failed!");
        abort();
    };
    /* Enable the device to produce data */
    ESP_LOGI(TAG, "Going into measurement mode");
    adxl345_set_power_mode(adxl, 1);

    ESP_LOGI(TAG, "Loop, reading raw xyz");
    while(1)
    {
        int16_t x, y, z;
    //    float xf, yf, zf;

        // try directly reading the x, y and z values from individual 8-bit registers
        x = ((int16_t)adxl345_get_register_value(adxl,ADXL345_DATAX1) << 8) + adxl345_get_register_value(adxl,ADXL345_DATAX0);
        y = ((int16_t)adxl345_get_register_value(adxl,ADXL345_DATAY1) << 8) + adxl345_get_register_value(adxl,ADXL345_DATAY0);
        z = ((int16_t)adxl345_get_register_value(adxl,ADXL345_DATAZ1) << 8) + adxl345_get_register_value(adxl,ADXL345_DATAZ0);
        ESP_LOGI(TAG, "Got Register Values x:%04x, y:%04x, z:%04x.",x,y,z);

        // using the 'raw' read function:
        adxl345_get_xyz(adxl, &x, &y, &z);
        ESP_LOGI(TAG, "Got x:%d, y:%d, z:%d.",x,y,z);

        // adxl345_get_g_xyz(adxl, &xf, &yf, &zf);
        // ESP_LOGI(TAG, "Got x:%f, y:%f, z:%f.",xf,yf,zf);
        vTaskDelay(100);
    };
}

You can see in the code where I try reading the registers individually, and then I try using the raw xyz function.

The initialization code looks like this:

Code: Select all

/***************************************************************************//**
 * @brief Initializes the communication peripheral and checks if the ADXL345
 *        part is present.
 *
 * @param *handle            - Pointer to the  SPI device handle.
 * @param *buscfg            - Pointer to the bus configuration
 * @param *devcfg            - Pointer to the device configuration
 *
 * @return status    - Result of the initialization procedure.
 *                     Example: -1 - SPI peripheral was not initialized or
 *                                   ADXL345 part is not present.
 *                               0 - SPI peripheral is initialized and
 *                                   ADXL345 part is present.
*******************************************************************************/
int32_t adxl345_init(spi_device_handle_t handle, spi_bus_config_t *buscfg, 
                        spi_device_interface_config_t *devcfg)
{
	esp_err_t ret;

    ESP_LOGI(TAG, "Initializing bus SPI%d...", ADXL_SPI+1);
    buscfg->miso_io_num = ADXL_MISO;
    buscfg->mosi_io_num = ADXL_MOSI;
    buscfg->sclk_io_num = ADXL_SCK;
    buscfg->quadwp_io_num = -1;
    buscfg->quadhd_io_num = -1;
    buscfg->flags = SPICOMMON_BUSFLAG_MASTER;
    
    //Initialize the SPI bus
    ret = spi_bus_initialize(ADXL_SPI, buscfg, SPI_DMA_CH_AUTO);
//    ret = spi_bus_initialize(ADXL_SPI, buscfg, 0);  // zero disables DMA for now

    if  (ret != ESP_OK) {
        ESP_LOGE(TAG, "Got an error configuring bus: %d", ret);
        return -1;
    }

    ESP_LOGI(TAG, "Adding ADXL345 device to SPI bus...");
    devcfg->command_bits = 16;              // two bytes for a command
    devcfg->address_bits = 0;               // no address phase for ADXL345
    devcfg->clock_speed_hz = ADXL_CLK_FREQ; // 2MHz should be conservative 
    devcfg->mode = 3;                       // SPI mode 3 CPOL=1 and CPHA=1
    devcfg->spics_io_num = ADXL_CS;
    devcfg->queue_size = 1;
    devcfg->flags = SPI_DEVICE_HALFDUPLEX;
    devcfg->input_delay_ns = ADXL_INPUT_DELAY_NS;  // should be zero delay until we figure it out
    
    //Attach the ADXL345 to the SPI bus
    // I'm using the global variable adxl handle because I couldn't get it to work
    // passing by reference.
    ret = spi_bus_add_device(ADXL_SPI, devcfg, &adxl);
    if  (ret != ESP_OK) {
        ESP_LOGE(TAG, "Got an error adding device: %d", ret);
        return -1;
    }

    ESP_LOGI(TAG, "Setting DATA_FORMAT to 0b00001000 (full range)");
    adxl345_set_register_value(adxl, ADXL345_DATA_FORMAT, 0b00001000);

    ESP_LOGI(TAG, "Reading the device ID");\
    // Again using the global handle
	if (adxl345_get_register_value(adxl, ADXL345_DEVID) != ADXL345_ID)
		return -1;

    ESP_LOGI(TAG, "Going out of measurement mode");
    adxl345_set_power_mode(adxl, 0);

	return 0;
}

A single register read is done by this function:

Code: Select all

/***************************************************************************//**
 * @brief Reads the value of a register.
 *
 * @param handle           - The SPI device handle.
 * @param register_address - Address of the register.
 *
 * @return register_value  - Value of the register.
*******************************************************************************/
uint8_t adxl345_get_register_value(spi_device_handle_t handle, uint8_t register_address)
{
	uint8_t data_buffer[2] = {0, 0};
	uint8_t register_value = 0;

    data_buffer[0] = ADXL345_SPI_READ | register_address;
	data_buffer[1] = 0;
    spi_transaction_t t = {
            .flags = SPI_TRANS_USE_RXDATA,
            .cmd = data_buffer[0]<<8 | data_buffer[1],
            .rxlength = 8,
    };
    esp_err_t ret = spi_device_polling_transmit(handle,&t);
    if  (ret != ESP_OK) {
        ESP_LOGE(TAG, "Got an error reading a register: %d, err: %d", register_address, ret);
    }
    register_value = t.rx_data[0];

    return register_value;
}

A multi-byte read is done by this function:

Code: Select all

/***************************************************************************//**
 * @brief Reads the raw output data of each axis.
 *
 * @param handle    - The SPI device handle.
 * @param x         - X-axis's output data.
 * @param y         - Y-axis's output data.
 * @param z         - Z-axis's output data.
 *
 * @return None.
*******************************************************************************/
void adxl345_get_xyz(spi_device_handle_t handle,
		     int16_t* x,
		     int16_t* y,
		     int16_t* z)
{
    uint8_t first_reg_address = ADXL345_DATAX0;
	uint8_t *read_buffer = heap_caps_malloc(8, MALLOC_CAP_DMA);  // allocate the read buffer so DMA works
    
    read_buffer[0] = ADXL345_SPI_READ | 
                        ADXL345_SPI_MB |
                        first_reg_address;
    read_buffer[1] = 0;

    spi_transaction_t t;
    memset(&t,0, sizeof(t));

    t.cmd = (uint16_t)(read_buffer[0]<<8) + read_buffer[1];
    t.rxlength = 7*8;
    t.rx_buffer = read_buffer;

    esp_err_t ret = spi_device_polling_transmit(handle,&t);
    if  (ret != ESP_OK) {
        ESP_LOGE(TAG, "Got an error reading xyz raw: err: %d", ret);
    }
    ESP_LOGI(TAG, "Sorted read_buffer = 0x%02x,0x%02x%02x,0x%02x%02x,0x%02x%02x",
            read_buffer[0], read_buffer[2], read_buffer[1], read_buffer[4], 
            read_buffer[3], read_buffer[6], read_buffer[5]);
    /* x = ((ADXL345_DATAX1) << 8) + ADXL345_DATAX0 */
    *x = ((int16_t)read_buffer[2] << 8) + read_buffer[1];
    /* y = ((ADXL345_DATAY1) << 8) + ADXL345_DATAY0 */
    *y = ((int16_t)read_buffer[4] << 8) + read_buffer[3];
    /* z = ((ADXL345_DATAZ1) << 8) + ADXL345_DATAZ0 */
    *z = ((int16_t)read_buffer[6] << 8) + read_buffer[5];
}

In the multi-byte function I put code to output the read_buffer contents in an ordered fashion so they could easily be compared to the individual register read output.

I determined that the y and z contents are shifted by looking at my debug output and flipping the board over so that the force of gravity would be negative in the z:

Board upright:

Code: Select all

I (18313) main: Got Register Values x:0000, y:ffffffea, z:00e5.
I (18313) adxl345: Sorted read_buffer = 0x00,0xffea,0x00e5,0x0000
I (18313) main: Got x:-22, y:229, z:0.
I (19313) main: Got Register Values x:0000, y:ffffffe8, z:00e5.
I (19313) adxl345: Sorted read_buffer = 0x00,0xffe8,0x00e5,0x0000
I (19313) main: Got x:-24, y:229, z:0.

Board flipped over:

Code: Select all

I (63313) main: Got Register Values x:0004, y:ffffffe5, z:fffffef5.
I (63313) adxl345: Sorted read_buffer = 0x00,0xffe5,0xfef5,0x0000
I (63313) main: Got x:-27, y:-267, z:0.
I (64313) main: Got Register Values x:0002, y:ffffffe4, z:fffffef4.
I (64313) adxl345: Sorted read_buffer = 0x00,0xffe4,0xfef4,0x0000
I (64313) main: Got x:-28, y:-268, z:0.

Clearly on the multi-byte read the x values are missing from the read_buffer, replaced by y, z replaces y, and the z locations are zero.

I've used an arduino library to confirm the chip is working correctly: https://github.com/wollewald/ADXL345_WE/tree/main

I must have something wrong in my driver, I really need to get this working on IDF for the application.

I appreciate any help and suggestions!
Thanks,
-mark.

[mdroberts1243]

Further to this, I decided to look at the multibyte transactions on an oscilloscope with a SPI decoder. I was able to capture and analyze a transaction with the board upright (positive 1g on Z) and upside down (-1g on Z). Both transactions seem to be fine, except an extra unexpected couple of bytes were output (I assume this is something to do with DMA making even blocks of four bytes).

I created a couple of slides with the two transactions which I'll try to attach here as a PDF.

Really puzzled why my driver isn't getting the right values in the read_buffer!
-mark.

Multi-byte read of ADXL345 on ESP32S2.pdf

Two scope traces of multibyte reads

(213.6 KiB) Downloaded 347 times

[MicroController]

devcfg->command_bits = 16; // two bytes for a command

Note that, for a read, the command is only one byte.
Looking at the scoped signals, things seem to work: After the command byte 0xF2 is sent, the ADXL responds with 6 bytes of data which look quite plausible (0xFFEC = -20, 0xFFF2 = -14, 0xFEF7 = -265).

t.rxlength = 7*8;

6*8 bits should be enough.

And make sure you free(read_buffer) at the end

Btw, if you'd use an int16_t* read_buffer you could just do *x = read_buffer[0]; *y = read_buffer[1]; *z = read_buffer[2];
DMA doesn't make sense for those small transfers, so you could just do away with it and make int16_t read_buffer[3] a local variable.

[mdroberts1243]

Thank-you very much for your ideas and quick response!

Note that, for a read, the command is only one byte.

This was the key point for me. I started out doing register writes (16 bit command) and reads (16-bit command still works, register value is in the first byte of t.rx_data) but apparently a 16-bit command screws up the multi-byte transfer so that one or two bytes are dropped from the read_buffer (the x bytes) and I got y where I expected to see x and z where I expected to see y.

This was very troubling given that (as noted) the scoped hardware looked to be operating perfectly.

I re-wrote the xyz read function, using spi_transaction_ext_t to override the command bits (the key to getting it working), and also used local variable for read_buffer, turned off DMA in the configuration, etc. to simplify things.

Now I see correct values in both single register reads and multi-byte reads, giving me:

Code: Select all

I (992313) main: Got x:0.003900, y:-0.039000, z:1.926600.
I (992813) main: Got x:0, y:-11, z:494.
I (992813) main: Got x:0.000000, y:-0.042900, z:1.926600.
I (993313) main: Got x:1, y:-12, z:494.

The new get xyz function looks like:

Code: Select all

/***************************************************************************//**
 * @brief Reads the raw output data of each axis.
 *
 * @param handle    - The SPI device handle.
 * @param x         - X-axis's output data.
 * @param y         - Y-axis's output data.
 * @param z         - Z-axis's output data.
 *
 * @return None.
*******************************************************************************/
void adxl345_get_xyz(spi_device_handle_t handle,
		     int16_t* x,
		     int16_t* y,
		     int16_t* z)
{
    uint8_t mb_read_datax0 = ADXL345_SPI_READ | 
                        ADXL345_SPI_MB | ADXL345_DATAX0;
    uint8_t read_buffer[8] = {0,0,0,0,0,0,0,0};

    spi_transaction_ext_t t; // supports variable command bits
    memset(&t,0, sizeof(t));

    t.base.flags = SPI_TRANS_VARIABLE_CMD;
    t.command_bits = 8;
    t.base.cmd = mb_read_datax0;
    t.base.rxlength = 6*8;
    t.base.rx_buffer = read_buffer;

    esp_err_t ret = spi_device_polling_transmit(handle, (spi_transaction_t *)&t);
    if  (ret != ESP_OK) {
        ESP_LOGE(TAG, "Got an error reading xyz raw: err: %d", ret);
    }

    /* x = ((ADXL345_DATAX1) << 8) + ADXL345_DATAX0 */
    *x = ((int16_t)read_buffer[1] << 8) + read_buffer[0];
    /* y = ((ADXL345_DATAY1) << 8) + ADXL345_DATAY0 */
    *y = ((int16_t)read_buffer[3] << 8) + read_buffer[2];
    /* z = ((ADXL345_DATAZ1) << 8) + ADXL345_DATAZ0 */
    *z = ((int16_t)read_buffer[5] << 8) + read_buffer[4];

}

Thanks again for taking the time to make a considered response!
-mark.