-
Notifications
You must be signed in to change notification settings - Fork 71
Update sound meter code to work with CPB and CPX #93
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
Conversation
) | ||
# Only Circuit Playground Express has touch_A7 available. So, we'll use it to determine whether or | ||
# not the board being used is a CPX. If it is a CPX, run the first code block. | ||
if hasattr(cp, "touch_A7"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a test to the cp
object so you can ask which board you got :)
@@ -78,6 +79,9 @@ class CircuitPlaygroundBase: # pylint: disable=too-many-public-methods | |||
|
|||
_audio_out = None | |||
|
|||
# Circuit Playground specific board names as found in os.uname().machine | |||
_CP_TYPES = ("Bluefruit", "Express") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhalbert would you think doing this consts make more sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested consts as a possibility but @kattni preferred strings. We need the strings anyway to search for them in the os.uname()
field to distinguish the boards.
We could use strings that have class attribute names too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't you just implementing sound_level
on the Express class? That would remove the need for conditional examples at all.
All other comments are based on doing conditional examples but it gets hairy pretty quickly.
This can be simplified by using Python's built in type logic. It has isinstance()
which can be used to see if the CP object is a Bluefruit or Express object. It doesn't require any changes to the base.
init.py will need to be:
if sys.platform == "nRF52840":
import bluefruit
cp = bluefruit.Bluefruit()
elif sys.platform == "Atmel SAMD21":
import express
cp = express.Express()
The constructors will need to be moved... which means a bunch of guides will need to be updated.
https://gist.github.com/tannewt/69bee4b4ebe8e13bba9b256943474bb0
board.MICROPHONE_CLOCK, board.MICROPHONE_DATA, sample_rate=16000, bit_depth=16 | ||
) | ||
# Check to see if the board type is a Circuit Playground Express, and, if so, run the following: | ||
if cp.circuit_playground_is_type("Express"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if cp.circuit_playground_is_type("Express"): | |
if isinstance(cp, Express): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will also need another import from adafruit_circuitplayground.express import Express
and the `cp instantiate will need to be in init (otherwise this import will construct the cpx object.)
@tannewt We designed this to be backwards compatible because there is SO much usage of this code using the previous
|
Originally I was but I think implementing
When you say "don't fit" do you mean in memory or in flash when frozen? Is there an existing implementation that doesn't work? I can help get it to fit. I'm surprised that the type functions in this PR are any less space than implementing I don't want to see a non-Python way of checking the type of something. There are already ways to do it. |
Fixes #76