Reading data from MCP23017 using I2C master library

Go To Last Post
10 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have used Arduino I2C (Wire) library for a while now without issues, however, now I want to replace it with I2C master library. I'm using Arduino framework for quick testing, so this is the code I'm trying to use to read button states from two MCP23017 port expanders:

 

#include <i2c_master.h>

const uint8_t expanderAddress[] = { 0x42, 0x40 };   //chip address
const uint8_t gpioAddress[]     = { 0x12, 0x13 };   //input/output
const uint8_t iodirAddress[]    = { 0x00, 0x01 };   //port A/port B
const uint8_t gppuAddress[]     = { 0x0C, 0x0D };   //interrupt/pull-up

uint32_t mcpData;

void initMCP()  {

    i2c_init();

    i2c_start(expanderAddress[0] + I2C_WRITE);         //enable inputs, expander A, port A
    i2c_write(iodirAddress[0]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[0] + I2C_WRITE);         //enable inputs, expander A, port B
    i2c_write(iodirAddress[1]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[1] + I2C_WRITE);         //enable inputs, expander B, port A
    i2c_write(iodirAddress[0]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[1] + I2C_WRITE);         //enable inputs, expander B, port B
    i2c_write(iodirAddress[1]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[0] + I2C_WRITE);         //enable pull-ups, expander A, port A
    i2c_write(gppuAddress[0]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[0] + I2C_WRITE);         //enable pull-ups, expander A, port B
    i2c_write(gppuAddress[1]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[1] + I2C_WRITE);         //enable pull-ups, expander B, port A
    i2c_write(gppuAddress[0]);
    i2c_write(0xFF);
    i2c_stop();

    i2c_start(expanderAddress[1] + I2C_WRITE);         //enable pull-ups, expander B, port B
    i2c_write(gppuAddress[1]);
    i2c_write(0xFF);
    i2c_stop();

}

void setup()  {

  initMCP();
  Serial.begin(38400);

}

void loop() {

  if (readStates()) {

    for (int i=0; i<32; i++)  {

        Serial.print("Button ");
        Serial.print(i);
        Serial.print(" state: ");
        Serial.println(bitRead(mcpData, i));

    } Serial.println();

  } delay(1000);

}

bool readStates()   {

    mcpData = 0;

    for (int i=0; i<2; i++) {

        i2c_start(expanderAddress[i] + I2C_WRITE);
        i2c_write(gpioAddress[0]);
        i2c_stop();

        i2c_start(expanderAddress[i] + I2C_READ);
        mcpData |= i2c_read_ack();
        mcpData <<= 8;
        i2c_stop();

        i2c_start(expanderAddress[i] + I2C_WRITE);
        i2c_write(gpioAddress[1]);
        i2c_stop();

        i2c_start(expanderAddress[i] + I2C_READ);
        mcpData |= i2c_read_ack();
        mcpData <<= 8;
        i2c_stop();

    } return true;

}

The problem is - this doesn't seem to work - that is, I don't get correct button readouts, however, with Wire library from Arduino it works just fine (notice the changed address in code):

 

#include <Wire.h>

const uint8_t expanderAddress[] = { 0x21, 0x20 };   //chip address
const uint8_t gpioAddress[]     = { 0x12, 0x13 };   //input/output
const uint8_t iodirAddress[]    = { 0x00, 0x01 };   //port A/port B
const uint8_t gppuAddress[]     = { 0x0C, 0x0D };   //interrupt/pull-up

uint32_t mcpData;

void initMCP()  {

    Wire.begin(); // wake up I2C bus

    Wire.beginTransmission(expanderAddress[0]);         //enable inputs, expander A, port A
    Wire.write(iodirAddress[0]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[0]);         //enable inputs, expander A, port B
    Wire.write(iodirAddress[1]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[1]);         //enable inputs, expander B, port A
    Wire.write(iodirAddress[0]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[1]);         //enable inputs, expander B, port B
    Wire.write(iodirAddress[1]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[0]);         //enable pull-ups, expander A, port A
    Wire.write(gppuAddress[0]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[0]);         //enable pull-ups, expander A, port B
    Wire.write(gppuAddress[1]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[1]);         //enable pull-ups, expander B, port A
    Wire.write(gppuAddress[0]);
    Wire.write(0xFF);
    Wire.endTransmission();

    Wire.beginTransmission(expanderAddress[1]);         //enable pull-ups, expander B, port B
    Wire.write(gppuAddress[1]);
    Wire.write(0xFF);
    Wire.endTransmission();

}

