AVR Freaks

Yes,  that looks fine.   Live with it.

There is a big advantage in writing a single "universal" function.   In fact,  I use

int8_t twi_master_trans(uint8_t slave, uint8_t *txbuf, int txn, uint8_t *rxbuf, int rxn);

You might write a helper function that simplifies your common calls e.g.

int8_t i2c_reg_val(uint8_t slave, uint8_t reg, uint8_t val);

Just use the style that makes you happy.    Only worry if you start running out of Flash or stack.    Most straightforward programs never approach memory limits.

David.

that seems wasteful with a lot of "just shuffling bytes around". IS this the preferred way to do it?

Yes, I'm afraid so.

This is a common "difference in abstractions" between "higher level systems" and what we're used to in AVR-class systems.

Small microcontrollers tend to be programed with "stream-based" semantics, where the bottom-level primitive is "get/put single-byte", while bigger systems tend to be "file-based", where the bottom level is "read/write buffer", so you wind up with:

// microcontroller
void write(file, buffer, len) {
   for (len) {
     putc(file, *buffer++);
  }
}


// bigger system
void putc(file, c) {
   write(file, &c, 1);
}

On systems that are actually small and have byte-oriented peripherals, it's mostly a toss-up.  a low-level write() is likely to be more efficient for longer writes, and a low-level putc() is more efficient for very short writes.

Once you get to more complex peripherals (USB, Ethernet, DMA, actual disk files ... anything whose native implementation deals with more than one byte at a time), the low-level write() becomes MUCH better matched to reality, and the inefficiencies of implementation of "shifting bytes around" are dwarfed by the inefficiencies of very small transactions, anyway.)

(It's still annoying that "standard APIs" essentially fail to recognize the existence of single-byte peripherals :-( )

define a struct that holds all the fields you need and then make a const 'struct' and add that to the function call instead of all the separate fields.

I suppose this is a "number of repetitions" thing. If you find yourself writing "Your first inclination" many more times than you'd like to, then write a small Facade function which simplifies the Microchip API.

My own tolerance limit may be quite low here - perhaps 3 times.

A helper function is pretty simple.

static int8_t i2cRegData(uint8_t slave, uint8_t reg, uint8_t data)
{
    uint8_t txbuf[2];
    txbuf[0] = reg;
    txbuf[1] = data;
    return twi_master_trans(slave, txbuf, 2, NULL, 0);
}

and there might be multiple calls to this helper e.g.

    i2cRegData(MCP23017, 0x0A, 0xA0); //BANK=1, SEQOP=1 if BANK0 (POR)
    i2cRegData(MCP23017, 0x05, 0xA0); //BANK=1, SEQOP=1 i.e. BYTE_MODE
    i2cRegData(MCP23017, IODIRA, (LCD_CMD_MASK | LCD_RESET | LCD_BLK) ^ 0xFF); //Output
    i2cRegData(MCP23017, IODIRB, 0xFF ^ 0xFF);                              //Output
    i2cRegData(MCP23017, OLATA, LCD_CMD_PORT = LCD_RESET);
    i2cRegData(MCP23017, OLATA, LCD_CMD_PORT = 0);
    i2cRegData(MCP23017, OLATA, LCD_CMD_PORT = LCD_RESET);

whereas this program only calls the full-fat sequence in one place e.g.

                twi_master_trans(MCP23017, txbuf, cnt, NULL, 0); //N strobes

Yes,  the helper will involve a bit of extra stack use and Flash.   But the the "calls" are smaller.

In practice high level I2C calls are never going to be time-critical.   But the HAL "universal" function has probably been written for the hardware.

Use whatever you are happier with.    You will find it easier to develop.

And for successors to maintain.

David.