adafruit / Adafruit_CircuitPython_EMC2101

CircuitPython driver for EMC2101 brushless fan controller
MIT License
3 stars 9 forks source link

Overriding property getter and setters is not supported in Circuitpython #27

Open Neradoc opened 1 year ago

Neradoc commented 1 year ago

As reported on the forum: https://forums.adafruit.com/viewtopic.php?t=199790 Trying to use the LUT example results in:

Lut: 27 deg C => 25.0% duty cycle
34 deg C => 50.0% duty cycle
42 deg C => 75.0% duty cycle
Traceback (most recent call last):
  File "<stdin>", line 45, in <module>
  File "adafruit_emc2101/emc2101_ext.py", line 230, in fan_speed
  File "adafruit_emc2101/__init__.py", line 267, in fan_speed
  File "adafruit_register/i2c_struct.py", line 80, in __get__
AttributeError: 'super' object has no attribute 'i2c_device'

Can anyone with the module test it on a Circuitpython board and confirm ?

I don't pretend to understand why it's that error, but when I did some tests I ended up noticing that in Circuitpython you can't override @property accessors from subclasses, which emc2101_ext and emc2101_sub do.

Here is where it does it for example: https://github.com/adafruit/Adafruit_CircuitPython_EMC2101/blob/aea77f5a62ce099c99a35bfe7eae16676dc9fb8b/adafruit_emc2101/emc2101_ext.py#L223-L230

This works in C python, but not in Circuitpython.

class Thing:
    def __init__(self, i2c):
        self.i2c_device = i2c
    @property
    def i2c(self):
        print(self)
        return self.i2c_device

class Thing_sub(Thing):
    def __init__(self, i2c):
        super().__init__(i2c)
    @property
    def i2c(self):
        return super().i2c

c = Thing_sub("I2C")
print(c.i2c)
code.py output:
<super: <class 'Thing_sub'>, <Thing_sub object at 0x3fd8c9a0>>
Traceback (most recent call last):
  File "code.py", line 17, in <module>
  File "code.py", line 14, in i2c
  File "code.py", line 7, in i2c
AttributeError: 'super' object has no attribute 'i2c_device'

It's like super().i2c does call the i2c getter but with self being some super object that isn't what it should be ? I don't know what a solution would be, maybe change the accessor methods to call a _set_fan_speed() method that could be overloaded in the sub classes, and called with super() as needed, since that form of overloading is known to work.

jposada202020 commented 1 year ago

@Neradoc confirmed

Adafruit CircuitPython 8.0.2 on 2023-02-14; Adafruit Feather RP2040 with rp2040
>>> import em
Lut: 27 deg C => 25.0% duty cycle
34 deg C => 50.0% duty cycle
42 deg C => 75.0% duty cycle
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "em.py", line 23, in <module>
  File "/lib/adafruit_emc2101/emc2101_ext.py", line 230, in fan_speed
  File "/lib/adafruit_emc2101/__init__.py", line 267, in fan_speed
  File "/lib/adafruit_register/i2c_struct.py", line 80, in __get__
AttributeError: 'super' object has no attribute 'i2c_device'
>>> 
jposada202020 commented 1 year ago

Another possibility is to move the check_status to the parent class, and remove the property, because at the moment the check_status is disabled here: https://github.com/adafruit/Adafruit_CircuitPython_EMC2101/blob/aea77f5a62ce099c99a35bfe7eae16676dc9fb8b/adafruit_emc2101/emc2101_ext.py#L135 However, if we set this to TRUE using the same example


>>> import em
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "em.py", line 12, in <module>
  File "/lib/adafruit_emc2101/emc2101_lut.py", line 67, in __init__
  File "/lib/adafruit_emc2101/emc2101_ext.py", line 141, in __init__
  File "/lib/adafruit_emc2101/__init__.py", line 192, in __init__
  File "/lib/adafruit_emc2101/emc2101_lut.py", line 77, in initialize
  File "/lib/adafruit_emc2101/emc2101_lut.py", line 176, in lut_enabled
  File "/lib/adafruit_emc2101/emc2101_ext.py", line 152, in _check_status
  File "/lib/adafruit_emc2101/emc2101_ext.py", line 194, in check_status
RuntimeError: Status alert
brodyb1 commented 1 year ago

OP here from Adafruits forum, is this something I can make the change to or does it need to be updated and posted ?

jposada202020 commented 1 year ago

@brodyb1 hello :) just to clarify, when you say make the change to what exactly does this mean? Library needs to be updated and posted once a solution is found.

brodyb1 commented 1 year ago

@jposada202020 Haha sorry not a programmer, just tweak codes till it works. I meant editing the lib myself ( assuming someone here knows the fix) but I'm guessing now that only the creator can do that?

jposada202020 commented 1 year ago

@brodyb1 :) no worries, I did make the example to work, however, I do not have a fan so I cannot change the library as there are certain properties that need a close circuit to avoid raise an error. Let me know and I could share my tweaks :)

brodyb1 commented 1 year ago

@jposada202020 yes please!

brodyb1 commented 1 year ago

@jposada202020 Getting this error now, RuntimeError: No pull up found on SDA or SCL; check your wiring. I'm using the 4 pin STEMMA qt for the connections. I found this but not sure if it applies https://forums.adafruit.com/viewtopic.php?t=187874

jposada202020 commented 1 year ago

@brodyb1 strange.. I do not have the fan connected that is why the rpm are not changing but it is working for me


Adafruit CircuitPython 8.0.4 on 2023-03-15; Adafruit QT2040 Trinkey with rp2040
>>> import lut_example
Lut: 27 deg C => 25.0% duty cycle
34 deg C => 50.0% duty cycle
42 deg C => 75.0% duty cycle
25% duty cycle is 82.399994 RPM:
50% duty cycle is 82.399994 RPM:
75% duty cycle is 82.399994 RPM:
>>> 

When you say four pint connection are you talking to the sensor?
brodyb1 commented 1 year ago

@jposada202020 It goes Trinkey > EMC2101 > 3V to 5V Level Booster > Fan. All of the Adafruit boards have the Stemma QT , the JST SH 4 pin connectors, https://learn.adafruit.com/introducing-adafruit-stemma-qt/technical-specs. Blue wire is SDA and Yellow wire is SCL, the only time I got the SDA/SCL error before was when I mixed up the SDA/SCL wire with the tach/pwm wires. Stemma QT is an i2c https://learn.adafruit.com/emc2101-fan-controller-and-temperature-sensor/pinouts so I use i2c = board.STEMMA_I2C() instead of i2c = board.I2C() . Apologies if you know all this, just trying to clarify the best I can!

jposada202020 commented 1 year ago

@brodyb1 no worries. Glad that you are not having the SDA/SCl error know. could you try the changes here to see if your issue has improved? https://github.com/adafruit/Adafruit_CircuitPython_EMC2101/pull/28/files