Skip to content

Fixed 0/180 rotation coordinates #7

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 6, 2018
Merged

Fixed 0/180 rotation coordinates #7

merged 2 commits into from
Nov 6, 2018

Conversation

kattni
Copy link
Contributor

@kattni kattni commented Nov 5, 2018

No description provided.

@kattni kattni requested a review from dhalbert November 5, 2018 18:30
@kattni
Copy link
Contributor Author

kattni commented Nov 5, 2018 •
edited
Loading

Adds code to limit what NeoPixel coordinates are valid because some invalid offsets were coming up negative which was allowing it to light up an incorrect NeoPixel by counting backwards on the NeoPixel index.

Tested every NeoPixel coordinate on all four rotations, and tested every button press on all four rotations.

dhalbert
dhalbert requested changes Nov 5, 2018
View reviewed changes
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough testing!

adafruit_trellism4.py Outdated
@@ -68,6 +68,8 @@ def __init__(self, pin, *, width, height, rotation=0):
def __setitem__(self, index, value):
if not isinstance(index, tuple) or len(index) != 2:
raise IndexError("Index must be tuple")
if index[0] > 7 or index[1] > 7:
Copy link
Contributor

@dhalbert dhalbert Nov 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think could be a tighter check:

if index[0] >= self.width or index[1] >= self.height:
    raise IndexError(...)

index[0] is always the width, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

also check for the indexes < 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and it looks like this works. Adding now!

dhalbert
dhalbert approved these changes Nov 6, 2018
View reviewed changes
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Ok, great, and safer test for out of range at the end!

@dhalbert dhalbert merged commit f30754d into adafruit:master Nov 6, 2018
@kattni kattni mentioned this pull request Nov 6, 2018
Open
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 7, 2018
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@6086fba)
c34b8ae 
Updating https://github.com/adafruit/Adafruit_CircuitPython_BMP3XX to 1.1.0 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_BMP3XX#2 from kattni/pypi-setup

Updating https://github.com/adafruit/Adafruit_CircuitPython_HT16K33 to 2.1.1 from 2.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#26 from adafruit/setup_packages
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#25 from caternuson/iss22
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#24 from caternuson/iss21
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#23 from caternuson/iss20
  > ignore the board module imports in .pylintrc

Updating https://github.com/adafruit/Adafruit_CircuitPython_TrellisM4 to 1.1.2 from f30754d:
  < Merge pull request adafruit/Adafruit_CircuitPython_TrellisM4#7 from kattni/coordinate-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_BluefruitSPI, Adafruit_CircuitPython_BMP3XX, Adafruit_CircuitPython_FRAM, Adafruit_CircuitPython_MLX90393, Adafruit_CircuitPython_TFmini, Adafruit_CircuitPython_TrellisM4, Adafruit_CircuitPython_VS1053
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 9, 2018
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@6086fba)
ceefb3f 
Updating https://github.com/adafruit/Adafruit_CircuitPython_FRAM to 1.1.0 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_FRAM#3 from kattni/pypi

Updating https://github.com/adafruit/Adafruit_CircuitPython_TFmini to 1.1.0 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_TFmini#2 from kattni/pypi

Updating https://github.com/adafruit/Adafruit_CircuitPython_TrellisM4 to 1.2.0 from 1.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_TrellisM4#11 from kattni/pypi
  > Merge pull request adafruit/Adafruit_CircuitPython_TrellisM4#7 from kattni/coordinate-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_VS1053 to 1.1.0 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_VS1053#3 from kattni/pypi

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_FRAM, Adafruit_CircuitPython_TFmini, Adafruit_CircuitPython_TrellisM4, Adafruit_CircuitPython_VS1053
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhalbert dhalbert dhalbert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kattni @dhalbert