adafruit / Adafruit_CircuitPython_EMC2101

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

adding get_functions #28

Closed jposada202020 closed 1 year ago

jposada202020 commented 1 year ago

closes #27

For the reviewer: There are two changes here

  1. I have changed the name for each class initialization, due to CircuitPython MRO behaviour, when using the EMC2101_LUT class, and going up to EMC2101 main __init__the call self.initializate()there was calling the EMC2101_LUT initialization and not the EMC2101 one. I guess that is why they were using the super().initialization line.. But who knows. This solves the issue when the line below was change to True

https://github.com/adafruit/Adafruit_CircuitPython_EMC2101/blob/aea77f5a62ce099c99a35bfe7eae16676dc9fb8b/adafruit_emc2101/emc2101_ext.py#L135

  1. I Have changed the property in the parent class to get_xxxxx, to avoid property calling conflicts.

I have test with the three examples, be aware that, that I could not test the outside_temperature as I do not how to wire it... I did test with a fan. I use a mock up temperature for the tests. No other changes were made, this PR is solely to solve the issue and no to look further in Library problems

@brody please could you test, thanks :) Hope you have at least a start on how to change the library. Good Luck

EuropaSteve commented 4 months ago

Hello, I'm using a Qt Py RP2040, 128x64 OLED, and EMC 2101 Fan Controller with remote sensor. When I try to use the look up table I get 'super' object has no attribute 'i2c_device'. After googling around I found https://github.com/adafruit/Adafruit_CircuitPython_EMC2101/pull/28/files If I make the changes to library as listed everything works. The pull request was closed as complete but none of the changes are in the latest revisions of the EMC2101 library. Does this need to be re-opened?

dhalbert commented 4 months ago

@EuropaSteve This PR was never merged, as I think you see. @jposada202020 Do you remember why you closed this PR?