void setup()  {

  initMCP();
  Serial.begin(38400);

}

void loop() {

  if (readStates()) {

    for (int i=0; i<32; i++)  {

        Serial.print("Button ");
        Serial.print(i);
        Serial.print(" state: ");
        Serial.println(bitRead(mcpData, i));

    } Serial.println();

  } delay(1000);

}

bool readStates()   {

    //get new values from all buttons

    mcpData = 0;

    for (int i=0; i<2; i++) {

        mcpData <<= 8; //make room for new data

        Wire.beginTransmission(expanderAddress[i]);
        Wire.write(gpioAddress[0]);
        Wire.endTransmission();

        Wire.requestFrom((int16_t)expanderAddress[i], 1);
        while (Wire.available() == 0);
        mcpData |= Wire.read();
        mcpData <<= 8;
        Wire.endTransmission();

        Wire.beginTransmission(expanderAddress[i]);
        Wire.write(gpioAddress[1]);
        Wire.endTransmission();

        Wire.requestFrom((int16_t)expanderAddress[i], 1);
        while (Wire.available() == 0);
        mcpData |= Wire.read();
        Wire.endTransmission();

    }

}

So, what am I doing wrong in first example?

 

* Moved to Arduino forum. *

 

Last Edited: Wed. Mar 9, 2016 - 11:00 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If the Wire library is working ok,   why change it?

 

I presume that you are using the Fleury I2C library.

Read its HTML documentation.    Most of the functions have return values.    Use them.

 

Read the documentation for i2c_readAck() and i2c_readNak().

You will see the problem with your readStates() function.

 

Oh,  the most important lesson is:   void functions are bad.

The next lesson is:  if you write a bool function,   return a truthful result.

 

As a strategy,   your approach is excellent.   i.e. get the project working in Arduino-ese first.   Then replace the Wire methods with Fleury functions.     Ask again if my "hint" is not good enough. 

 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for advices. Point in replacing Arduino library is that I want to get rid of Arduino dependencies in my project. I understand all your points about bool/void functions, however, this is really a simple tryout code so I didn't bother. Anyways, first thing that I noticed is that I should replace I2C_read_ack with  I2C_read_nack since nack won't request another byte. Still doesn't work as expected.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So what do you expect and what do get?

 

As I said,   you have an excellent strategy.   I would tidy up the Wire code first.    Your current code is very difficult for me to follow.

I would do something like this:

void write_I2C_reg(byte ads, byte reg, byte val)
{
    Wire.beginTransmission(ads);         //Wire uses 7-bit ads
    Wire.write(reg);
    Wire.write(val);
    Wire.endTransmission();
}

byte read_I2C_reg(byte ads, byte reg)
{
    byte val;
    Wire.beginTransmission(ads);         //
    Wire.write(reg);
    Wire.endTransmission();
    Wire.requestFrom(ads, 1);
    while (Wire.available() == 0);
    val = Wire.read();
    Wire.endTransmission();
    return val;
}

void initMCP()
{
    write_I2C_reg(0x21, 0x13, 0xFF);         //enable inputs, expander A, GPIOB
    write_I2C_reg(0x21, 0x12, 0xFF);         //enable inputs, expander A, GPIOA
    write_I2C_reg(0x20, 0x13, 0xFF);         //enable inputs, expander B, GPIOB
    write_I2C_reg(0x20, 0x13, 0xFF);         //enable inputs, expander B, GPIOA
    write_I2C_reg(0x21, 0x01, 0xFF);         //enable inputs, expander A, IODIRB
    write_I2C_reg(0x21, 0x00, 0xFF);         //enable inputs, expander A, IODIRA
    write_I2C_reg(0x20, 0x01, 0xFF);         //enable inputs, expander B, IODIRB
    write_I2C_reg(0x20, 0x00, 0xFF);         //enable inputs, expander B, IODIRA
    write_I2C_reg(0x21, 0x0D, 0xFF);         //enable inputs, expander A, GPPUB
    write_I2C_reg(0x21, 0x0C, 0xFF);         //enable inputs, expander A, GPPUA
    write_I2C_reg(0x20, 0x0D, 0xFF);         //enable inputs, expander B, GPPUB
    write_I2C_reg(0x20, 0x0C, 0xFF);         //enable inputs, expander B, GPPUA
}

