search

adafruit / Adafruit_CircuitPython_DHT

CircuitPython support for DHT11 and DHT22 type temperature/humidity devices
MIT License
179 stars 62 forks source link

Deinitialize pulsein object on exit, gave option to choose whether or not to use pulseio #46

Closed evaherrada closed 3 years ago

evaherrada commented 4 years ago

Tried to test but my pi was not cooperating

jerryneedell commented 4 years ago

@dherrada I tested this on a Pi 4B+ with a DHT22 both with and without pulseio. It works both ways but I did see more checksum errors without pulseio.

use_pulseio=False

pi@gjnpi4desk:~/projects/DHT $ python3 dht_simpletest.py
Temp: 74.1 F / 23.4 C    Humidity: 51.8% 
Temp: 74.1 F / 23.4 C    Humidity: 51.7% 
Temp: 74.1 F / 23.4 C    Humidity: 51.8% 
Checksum did not validate. Try again.
Temp: 74.1 F / 23.4 C    Humidity: 50.5% 
Temp: 73.9 F / 23.3 C    Humidity: 51.8% 
Checksum did not validate. Try again.
Temp: 74.1 F / 23.4 C    Humidity: 50.6% 
Temp: 74.1 F / 23.4 C    Humidity: 52.0%

use_pulseio=True

pi@gjnpi4desk:~/projects/DHT $ python3 dht_pulseio.py 
Temp: 74.1 F / 23.4 C    Humidity: 52.0% 
Temp: 74.1 F / 23.4 C    Humidity: 51.6% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 74.1 F / 23.4 C    Humidity: 51.6% 
Temp: 74.1 F / 23.4 C    Humidity: 51.6% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 73.9 F / 23.3 C    Humidity: 51.5% 
Temp: 74.1 F / 23.4 C    Humidity: 51.6%
jerryneedell commented 4 years ago

I also test this on a feather_m4_express and the bitbang does not work at all

use_pulseio=False

Press any key to enter the REPL. Use CTRL-D to reload.
Adafruit CircuitPython 5.4.0-beta.0-402-g5cd6bb4d5 on 2020-05-30; Adafruit Feather M4 Express with samd51j19
>>> import dht_bitbang_m4
A full buffer was not returned. Try again.
A full buffer was not returned. Try again.
A full buffer was not returned. Try again.

use_pulseio=True

import dht_pulseio_m4 Temp: 76.3 F / 24.6 C Humidity: 50.3% Temp: 76.3 F / 24.6 C Humidity: 50.1% Temp: 76.1 F / 24.5 C Humidity: 50.1% 



FYI -- I upgraded to the latest version of the CP main repository and get the same results on the M4 -- biting  does not appear to be working on the M4  and I have previously found that it does not work on an M0 either....

Is bitbang supposed to work at all under CP? If not, it should not be an option. If it should work, then something is broken.
evaherrada commented 4 years ago

@jerryneedell Ok. I think I'll add a comment somewhere in the example that explains this.

evaherrada commented 4 years ago

Is bitbang supposed to work at all under CP?

