Skip to content

Binho Nova integration #43

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 15 commits into from
Dec 3, 2019
Merged

Binho Nova integration #43

merged 15 commits into from
Dec 3, 2019

Conversation

fig1010
Copy link
Contributor

@fig1010 fig1010 commented Dec 2, 2019

Adding BLINKA_NOVA support

jonathan-binho and others added 13 commits November 16, 2019 14:03
@jonathan-binho @jmg5150
Added support for Binho Nova Multi-Protocol USB Host Adapter
9690546
@fig1010
Have Binho use its own try block
e6925ab
@fig1010
Use BLINKA_NOVA as env variable instead
2f3ffab
@fig1010
change board id to NOVA and fix issue with autodetect being called mu…
903fc78 
…ltiple times after there's already a binho object
@fig1010
set _binho only if BLINKA_NOVA env variable exists
95e159f
@fig1010
Use binho connection singleton
d09621e
@jonathan-binho @fig1010
Added support for Binho Nova Multi-Protocol USB Host Adapter
0d67ae6
@fig1010
Use BLINKA_NOVA as env variable instead
86c9931
@fig1010
change board id to NOVA and fix issue with autodetect being called mu…
bf78cc6 
…ltiple times after there's already a binho object
@fig1010
set _binho only if BLINKA_NOVA env variable exists
9c68446
@fig1010
Use binho connection singleton
5084033
@fig1010
Merge branch 'master' of https://github.com/binhollc/Adafruit_Python_…
6cde03a 
…PlatformDetect
@fig1010
Clean up and remove unused code
0a1c8ff
@ladyada ladyada requested a review from makermelissa December 2, 2019 23:55
@ladyada
Copy link
Member

ladyada commented Dec 3, 2019

please fix
https://travis-ci.com/adafruit/Adafruit_Python_PlatformDetect/builds/139063671#L538

@fig1010
Copy link
Contributor Author

fig1010 commented Dec 3, 2019

please fix
https://travis-ci.com/adafruit/Adafruit_Python_PlatformDetect/builds/139063671#L538

Thanks, will monitor travis output from here on out and fix accordingly.

@ladyada
Copy link
Member

ladyada commented Dec 3, 2019

@makermelissa LGTM take a look too in case i missed something :)

makermelissa
makermelissa reviewed Dec 3, 2019
View reviewed changes
adafruit_platformdetect/board.py Outdated
@@ -68,6 +68,8 @@

MICROCHIP_MCP2221 = "MICROCHIP_MCP2221"

BINHO_NOVA = "NOVA"
Copy link
Collaborator

Choose a reason for hiding this comment

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

My only concern on this PR is that there's another board named Nova that will be added in the future. Can we change the string from "NOVA" to "BINHO_NOVA"? Otherwise, everything else looks good to me too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure. I can make this change to use "BINHO_NOVA" instead. Thanks for the heads up!

makermelissa
makermelissa approved these changes Dec 3, 2019
View reviewed changes
Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Looks good.

@makermelissa makermelissa merged commit 93c4d5c into adafruit:master Dec 3, 2019
@makermelissa
Copy link
Collaborator

makermelissa commented Dec 3, 2019 •
edited
Loading

Hi @fig1010, it looks like we have a slight problem here. Travis is failing because you have a circular dependency and as a result, PlatformDetect is not getting pushed to PyPI where it would be available to allow your other PR to pass its test. When I merged, I didn't realize it would push it only if it passed.

Could you submit another PR where PlatformDetect does not depend on Blinka?
Thanks.

@fig1010
Copy link
Contributor Author

fig1010 commented Dec 3, 2019

Hi @fig1010, it looks like we have a slight problem here. Travis is failing because you have a circular dependency and as a result, PlatformDetect is not getting pushed to PyPI where it would be available to allow your other PR to pass its test. When I merged, I didn't realize it would push it only if it passed.

Could you submit another PR where PlatformDetect does not depend on Blinka?
Thanks.

We're looking into this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ladyada ladyada ladyada approved these changes

@makermelissa makermelissa makermelissa 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.
4 participants
@fig1010 @ladyada @makermelissa @jonathan-binho