bool readStates()
{
    mcpData <<= 8;
    mcpData |= read_I2C_reg(0x20, 0x00);     //expander A, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(0x20, 0x01);     //expander A, GPIOB
    mcpData <<= 8;
    mcpData |= read_I2C_reg(0x21, 0x00);     //expander B, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(0x21, 0x01);     //expander B, GPIOB
    return true;
}

void setup(void)
{
    Wire.begin(); // wake up I2C bus
    initMCP();
    ...
}

Then when I "port to Fleury",  I just need to replace the helper functions with the Fleury equivalents:

void write_I2C_reg(byte ads, byte reg, byte val)
{
    i2c_start((ads << 1) + I2C_WRITE);       //Fleury uses 8-bit ads
    i2c_write(reg);
    i2c_write(val);
    i2c_stop();
}

byte read_I2C_reg(byte ads, byte reg)
{
    byte val;
    i2c_start((ads << 1) + I2C_WRITE);       //Fleury uses 8-bit ads
    i2c_write(reg);
    i2c_start((ads << 1) + I2C_READ);       //Fleury uses 8-bit ads
    val = i2c_readNak();
    i2c_stop();
    return val;
}

Obviously Wire.begin() is i2c_init().   And in any real program you would ALWAYS test the return value from i2c_start().

 

Untested.

 

David.

 

Edit.   I note that you are not very intuitive over your "Expander A" and GPIO usage.    I have probably got them wrong.

Last Edited: Wed. Mar 9, 2016 - 12:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I expect change of state when I press buttons. The code you posted had some errors, so this is the correct code for Wire library:

 

#include <Wire.h>

const uint8_t expanderAddress[] = { 0x21, 0x20 };   //chip address
const uint8_t gpioAddress[]     = { 0x12, 0x13 };   //input/output
const uint8_t iodirAddress[]    = { 0x00, 0x01 };   //port A/port B
const uint8_t gppuAddress[]     = { 0x0C, 0x0D };   //interrupt/pull-up

uint32_t mcpData;

void write_I2C_reg(byte address, byte reg, byte value)
{
    Wire.beginTransmission(address);         //Wire uses 7-bit ads
    Wire.write(reg);
    Wire.write(value);
    Wire.endTransmission();
}

byte read_I2C_reg(byte address, byte reg)
{
    byte value;

    Wire.beginTransmission(address);
    Wire.write(reg);
    Wire.endTransmission();

    Wire.requestFrom(address, 1);
    while (Wire.available() == 0);
    value = Wire.read();
    Wire.endTransmission();
    return value;
}

void initMCP()
{
    write_I2C_reg(expanderAddress[0], gpioAddress[0], 0xFF);         //enable inputs, expander A, GPIOA
    write_I2C_reg(expanderAddress[0], gpioAddress[1], 0xFF);         //enable inputs, expander A, GPIOB

    write_I2C_reg(expanderAddress[1], gpioAddress[0], 0xFF);         //enable inputs, expander B, GPIOA
    write_I2C_reg(expanderAddress[1], gpioAddress[1], 0xFF);         //enable inputs, expander B, GPIOB

    write_I2C_reg(expanderAddress[0], iodirAddress[0], 0xFF);         //enable inputs, expander A, IODIRA
    write_I2C_reg(expanderAddress[0], iodirAddress[1], 0xFF);         //enable inputs, expander A, IODIRB

    write_I2C_reg(expanderAddress[1], iodirAddress[0], 0xFF);         //enable inputs, expander B, IODIRA
    write_I2C_reg(expanderAddress[1], iodirAddress[1], 0xFF);         //enable inputs, expander B, IODIRB

    write_I2C_reg(expanderAddress[0], gppuAddress[0], 0xFF);         //enable inputs, expander A, GPPUA
    write_I2C_reg(expanderAddress[0], gppuAddress[1], 0xFF);         //enable inputs, expander A, GPPUB

    write_I2C_reg(expanderAddress[1], gppuAddress[0], 0xFF);         //enable inputs, expander B, GPPUA
    write_I2C_reg(expanderAddress[1], gppuAddress[1], 0xFF);         //enable inputs, expander B, GPPUB
}

bool readStates()
{
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[0], gpioAddress[0]);     //expander A, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[0], gpioAddress[1]);     //expander A, GPIOB
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[1], gpioAddress[0]);     //expander B, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[1], gpioAddress[1]);     //expander B, GPIOB
    return true;
}

