-
Notifications
You must be signed in to change notification settings - Fork 16
Adds initial working code. #2
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
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.
Other than these two "administrivia" things, looks excellent to me. We need to update the cookiecutter with these two changes; I plan on doing that tonight.
.pylintrc
Outdated
# (useful for modules/projects where namespaces are manipulated during runtime | ||
# and thus existing member attributes cannot be deduced by static analysis. It | ||
# supports qualified module names, as well as Unix pattern matching. | ||
ignored-modules= |
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.
While a patch is in work to do this, can you add board
to the ignored-modules? This will cover future PyPi work.
.travis.yml
Outdated
|
||
install: | ||
- pip install pylint circuitpython-build-tools Sphinx sphinx-rtd-theme | ||
|
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.
Could you change this to:
- pip install circuitpython-build-tools Sphinx sphinx-rtd-theme
- pip install --force-reinstall pylint==1.9.2
While pylint is currently happy, who knows what will get added in future versions. 😄
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.
Thank you for adding this! I think it'll be helpful. Just a few comments now.
adafruit_tca9548a.py
Outdated
self.i2c = i2c | ||
self.address = address | ||
self.used_channels = [] | ||
|
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:
def __len__(self):
return 8
That way it can be iterated over.
adafruit_tca9548a.py
Outdated
|
||
def __getitem__(self, key): | ||
if not 0 <= key <= 7: | ||
raise ValueError("Channel must be an integer in the range: 0-7") |
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.
IndexError instead since its a sequence.
adafruit_tca9548a.py
Outdated
if not 0 <= key <= 7: | ||
raise ValueError("Channel must be an integer in the range: 0-7") | ||
if key in self.used_channels: | ||
raise ValueError("Channel already in use.") |
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 wouldn't track this because you could have multiple devices on a single channel. The locking should prevent bus usage conflicts.
adafruit_tca9548a.py
Outdated
if key in self.used_channels: | ||
raise ValueError("Channel already in use.") | ||
self.used_channels.append(key) | ||
return TCA9548A_Channel(self, key) |
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 would cache the channel so the same object is returned for the same accesses.
requirements.txt
Outdated
@@ -0,0 +1 @@ | |||
adafruit-circuitpython-busdevice |
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 isn't a dependency because its not imported by adafruit_tca9548c.py.
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.
My 2 requests are good now. Thanks @caternuson!
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.
Thanks! Sorry for the slow review!
See
examples/tca9548a_simpletest.py
for example usage.