adafruit/Adafruit_CircuitPython_SHTC3

Calling the sleeping getter before the setter would throw an error

tcfranks opened this issue · 8 comments

Discovered this curiosity in the PR for Annotations: there is a setter / getter for "sleeping" line 160-175), but there is no self._cached_sleep defined in the original code, only self._sleeping, which is referenced in the measurements function. self._cached_sleep is only referenced in the getter / setter functions. is this intentional? or an error? Seems like calling the getter before the setter would throw an error at least....and no other code references this, so I'm wondering if it was intended to be a manual setter / getter for the self.sleeping boolean....

@tcfranks I took a look and I think you are correct, it does seem to be an issue. Did you want to file one for it?

@gamblor21 - so you concur, but do you know what should be fixed? After reviewing this I think the getter/setter point to self.sleeping, since that is what other functions set to signal sleeping or not. Actually I think I'll set up an emulation to verify my assumptions about this first.....

self._cached_sleep should be set in init(). Probably best to do that through the setter so the register writes occur also and the cached value agrees with hardware setting.

self._cached_sleep = None  # create it
self.sleeping = False  # set it

@caternuson that was what I thought as well

@caternuson
@tekktrik, @FoamyGuy - perhaps you want to weigh in on this conversation?

Don't have a physical unit to test with, so did a code simulation by copying it to a pico and commenting out the actual I2C calls. So the
self.sleeping = False
in the init actually triggers the sleeping setter. In fact, you can no longer call the sleeping setter or getter using function notation, i.e. self.sleeping(True), you have to use an assignment statement. I'm not sure if that's a 'normal trick' in python, or , since I'm coming from other languages, if it's the most horrifying thing I've seen.

Anyway, the init fails in the simulation, because the self.sleeping = false statement triggers the setter, and it can't find the self._cached_sleep attribute to set. So that's simple, just add it to the init to create it. That said, that's all I'm going to submit for this.

I'm trying to recreate this and can not. The current init is calling the sleep setter, which then creates the cached value. So seems OK.

This simple test is working:

Adafruit CircuitPython 7.3.3 on 2022-08-29; Adafruit QT Py M0 with samd21e18
>>> import board
>>> import adafruit_shtc3
>>> shtc3 = adafruit_shtc3.SHTC3(board.I2C())
>>> shtc3.sleeping
False
>>> 

Can someone post example code that demonstrates the issue.

In fact, you can no longer call the sleeping setter or getter using function notation,

This is normal behavior, we're actually taking a function and turning it into a descriptor, so it no longer uses function call notation.

And the setter should be able to set _cached_sleep, so I would expect the successful behavior @caternuson described.

closing (see comments in PR and issue) as CNR