I think so. The learn guide (https://learn.adafruit.com/dht/dht-circuitpython-code#step-2980241) says it should work on any pin with PWM support, which, on the M4, is most of them (https://learn.adafruit.com/assets/78438), and, on the M0, is a lot of them (https://learn.adafruit.com/assets/46247).

jerryneedell commented 4 years ago

tested updated PR on Raspberry Pi 4B= with DHT 22

pi@gjnpi4desk:~/projects/DHT $ python3 dht_simpletest.py 
Temp: 74.8 F / 23.8 C    Humidity: 54.3% 
Temp: 74.8 F / 23.8 C    Humidity: 53.9% 
Temp: 74.7 F / 23.7 C    Humidity: 53.8% 
Temp: 74.8 F / 23.8 C    Humidity: 53.9% 
Temp: 74.8 F / 23.8 C    Humidity: 53.8% 
Temp: 74.8 F / 23.8 C    Humidity: 53.8% 
^Creceived SIGINT

Traceback (most recent call last):
  File "dht_simpletest.py", line 34, in <module>
    time.sleep(2.0)
KeyboardInterrupt

pi@gjnpi4desk:~/projects/DHT $ python3 dht_bitbang.py    --- with use_pulseio=Fasle
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.6% 
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.7% 
Temp: 74.8 F / 23.8 C    Humidity: 53.6% 
^CTraceback (most recent call last):
  File "dht_bitbang.py", line 34, in <module>
    time.sleep(2.0)
KeyboardInterrupt

also tested using pulseio on feather m4_express -- worked normally.

evaherrada commented 3 years ago

@jerryneedell Do you think it's worth taking a deeper look into the bitbang issue on CircuitPython? Is there a valid reason that anyone would want to use the bitbang method over pulseio? I think that the really important ones are having both bit-banging and pulseio working on the Pi, and having pulseio working on CircuitPython. If you agree, then I think we should do what you suggested and disable bit-banging on CircuitPython for this library.

jerryneedell commented 3 years ago

@jerryneedell Do you think it's worth taking a deeper look into the bitbang issue on CircuitPython? Is there a valid reason that anyone would want to use the bitbang method over pulseio? I think that the really important ones are having both bit-banging and pulseio working on the Pi, and having pulseio working on CircuitPython. If you agree, then I think we should do what you suggested and disable bit-banging on CircuitPython for this library.

Since bitbang does not appear to work under CP and I am not aware of anyone interested in fixing it, I agree it should be removed. Just throw an error if Pulseio is not available.

evaherrada commented 3 years ago

@jerryneedell Ok, that's what I'll do.

evaherrada commented 3 years ago

@jerryneedell To be completely honest, I didn't test this last commit outside of mock testing the if statement in the repl in circuitpython and on the raspberry pi

goldyfruit commented 3 years ago

In my case I had to add dhtDevice.exit() to my try then it worked.

kattni commented 3 years ago

@jerryneedell Dylan is off for a while. Any chance you could take over this PR?

jerryneedell commented 3 years ago

@jerryneedell Dylan is off for a while. Any chance you could take over this PR?

I’ll take a look at it tomorrow. I kind if lost track of it.

jerryneedell commented 3 years ago

@kattni I have tested this on a feather_m0 (basic) and on a Raspberry Pi 4 It works on both using pulseio and as expected throws an error on the M0 if pulseio is disabled. On the Raspberry Pi 4 there are occasional errors - more when using bitbang.

By "taking over" this PR - did you want me to do more than this testing? I do not know what the full motivation was for this PR to begin with. The issues with bitbang seem to have eclipsed the issue related to de-init.

Do you want to go ahead and merge this as it is since it appears to be working and no one else has commented on the changes. I wish there were a better way to do this since it has a large piece of code that is never used by CircuitPython (only Blinka) but I don't have anything in mind. Is the bitbang support from Blinka important enough to keep it?

Let me know how you want to proceed.

kattni commented 3 years ago

Is the bitbang support from Blinka important enough to keep it?

@makermelissa Can you weigh in on this? Please glance over the entire thread. The question regarding Blinka is in the comment from Jerry immediately before this one, and that's the thing I'm specifically looking for input on. Thanks!

kattni commented 3 years ago

@jerryneedell I'm bringing in Melissa for the Blinka part of the decision. Then we can sort out where to go from here.

jerryneedell commented 3 years ago

If I recall correctly, bitbang was originally introduced for the RPi because pulseio was not available. See #18 and #14. Are there are some SPC's supported by Blinka that still do not have pulseio? I was just wondering if it was still useful to have. It would simplify and reduce the size the library to take it out if it is not needed.

jerryneedell commented 3 years ago

especially note this comment https://github.com/adafruit/Adafruit_CircuitPython_DHT/pull/18#issuecomment-431088168

makermelissa commented 3 years ago

That's a good point @jerryneedell. There are actually only a few boards (Raspberry Pi only perhaps) that have the pulseio available, so it's probably worth keeping for the other boards that don't have the hardware support.

jerryneedell commented 3 years ago

If we are going to keep bitbang, then I think this PR is OK - possibly with one change as noted above.

kattni commented 3 years ago

@jerryneedell Can you please make the change you suggested? Then we can get this merged.

jerryneedell commented 3 years ago

@jerryneedell Can you please make the change you suggested? Then we can get this merged.

OK -- I "think" I did that correctly ...

SecT0uch commented 3 years ago

I have several remarks regarding this:

I'm using a Raspberry Pi zero W, on pin D18

I can submit a PR regarding the exit() function if needed.