Skip to content

Conversation

@Nexnar
Copy link

@Nexnar Nexnar commented Oct 18, 2024

Changed the mc23017.cpp and main.cpp according to the driver tutorial for Bowen Quan

Copy link
Contributor

@bquan0 bquan0 left a comment

Choose a reason for hiding this comment

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

Overall, a good start! Just a few things you need to change with your I2C reads and writes, and which addresses you are using.

Copy link
Contributor

@bquan0 bquan0 left a comment

Choose a reason for hiding this comment

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

Some comments for main.cpp.

Copy link
Contributor

@bquan0 bquan0 left a comment

Choose a reason for hiding this comment

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

The functionality looks good to me, just a few things in main.cpp that need to be fixed. Good job with fixing the I2C reads!
Also, for future reference, please resolve conversations after fixing them instead of reacting with 👍 . It makes reviewing the PR easier for me.

Copy link
Contributor

@bquan0 bquan0 left a comment

Choose a reason for hiding this comment

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

Once again, please resolve the previous comments since you've dealt with them.

src/main.cpp Outdated
void setup() {
// TODO: initialize i2c and mcp23017 object
Wire.begin();
uint8_t directions[8] = {1, 1, 1, 1, 1, 1, 1, 0};
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch on the array not being 8 numbers long, but you still haven't addressed the problem I mentioned last time. According to the data sheet, 1 = input and 0 = output. If you wanted to turn on an LED with pin 0, would that require an input to the IO expander or an output from the IO expander?

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.

3 participants