void setup()  {

  Wire.begin();
  initMCP();
  Serial.begin(38400);

}

void loop() {

  readStates();

    for (int i=0; i<32; i++)  {

        Serial.print(F("Button "));
        Serial.print(i);
        Serial.print(F(" state: "));
        Serial.println(bitRead(mcpData, i));

    } Serial.println(); mcpData = 0;

  delay(50);

}

Works as expected.

 

Now, this still doesn't work (I2C master):

 

#include <i2c_master.h>

const uint8_t expanderAddress[] = { 0x21, 0x20 };   //chip address
const uint8_t gpioAddress[]     = { 0x12, 0x13 };   //input/output
const uint8_t iodirAddress[]    = { 0x00, 0x01 };   //port A/port B
const uint8_t gppuAddress[]     = { 0x0C, 0x0D };   //interrupt/pull-up

uint32_t mcpData;

void write_I2C_reg(byte address, byte reg, byte value)
{
    i2c_start((address << 1) + I2C_WRITE);       //Fleury uses 8-bit ads
    i2c_write(reg);
    i2c_write(value);
    i2c_stop();
}

byte read_I2C_reg(byte address, byte reg)
{
    byte value;

    i2c_start((address << 1) + I2C_WRITE);       //Fleury uses 8-bit ads
    i2c_write(reg);
    i2c_start((address << 1) + I2C_READ);       //Fleury uses 8-bit ads
    value = i2c_read_nack();
    i2c_stop();

    return value;
}

void initMCP()
{
    write_I2C_reg(expanderAddress[0], gpioAddress[0], 0xFF);         //enable inputs, expander A, GPIOA
    write_I2C_reg(expanderAddress[0], gpioAddress[1], 0xFF);         //enable inputs, expander A, GPIOB

    write_I2C_reg(expanderAddress[1], gpioAddress[0], 0xFF);         //enable inputs, expander B, GPIOA
    write_I2C_reg(expanderAddress[1], gpioAddress[1], 0xFF);         //enable inputs, expander B, GPIOB

    write_I2C_reg(expanderAddress[0], iodirAddress[0], 0xFF);         //enable inputs, expander A, IODIRA
    write_I2C_reg(expanderAddress[0], iodirAddress[1], 0xFF);         //enable inputs, expander A, IODIRB

    write_I2C_reg(expanderAddress[1], iodirAddress[0], 0xFF);         //enable inputs, expander B, IODIRA
    write_I2C_reg(expanderAddress[1], iodirAddress[1], 0xFF);         //enable inputs, expander B, IODIRB

    write_I2C_reg(expanderAddress[0], gppuAddress[0], 0xFF);         //enable inputs, expander A, GPPUA
    write_I2C_reg(expanderAddress[0], gppuAddress[1], 0xFF);         //enable inputs, expander A, GPPUB

    write_I2C_reg(expanderAddress[1], gppuAddress[0], 0xFF);         //enable inputs, expander B, GPPUA
    write_I2C_reg(expanderAddress[1], gppuAddress[1], 0xFF);         //enable inputs, expander B, GPPUB
}

bool readStates()
{
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[0], gpioAddress[0]);     //expander A, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[0], gpioAddress[1]);     //expander A, GPIOB
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[1], gpioAddress[0]);     //expander B, GPIOA
    mcpData <<= 8;
    mcpData |= read_I2C_reg(expanderAddress[1], gpioAddress[1]);     //expander B, GPIOB

    return true;
}

void setup()  {

  i2c_init();
  initMCP();
  Serial.begin(38400);

}

void loop() {

  readStates();

    for (int i=0; i<32; i++)  {

        Serial.print(F("Button "));
        Serial.print(i);
        Serial.print(F(" state: "));
        Serial.println(bitRead(mcpData, i));

    } Serial.println(); mcpData = 0;

  delay(50);

}

There's no change of state on button presses.

EDIT: Actually some buttons work, and some don't using this. When using Wire each button works.

EDIT2: More specifically, buttons connected to PORTA of each expander work, rest don't.

Last Edited: Wed. Mar 9, 2016 - 12:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well,  you have taken something that looked pretty readable.   Then obfuscated it.

Seriously,   you might just as well use the numbers / values from the Data Sheet.   (if you want someone else to read your code)

 

I did say "Untested".   For a start,  I see that my readStates() was using IODIRA instead of GPIOA.

