Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proved inverted inputs and pullup mode #15

Closed
wants to merge 15 commits into from

Conversation

Beerlesklopfer
Copy link

Hi Bertrand,

this is the output of the "InvertedInputs.ino" sketch. So it is proved to work correctly.

Best gregards Joerg

INTERRUPT PORT A FIRED
INTERRUPT PORT B FIRED
Checking setting port mode

------------- SUCCEEDED --------------

Checking setting pin mode
INTERRUPT PORT A FIRED
INTERRUPT PORT B FIRED

------------- SUCCEEDED --------------

Jörg Bernau and others added 14 commits February 25, 2020 01:17
That is quite more efficient!

Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
See above.

Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
You are right.

Co-Authored-By: Bertrand Lemasle <blemasle@gmail.com>
Copy link
Owner

@blemasle blemasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that there is a mistake in your code. Feel free to correct me If I'm wrong :)

src/MCP23017.cpp Show resolved Hide resolved

if(mode == OUTPUT)
{
iodir |= _BV(pin);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you invert the iodir assignation logic between OUTPUT and INPUT ? An OUTPUT pin must be cleared according to the datasheet, so iodir &= ~_BV(pin); was the correct way of handling this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it first as you suggested, but the the result was just inverted as expected. This way solved that problem. My wiring was as described in the new example.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm lost with that change. The code you're suggesting was initially the one I wrote, before it was corrected by #4.
I'll try to have a deeper look at this myself this weekend and sort it out once and for all. If that's true, then the datasheet is lying ^^

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I finally had the chance to get an mcp23017 out of the closet and test the library again.
I stick with my version of iodir, and this is very easy to prove :

I have a bunch of input switches on port B (active high), and some leds on port A.

#include <Wire.h>
#include <MCP23017.h>

#define MCP23017_ADDR 0x20
MCP23017 mcp = MCP23017(MCP23017_ADDR);

void setup() {
    Wire.begin();
    Serial.begin(115200);
    
    mcp.init();
    mcp.portMode(MCP23017_PORT::A, 0);         //Port A as output
    mcp.portMode(MCP23017_PORT::B, 0b11111111);//Port B as input

    mcp.writeRegister(MCP23017_REGISTER::GPIOA, 0x00); //Reset port A 
    mcp.writeRegister(MCP23017_REGISTER::GPIOB, 0x00); //Reset port B

    mcp.writeRegister(MCP23017_REGISTER::GPPUA, 0x00); //No pull ups on port A
    mcp.writeRegister(MCP23017_REGISTER::GPPUB, 0x00); //No pull ups on port B

    mcp.pinMode(7, INPUT);         //Pin 7 as input

    uint8_t conf = mcp.readRegister(MCP23017_REGISTER::IODIRA);
    Serial.print("IODIRA : ");
    Serial.print(conf, BIN);
    Serial.println();    
}


void loop() {
    uint8_t currentB;

    currentB = mcp.readPort(MCP23017_PORT::B);
    mcp.writePort(MCP23017_PORT::A, currentB);
}

IODIRA : 10000000

In the example above, the led on pin 7 is not configured as an input, so it does not lit, whereas every other led do lit. This is with the current version of the library. Integrating your changes on IODIR gives

IODIRA : 0

And all the leds are working (which proves that pin 7 is not configured as an input.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved by v2.0.0. Feel free to reopen the discussion if you still disagree on the resolution. But in this is case, please read the comments on your example first. Your sketch has mistakes that clearly indicates you misused the library :)

Thanks for the suggestion, glad to see it finally made it to the library ! 👍

}
else
{
iodir &= ~_BV(pin);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark as above :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me too 👍

bool error = false;

// PORT A is input inverted with pullups
mcp.portMode( MCP23017_PORT::A, INPUT, 0xFF, 0xFF);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second parameter of port mode use an uint8_t, each bit represents the direction of the corresponding pin. As the MCP23017 use 1 for input, and 0 for output, here you're actually setting the port A to be an output port entirely.

Suggested change
mcp.portMode( MCP23017_PORT::A, INPUT, 0xFF, 0xFF);
mcp.portMode( MCP23017_PORT::A, 0xFF, 0xFF, 0xFF);

Would have produced the result you were looking for.

mcp.clearInterrupts();

// PORT B is output
mcp.portMode(MCP23017_PORT::B, OUTPUT);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, OUTPUT on the arduino is 1, to here you're only setting the first pin of the B port as an input.

Suggested change
mcp.portMode(MCP23017_PORT::B, OUTPUT);
mcp.portMode(MCP23017_PORT::B, 0x00);

@blemasle blemasle closed this Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants