Skip to content

Swapping order of R and G LEDs, using board defined pins. #36

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

Closed
wants to merge 4 commits into from

Conversation

sandyjmacdonald
Copy link
Contributor

The final hardware swaps the order of the red and green LED elements, so this PR reflects that. I've also updated the rainbow example to use board.I2C() when setting up the I2C bus as there are board definitions for the Pimoroni RP2040 boards in CircuitPython now!

@sandyjmacdonald
Copy link
Contributor Author

How weird! It's not even the code I touched that it's failing on, as far as I can see.

@sandyjmacdonald
Copy link
Contributor Author

Any idea why it's failing on pylint, @makermelissa ?

@makermelissa
Copy link
Collaborator

Yeah, PyLint was fixed recently, so some checks that were passing before are not passing now. This isn't the only repo that has been affected and we're working on it.

@tannewt
Copy link
Member

tannewt commented Feb 25, 2021

I've proposed adafruit/cookiecutter-adafruit-circuitpython#111 to fix the duplicate code issue. We'll need to wait for that to settle before merging (otherwise releases will fail.)

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Code looks good!

@sandyjmacdonald
Copy link
Contributor Author

LOL. I got the x/y coordinates of the LEDs transposed again, because I'd flipped them in my function to map the 0-15 indices to x/y coordinates. Anyhoo... now switch 0 is 0,0, switch 1 is 1, 0, through to switch 15 being 3, 3.

@dglaude
Copy link
Contributor

dglaude commented Mar 1, 2021

Maybe #32 should be considered and this follow that scheme... else the library will inflate (like with that mapping table).

@kattni
Copy link
Contributor

kattni commented Mar 1, 2021

@sandyjmacdonald Thanks for the PR! We're working on a patch to all the libraries to fix the issue causing this PR to fail. However, it may be up to a week before the patch is fully applied.

How comfortable are you with Git and GitHub? I can walk you through making the changes that need to be made to get this passing. If you're not comfortable with that, I should be able to make the changes myself and apply them to this PR. Let me know if you're interested in giving it a try!

@sandyjmacdonald
Copy link
Contributor Author

@sandyjmacdonald Thanks for the PR! We're working on a patch to all the libraries to fix the issue causing this PR to fail. However, it may be up to a week before the patch is fully applied.

How comfortable are you with Git and GitHub? I can walk you through making the changes that need to be made to get this passing. If you're not comfortable with that, I should be able to make the changes myself and apply them to this PR. Let me know if you're interested in giving it a try!

I can definitely give it a try!

@kattni
Copy link
Contributor

kattni commented Mar 2, 2021

@sandyjmacdonald It looks like the patch may get applied sooner than I thought, so if you'd rather simply wait, we can do that as well. However! You are welcome to try the following. It will not affect the patch as these are the changes that would be applied by the patch, and therefore the patch will simply skip this library if it's already updated.

If you would like to, what you'll do is make changes in the same branch as used for this PR (which is master on your fork) and git push the same as you did with this change. It will automatically update this PR with your new changes. If you run into issues with this, please ping me and I will help you sort it out.

Please update the following two files.

First Adafruit_CircuitPython_IS31FL3731/.github/workflows/build.yml should look like the contents of this gist. (You are removing the Pylint section.)

Second Adafruit_CircuitPython_IS31FL3731/.pre-commit-config.yaml should look like the contents of this gist. (You are adding a Pylint section.)

@makermelissa
Copy link
Collaborator

Closing because I have included these changes as part of #38.

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.

5 participants