As I asked earlier,   what do you expect and what did you get?

 

Most I2C devices are happy with a Repeated Start.    If the MCP23017 is not,  just add an i2c_stop().

 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:

Well,  you have taken something that looked pretty readable.   Then obfuscated it.

 

I don't see how replacing hardcoded values with values inside array for easier readability is "obfuscating".

 

david.prentice wrote:
As I asked earlier,   what do you expect and what did you get?

 

Since I enabled all pull-up resistors, and in my code I'm iterating over 32 buttons (two expanders), all values should read "1" until button is pressed. Then it should change to 0. This is exactly what is happening with Wire code. 

 

And now suddenly none of the I2C master codes posted here won't work at all - I get nothing in serial monitor (wire works). So frustrating.

 

Last Edited: Wed. Mar 9, 2016 - 02:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry,  I had edited for my own readability.    And to compare with the data sheet.

I see little point in having the lookup tables.   You are only doing 32-bits.

 

Note that you are relying on the power-up state.   i.e. IOCON = 0x00 so BANK = 0

I would be happier with a more foolproof initialisation because if you get the BANK = 1,   all the registers change addresses.

 

Have you got two 4k7 pull-up resistors on the SDA, SCL lines?

Arduino users are often too mean to spend $0.02.

 

I2C must have external pull-up resistors.    Modules designed for Arduino users sometimes have them and sometimes not.   

 

A proper I2C bus would just have a single pair of pull-ups.    Not pull-ups on every module.    However,   you will not go too far wrong if you only have 3 or 4 I2C devices.   (four 4k7 in parallel = 1.2k which is fine)

 

David.

Last Edited: Wed. Mar 9, 2016 - 02:58 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I do use 4k7 pull-ups on SDA/SCL.

 

EDIT: Connection error... anyways back to couple of posts back. PORTA works, PORTB doesn't. Weird.

EDIT2: Ahh, tried to add i2c_stop() to read_reg function, now it works. Thanks a lot for your time.

 

byte read_I2C_reg(byte address, byte reg)
{
    byte value;

    i2c_start((address << 1) + I2C_WRITE);       //Fleury uses 8-bit ads
    i2c_write(reg);
    i2c_stop();
    i2c_start((address << 1) + I2C_READ);       //Fleury uses 8-bit ads
    value = i2c_read_nack();
    i2c_stop();

    return value;
}

 

Last Edited: Wed. Mar 9, 2016 - 03:14 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This sounds like a BANK problem.    I would at least use:

void initMCP()
{
    // ensure that we know the configuration
    write_I2C_reg(0x21, 0x0A, 0x00);         //IOCON=0x00 if BANK=0
    write_I2C_reg(0x21, 0x05, 0x00);         //IOCON=0x00 if BANK=1
    write_I2C_reg(0x20, 0x0A, 0x00);         //IOCON=0x00 if BANK=0
    write_I2C_reg(0x20, 0x05, 0x00);         //IOCON=0x00 if BANK=1
    write_I2C_reg(0x21, 0x01, 0xFF);         //enable inputs, expander A, IODIRB
    write_I2C_reg(0x21, 0x00, 0xFF);         //enable inputs, expander A, IODIRA
    write_I2C_reg(0x20, 0x01, 0xFF);         //enable inputs, expander B, IODIRB
    write_I2C_reg(0x20, 0x00, 0xFF);         //enable inputs, expander B, IODIRA
    write_I2C_reg(0x21, 0x0D, 0xFF);         //pullups, expander A, GPPUB
    write_I2C_reg(0x21, 0x0C, 0xFF);         //pullups, expander A, GPPUA
    write_I2C_reg(0x20, 0x0D, 0xFF);         //pullups, expander B, GPPUB
    write_I2C_reg(0x20, 0x0C, 0xFF);         //pullups, expander B, GPPUA
}

If you are using interrupts,   inversions, ... I would be a lot happier if you initialised all the registers properly.

Mind you,   the purpose of the exercise is to replicate the "good" behaviour of the Wire library.    Since the Wire code does not ensure the MCP23017 registers,   I would not expect that Fleury would be different.

 

Incidentally,   the MCP23017 has got lots of nice features like SEQOP, MIRROR, ...

You could use a generic block-read and block-write function.   Or poll the same register,  ...

 

Untested.   I have never used this chip.    It is a lot cleverer than the PCF8574 expander.

 

David.