Skip to content

Abstracted Status NeoPixel to make DotStar usable #14

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 3 commits into from
Feb 26, 2019
Merged

Abstracted Status NeoPixel to make DotStar usable #14

merged 3 commits into from
Feb 26, 2019

Conversation

makermelissa
Copy link
Collaborator

@makermelissa makermelissa commented Feb 26, 2019 •
edited
Loading

I abstracted Status NeoPixel to make DotStar usable. I also updated all of the examples. As this change affects the Adafruit IO library, please do the PR shortly after this PR. I will link after it's created. This is in regards to issue #13.

@makermelissa makermelissa requested a review from a team February 26, 2019 18:30
@makermelissa makermelissa mentioned this pull request Feb 26, 2019
Merged
@makermelissa
Copy link
Collaborator Author

The Adafruit IO PR is adafruit/Adafruit_CircuitPython_AdafruitIO#14 and both should be merged at the same time.

jerryneedell
jerryneedell reviewed Feb 26, 2019
View reviewed changes
adafruit_esp32spi/adafruit_esp32spi_wifimanager.py Outdated
"""
:param ESP_SPIcontrol esp: The ESP object we are using
:param dict secrets: The WiFi and Adafruit IO secrets dict (See examples)
:param int attempts: (Optional) Failed attempts before resetting the ESP32 (default=2)
:param status_neopixel: (Optional) The neopixel pin - Usually board.NEOPIXEL (default=None)
:type status_neopixel: Pin
:param status_pixel: (Optional) The pixel pin - Usually board.NEOPIXEL (default=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is Pin optional -- why would you pass in a pin name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I need to fix that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll have to wait until I get to my charger though as my laptop with my code died.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I was able to use the online github editor to fix. The reason it is optional is because the library will function even if you don't pass it in. You just won't be able to visually tell the status.

Copy link
Contributor

Choose a reason for hiding this comment

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

np with optional -- I was just confused but the Pin reference -- thanks for fixing. I like this because the user can use any neopixel/dotstar (single or string) available

@makermelissa
Copy link
Collaborator Author

Please note that this PR is very lightly tested as I didn't have all my hardware with me.

@makermelissa makermelissa self-assigned this Feb 26, 2019
@jerryneedell
Copy link
Contributor

I'll do some testing a bit later today.

@brentru
Copy link
Member

brentru commented Feb 26, 2019

I've tested this with Adafruit IO CircuitPython and it works without issue on updated examples.

@jerryneedell
Copy link
Contributor

has anyone tested it with a dotstar?

@makermelissa
Copy link
Collaborator Author

I tested on an ItsyBitsy M4 without an ESP32 attached.

jerryneedell
jerryneedell approved these changes Feb 26, 2019
View reviewed changes
Copy link
Contributor

@jerryneedell jerryneedell left a comment

Choose a reason for hiding this comment

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

tested dotstar function on itsy_bitsy_m4 with local time test -- worked fine! Looks good to me! Thanks

@makermelissa makermelissa merged commit 109eba6 into adafruit:master Feb 26, 2019
@brentru
Copy link
Member

brentru commented Feb 26, 2019

Merged in the Adafruit IO CircuitPython PR which referenced this PR, looks good!

@makermelissa makermelissa mentioned this pull request Feb 26, 2019
Closed
@brentru brentru mentioned this pull request Feb 26, 2019
Merged
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 27, 2019
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@eab60b0)
c0d8067 
Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 1.3.1 from 1.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#17 from brentru/update-comments-for-pixel
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#18 from brentru/expose-rssi
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#16 from brentru/expose-ip-addr
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#14 from makermelissa/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerryneedell jerryneedell jerryneedell approved these changes

Assignees

@makermelissa makermelissa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@makermelissa @jerryneedell @brentru