-
Notifications
You must be signed in to change notification settings - Fork 25
Fixes Issue#5 and use adafruit_register #9
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
- Add properties (get & set) for config register and config fields break-up
- Change internals to use adafruit_register for register and bit-field read/write - Updated table of constants, fix BusADCResolution values, add new values - Add new properties to access registers (advanced API) - Update ina219_simpletest to show some usage of the new properties
- Change internals to use adafruit_register for register and bit-field read/write - Updated table of constants, fix BusADCResolution values, add new values - Add new properties to access registers (advanced API) - Update ina219_simpletest to show some usage of the new properties
hey @barbudor it looks like pylint found a few things to complain about; take a look and let me know if you have questions on how to resolve them: https://travis-ci.com/adafruit/Adafruit_CircuitPython_INA219/builds/104711311 |
@siddacious Many thanks for your help to make my 1st PR |
Last comment : tested on CPX with CP 4.0.0.beta5 |
adafruit_ina219.py
Outdated
|
||
class Reset: | ||
"""Constants for ``reset``""" | ||
RESET = const(0x01) # write RESET to reset |
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.
as its one bit, this could just be True or False instead of a custom class?
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 think it doesn't make any sense to use const()
inside classes — it only works with global variables.
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.
Hi @ladyada, indeed this one is useless and should be removed. I will do it as soon as no more review is expected.
Hi @deshipu, I'm dupplicating what I've seen in IN260 : https://github.com/adafruit/Adafruit_CircuitPython_INA260/blob/master/adafruit_ina260.py
Using classes for constant allows to use meaning full names such as Mode.CONTINUOUS
I believe this is the same as DigitalInOut.Direction.OUTPUT
My understanding is that a const()
will use less memory than a variable.
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.
Does it makes sense to keep this one too ?
class BusVoltageRange:
"""Constants for bus_voltage_range"""
RANGE_16V = const(0x00) # set bus voltage range to 16V
RANGE_32V = const(0x01) # set bus voltage range to 32V (default)
May be renaming bus_voltage_range
as bus_voltage_range_32V
and use False
/True
?
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.
const()
works by replacing every occurrence of a variable with the number itself before compiling the code. It only works with globals, because otherwise it would need to know the runtime context of the variable to be able to tell if it's really this variable or some other local variable that just happens to be named the same. It makes the code slightly faster by avoiding variable lookup.
Since the global variable still needs to be created, so that it can be accessed by any other modules that import this module, it doesn't use less memory. For it to use less memory, you would need to: 1. use global variables, as already mentioned, 2. name them with an underscore as the first character, so that they can't be used by external modules. Then, and only then, it makes sense to use const()
. Otherwise I would recommend removing it to avoid confusion.
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.
More details: micropython/micropython#573
Hello |
Thanks @tannewt |
Updating https://github.com/adafruit/Adafruit_CircuitPython_CCS811 to 1.1.5 from 1.1.4: > Merge pull request adafruit/Adafruit_CircuitPython_CCS811#29 from mitchellhenke/patch-1 Updating https://github.com/adafruit/Adafruit_CircuitPython_Crickit to 2.1.5 from 2.1.4: > Merge pull request adafruit/Adafruit_CircuitPython_Crickit#16 from caternuson/example_update Updating https://github.com/adafruit/Adafruit_CircuitPython_GPS to 3.2.4 from 3.2.3: > Merge pull request adafruit/Adafruit_CircuitPython_GPS#17 from jomogalla/checking-input-buffer-before-reading > Merge pull request adafruit/Adafruit_CircuitPython_GPS#24 from dherrada/docs > Merge pull request adafruit/Adafruit_CircuitPython_GPS#21 from dherrada/master > Merge pull request adafruit/Adafruit_CircuitPython_GPS#23 from dherrada/docs Updating https://github.com/adafruit/Adafruit_CircuitPython_INA219 to 3.2.0 from 3.1.2: > Merge pull request adafruit/Adafruit_CircuitPython_INA219#9 from barbudor/master Updating https://github.com/adafruit/Adafruit_CircuitPython_LPS35HW to 1.1.0 from 1.0.0: > added low pressure threshold Updating https://github.com/adafruit/Adafruit_CircuitPython_Seesaw to 1.4.1 from 1.4.0: > Merge pull request adafruit/Adafruit_CircuitPython_seesaw#28 from makermelissa/master > Merge pull request adafruit/Adafruit_CircuitPython_seesaw#29 from robotics-masters/wallarug/robohatmm1 Updating https://github.com/adafruit/Adafruit_CircuitPython_SGP30 to 2.0.2 from 2.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_SGP30#15 from djmbritt/patch-1
Fixes Issue#5 and use adafruit_register