C++ pointer perils? How do I successfully create a more universal i2c / TWI library for Arduino?

Question

I'm working on a project where I use multiple i2c IC's / sensors. Adafruit has very nice basic libraries for the individual IC's and that is very convenient (spares hours of digging through datasheets). But their libraries are inflexible. I tried to alleviate this by applying dependency injection.

So the solution would be to use the TwoWire instance and pass it as a reference to the (in my case) MCP23008 library (dependency injection) and let the library make use of the same instance of TwoWire to do all i2c calls.

To make the topic more readable - but still provide insight in what I already tested and why - I will first mention the problem and then give more background info at the bottom.

// part from the modified library for the MCP23008 
// i2c port expander by Limor Fried

MyLib_MCP23008.cpp

// I added a constructor
MyLib_MCP23008::MyLib_MCP23008(TwoWire *w)
{
    // Constructors
    this->_wire = w;
}

// convenience begin with default address
void MyLib_MCP23008::begin() {
    begin(MCP23008_ADDRESS); // defined in .h as 0x20
}

void MyLib_MCP23008::begin(uint8_t addr) {
    if (addr > 7) {
        addr = 7;
    }
    this->i2caddr = addr;
    // set defaults!
    // _wire->begin(); don't do this here, expect the dependent TwoWire to do this...
    // there is a condition for ARDUINO >= 100 else in the original...
    _wire->beginTransmission(MCP23008_ADDRESS | i2caddr);
    _wire->write((byte)MCP23008_IODIR);
    _wire->write((byte)0xFF);  // all inputs
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->write((byte)0x00);
    _wire->endTransmission();
}

I also changed all "Wire." references to "_wire->" in the other methods.

MyLib_MCP23008.h:

// I left out the defines for better reading

class MyLib_MCP23008 {
    public:
        MyLib_MCP23008(TwoWire *w = &Wire);
        void begin(uint8_t addr);
        void begin(void);

        void pinMode(uint8_t p, uint8_t d);
        void digitalWrite(uint8_t p, uint8_t d);
        void pullUp(uint8_t p, uint8_t d);
        uint8_t digitalRead(uint8_t p);
        uint8_t readGPIO(void);
        void writeGPIO(uint8_t);

    private:
        uint8_t i2caddr;
        uint8_t read8(uint8_t addr);
        void write8(uint8_t addr, uint8_t data);
        TwoWire *_wire;
};

In my test sketch (mcp23008_test.ino):

TwoWire myWire(&sercom2, 4, 3);
MyLib_MCP23008 mcp23008(&myWire);
const int mcp_address = 0x20;

void setup()
{
    myWire.begin();
    pinPeripheral(4, PIO_SERCOM_ALT);
    pinPeripheral(3, PIO_SERCOM_ALT);

    // if I can find mcp23008: 
    myWire.beginTransmission(mcp_address);
    if (myWire.endTransmission() == 0) {
        mcp23008.begin();
    }

    // irrelevant setup stuff for pinMode / Serial etc.

    mcp23008.pinMode(mcpLED, OUTPUT);
}

void loop() {

    mcpLEDState = !mcpLEDState;
    mcp23008.digitalWrite(mcpLED, !mcpLEDState); // i2c LED alas nothing...
    digitalWrite(hbLED, mcpLEDState); // heartbeat: works fine

    // after this I scan the i2c wire 
    // using begin/endTransmission calls
    // and output it to Serial
    // result: works fine...

}

The program does not crash (I use a heartbeat LED and Serial output keeps working, the i2c bus also stays responsive (begin/end) but the LED of connected to the mcp23008 does not react in the TwoWire dependency injection version of the library).

One thing I will try is adding a logic analyser to the i2c bus and see what message come by... But as the 'hacked' library solution does work without any problem I guess there is something else going on.

Note I have a work-around and that is including all the library stuff directly into the main sketch as sub ino-files, that works but is not very SOLID of course.

Background information:

To make things manageable I first focussed on the MCP23008 IC: I have connected an LED to one of the pins, writing a LOW to it will make the LED turn on (as I sink it to 3.3V) and a HIGH will turn it off. So far so good, if I hack the library to support my specific i2c port and this works without any problems:

TwoWire myWire(&sercom2, 4, 3);

Then in begin I do:

myWire.begin();
pinPeripheral(4, PIO_SERCOM_ALT);
pinPeripheral(3, PIO_SERCOM_ALT);

inside the begin method of the library.

I then replace all the Wire references in the library with myWire.

Again till this point so far so good, I can let the LED blink from my main sketch etc.

So this works for my basic case but is a very ugly solution: you might have again some different port you want to use for your MCP23008. And in my case I want to use more than one i2c device and as they all do myWire.begin(); in their begin method the i2c bus freezes etc.

And as I said earlier: one does not really want the library itself to do the myWire.begin stuff (and if needed additional pinPeripheral calls etc): one wants the main sketch to do so.

Nathan Seidle from SparkFun Electronics wrote some nice article about the inflexibility of Arduino libraries issue: https://www.sparkfun.com/news/2194

But his solution and the one he based his solution on are not working for me: I can see the MCP23008 on the bus but it does not react to my command when using the modified library.

I also looked at the https://github.com/arduino/ArduinoCore-samd/blob/master/libraries/Wire/Wire.cpp/.h as it pretty much does the same that I try to accomplish but than at the Wire level. I don't see any deviations in my code from that example but looking at one's own code is always a weakness of course.

Update

This minimalistic implementation does work: I used a standard Wire | master writer as basic sketch on one board and the standard Wire | slave receiver at the other board.

I changed the master:

#include <Wire.h>
#include "GenericWireTest.h"

TwoWire myWire(&sercom3, 0, 1);
GenericWireTest genericWire(&myWire);

void setup()
{
  Wire.begin(); // join i2c bus (address optional for master)
  genericWire.begin();
  Serial.begin(115200);
}

byte x = 0;

void loop()
{
  genericWire.sendMessage(x);
  Serial.print("Wrote: x = ");
  Serial.println(x);
  x++;
  delay(500);
}

CPP file:

#include "Wire.h"
#include "GenericWireTest.h"

GenericWireTest::GenericWireTest(TwoWire *w)
{
  this->_wire = w;
}

void GenericWireTest::sendMessage(int x) {
  _wire->beginTransmission(4); // transmit to device #4
  _wire->write("x is ");        // sends five bytes
  _wire->write(x);              // sends one byte
  _wire->endTransmission();    // stop transmitting
}

void GenericWireTest::begin(uint8_t addr) {
  // nothing to do in this case...
  begin();
}

void GenericWireTest::begin(void) {
  _wire->begin();
}

H-file:

#include "Wire.h"

class GenericWireTest {
    public:
        GenericWireTest(TwoWire *w = &Wire);
        void begin(uint8_t addr);
        void begin(void);

        void sendMessage(int x);

    private:
        uint8_t i2caddr;
        TwoWire *_wire;
};

This works as expected... Note: I added _wire->begin(); in order to maybe confuse the Wire channel but it does not get confused even then.

Update 2:

I was actually able to confirm that the injection works with other i2c peripherals including the FXOS8700 accelerometer/magnetometer. So it is maybe just related to the MCP or I made a little mistake somewhere. Maybe time to dig deep into the datasheet of the MCP.

Update 3: Also works fine for FXAS21002C so I guess it is some MCP related issue. I created a work-around for now by adding the MCP code to an ino file in my main project, then I could use dependency injection without any problems, not as neat as I would like, but for now workable...