Multi-instance TWI devices: style question & enhancement suggestion

[Ray]

I have run into a challenge with the app_twi API, and I'd like to run it by you for your advice. To be clear, I am not blocked. But I am not pleased with the source code maintainability aesthetics of my current solution and I'd either like your advice or an API enhancement; not sure which.

In general, I like the static-array oriented programming style of setting up arrays of TWI transfers. For my current project I have written about a dozen drivers of varying complexity, with tables such as you'll see at the bottom of this post.

But I just ran into a challenge. As I'm sure you're aware, many if not most chips are designed with at least two hard-wired I2C addresses, using one or two pins to program which of those addresses to use.

In this project I have found the inescapable need to have TWO instances of identical chips within the design, each using a unique I2C address. At first, I thought "ok, no problem, I'll just parameterize the driver". I figured that at driver init I'd just pass an argument as to which instance I am initializing, etc.

However, it isn't that simple. At all. As it turns out, all the macros for API_TWI_READ/WRITE are designed around static const-based tables. Even if it were possible to write code to dynamically rewrite the I2C_ADDRESS (operation) field prior to the app_twi_schedule, it is certainly not stylistically the intent of those macros or that function to allow dynamic reassignment. The developer's intent was clearly that the driver would be statically-compiled for a single I2C address.

Right now, the way I am solving this (and I find it ugly) by placing my entire complex driver, in essence, into a header file that is #included by two other files that generate two runtime instances of the driver with different I2C addresses. This works just fine functionally, but I just don't think it's a good practice stylistically.

Is there a different coding style for multi-instance that was intended by the app_twi developer?

I've thought about what I might like to see in terms of an API change to make this easier for the developer, and so please do take this constructively as an enhancement suggestion:

I'd be very surprised if a high percentage of app_twi_transfer tables intermix multiple I2C addresses in the same transfer. As such, I might suggest introducing a new 3-argument form of APP_TWI_READ and APP_TWI_WRITE that don't have any I2C addresses specified. When using this form, you might then at runtime place the I2C address directly into an argument in a NON-const non-static app_twi_transaction, or into a new variation of the call to app_twi_schedule itself.

I'd be surprised if this didn't then become the most commonly-used API mechanism, given that most drivers issue all commands to the same address.

Looking forward to your thoughts.

