search

pimoroni / i2cdevice-python

Domain-specific language for describing smbus/i2c register maps
MIT License
26 stars 14 forks source link

Experimental new API endpoints #3

Closed Gadgetoid closed 5 years ago

Gadgetoid commented 5 years ago

This changeset introduces a new set and get method for i2cdevice. The purpose of these methods is to provide a less-bonkers approach to writing and reading values than the current (MicroPython incompatible) system of overriding getattribute and redirecting attribute lookups to our internal register dictionary.

Get

The get method accepts a register name, reads the register and returns a namedtuple of all the available fields. These changes turn accessing multiple registers from:

with device.REGISTER_NAME as REGISTER_NAME:
    status = REGISTER_NAME.get_status()
    interrupt = REGISTER_NAME.get_interrupt()

To:

my_register= device.get('REGISTER_NAME')
my_register.status
my_register.interrupt

Note: It is important to understand that my_register is, at this point, a stateless namedtuple that's disconnected from the hardware. If it's a non-volatile register (IE: it never changes) then you can keep this object for the lifetime of your program. If you want to read it again (in the case of a volatile register) then you must call device.get('REGISTER_NAME') again.

Set

Set cleans up the current conflation of read and write whereupon using this pattern:

with device.REGISTER_NAME as REGISTER_NAME:
    REGISTER_NAME.set_status(status)
    REGISTER_NAME.set_interrupt(interrupt)
    REGISTER_NAME.write()

Could result in errors where the user (me, usually) forgets to call write() and their expected register changes are thus never written. set is explicit about the fact it's for writing to a register, and thus does not allow for this mistake, the above code would become:

device.set('REGISTER_NAME',
                status=status,
                interrupt=interrupt)

This still allows for multi-register writes, but it is disambiguated from the get method and ensures your changes are always written to device. Under the hood this function reads the current register state, locks it, updates all the fields you have supplied, writes the changed register value(s) back to the device, and unlocks the register.

This change should come with a deadline for deprication of the existing, confusing and fragile API methods- removing the pattern device.REGISTER_NAME.get_field_name() in favour of device.get('REGISTER_NAME').field_name. This is a subtle aesthetic change on the face of it, but under the hood the latter method does not rely on any fragile and overzealous getattribute trickery.

@rabid-inventor and @septolum - since you've both worked with i2cdevice I'd appreciate your feedback here. All of our libraries can, and should be migrated over to this new style pretty easily and this will - hopefully (@septolum correct me if I'm wrong) facilitate MicroPython compatibility without having to use the ugly (and non read/write combining) device.set_field('REGISTER_NAME', 'FIELD_NAME', value) and `device.get_field('REGISTER_NAME', 'FIELD_NAME') approach.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 23

Files with Coverage Reduction New Missed Lines %
i2cdevice/init.py 3 98.37%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 13: 0.2%
Covered Lines: 206
Relevant Lines: 209
💛 - Coveralls
Gadgetoid commented 5 years ago

A possible flaw in this approach may be the instability of namedtuple in CircuitPython. See: https://circuitpython.readthedocs.io/en/3.x/docs/library/collections.html

There's nothing particularly special about namedtuple as a return type, it's just a fancy class that implements a property for each register field. However, reimplimenting it to avoid potential issues with CircuitPython's namedtyple might result in us bumping into the same problems that made the getattribute overloading approach untenable.

Gadgetoid commented 5 years ago

Libraries using i2cdevice -