static app_twi_transfer_t const transfers[] = {
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &status_aux[0], sizeof(status_aux[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &status_aux[1], sizeof(status_aux[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc1_l[0], sizeof(out_adc1_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc1_l[1], sizeof(out_adc1_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc1_h[0], sizeof(out_adc1_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc1_h[1], sizeof(out_adc1_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc2_l[0], sizeof(out_adc2_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc2_l[1], sizeof(out_adc2_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc2_h[0], sizeof(out_adc2_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc2_h[1], sizeof(out_adc2_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc3_l[0], sizeof(out_adc3_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc3_l[1], sizeof(out_adc3_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_adc3_h[0], sizeof(out_adc3_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_adc3_h[1], sizeof(out_adc3_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_x_l[0], sizeof(out_x_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_x_l[1], sizeof(out_x_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_x_h[0], sizeof(out_x_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_x_h[1], sizeof(out_x_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_y_l[0], sizeof(out_y_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_y_l[1], sizeof(out_y_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_y_h[0], sizeof(out_y_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_y_h[1], sizeof(out_y_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_z_l[0], sizeof(out_z_l[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_z_l[1], sizeof(out_z_l[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &out_z_h[0], sizeof(out_z_h[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &out_z_h[1], sizeof(out_z_h[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &int1_cfg[0], sizeof(int1_cfg[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &int1_cfg[1], sizeof(int1_cfg[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &int1_ths[0], sizeof(int1_ths[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &int1_ths[1], sizeof(int1_ths[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &int1_src[0], sizeof(int1_src[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &int1_src[1], sizeof(int1_src[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &who[0], sizeof(who[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &who[1], sizeof(who[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg1[0], sizeof(reg1[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg1[1], sizeof(reg1[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg2[0], sizeof(reg2[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg2[1], sizeof(reg2[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg3[0], sizeof(reg3[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg3[1], sizeof(reg3[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg4[0], sizeof(reg4[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg4[1], sizeof(reg4[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg5[0], sizeof(reg5[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg5[1], sizeof(reg5[1]), 0),
    APP_TWI_WRITE(LIS_I2C_ADDRESS, &reg6[0], sizeof(reg6[0]), APP_TWI_NO_STOP),
    APP_TWI_READ(LIS_I2C_ADDRESS, &reg6[1], sizeof(reg6[1]), 0),
};

[Nguyen Hoan Hoang]

Perhaps you could have a look at the EHAL library from this blog. It's reusable object oriented multi-architecture. UART, SPI & I2C are available. An example for reading any EEPROM connected to I2C

static const I2CCFG s_I2C_CfgData = {
    0,  // i2c device number
    {
        { SDA_PORT, SDA_PIN, SDA_PINOP },
        { SCL_PORT, SCL_PIN, SCL_PINOP },
    },
    100000,     // Rate in Hz
    I2CMODE_MASTER,
    0,              // Device address (slave mode only)
    5,              // Maximum number of retries
    APP_IRQ_PRIORITY_LOW,      // Interrupt priority
   
};

I2C g_I2C_Instance;

static const I2CCFG s_I2C_CfgData2 = {
    1,  // i2c device number
    {
        { SDA_PORT, SDA_PIN, SDA_PINOP },
        { SCL_PORT, SCL_PIN, SCL_PINOP },
    },
    100000,     // Rate in Hz
    I2CMODE_MASTER,
    0,              // Device address (slave mode only)
    5,              // Maximum number of retries
    APP_IRQ_PRIORITY_LOW,      // Interrupt priority
   
};

I2C g_I2C_Instance2;

static const SEEP_CFG s_Eeprom_CfgData = {
    (0xA2 >> 1),    // Dev addr (7bits address)
    2,                   // Addressing length in bytes
    32,                 // Page size in bytes
    8192,              // 8KB eeprom
    5,                   // Write delays in sec
    {-1, -1, },
};
 
SEEP g_Eeprom_Instance;

int main()
{
    g_I2C_Instance.Init(s_I2C_CfgData);
    g_I2C_Instance2.Init(s_I2C_CfgData2);

    // use i2c0 instance 1
    g_Eeprom_Instance.Init(s_Eeprom_CfgData, &g_I2C_Instance);

    // use i2c1 instance 2
    g_Eeprom_Instance.Init(s_Eeprom_CfgData, &g_I2C_Instance2);

    uint8_t d[32];
    
    // Read 32 bytes eeprom at address 0x40
    // returns number of bytes read
    int count = g_Eeprom_Instance.Read(0x40, d, 32);

    // multiple devices using same I2C interface
    g_Dev2_Instance.Init(s_Dev2_CfgData, &g_I2C_Instance);
    count = g_Dev2_Instance.Read(0x40, d, 32);

    // your device can support SPI want to use SPI instead
    //
    g_SPI_Instance.Init(s_SPI_CfgData);

    // same eeprom now using SPI instead
    g_Eeprom_Instance.Init(s_Eeprom_CfgData, &g_SPI_Instance);

    count = g_Eeprom_Instance.Read(0x40, d, 32);
}

As simple as that. Same code on nRF5x, LPCxx series,..

[bjorn-spockeli]

Hi Ray, I agree that it appears that the app_twi library is designed to communicate with a single TWI device, i.e statically-compiled for a single I2C address as you put it. I will convey your feedback to the app_twi developer. In order to achieve the functionality you want, then I think that you will have to go down one abstraction layer and use the twi driver directly, i.e. nrf_drv_twi.c/h, instead of the TWI Transaction Manager library(app_twi) which in turn uses the driver.