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

evaherrada · 2020-07-16T21:19:18Z

No description provided.

… not to use pulseio

evaherrada · 2020-07-16T21:19:45Z

Tried to test but my pi was not cooperating

jerryneedell · 2020-07-17T09:45:46Z

@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%

examples/dht_simpletest.py

jerryneedell · 2020-07-17T10:30:22Z

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 · 2020-07-17T13:28:09Z

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

evaherrada · 2020-07-17T13:58:15Z

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 · 2020-07-17T18:39:43Z

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 · 2020-07-24T14:47:25Z

@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 · 2020-07-24T16:56:29Z

@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 · 2020-07-24T17:03:28Z

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

evaherrada · 2020-07-24T18:53:59Z

@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 · 2020-08-14T15:39:55Z

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

kattni · 2020-08-17T19:54:00Z

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

jerryneedell · 2020-08-17T22:37:03Z

@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 · 2020-08-18T10:47:02Z

@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 · 2020-08-19T16:45:48Z

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 · 2020-08-19T16:46:15Z

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

jerryneedell · 2020-08-19T17:09:43Z

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 · 2020-08-19T17:17:50Z

especially note this comment #18 (comment)

makermelissa · 2020-08-19T17:26:32Z

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.

adafruit_dht.py

jerryneedell · 2020-08-19T17:40:18Z

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

kattni · 2020-08-19T18:03:27Z

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

jerryneedell · 2020-08-19T18:48:19Z

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

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

jerryneedell

added doctoring for new kwarg - use_pulseio
tested on RPi 4

kattni

Thanks everyone who worked on this!

SecT0uch · 2020-11-07T11:56:49Z

I have several remarks regarding this:

Shouldn't the exit() function be automatically called after a measure with pulseio ?
Shouldn't we quiet the exit() function? (Remove the print())

Impossible to measure using Bitbang for me:

pi@raspberrypi:~ $ python3 dht_simpletest.py 
A full buffer was not returned. Try again.
A full buffer was not returned. Try again.
A full buffer was not returned. Try again.
A full buffer was not returned. Try again.

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

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

Deinitialize pulsein object on exit, gave option to choose whether or…

5c1da73

… not to use pulseio

jerryneedell reviewed Jul 17, 2020

View reviewed changes

examples/dht_simpletest.py Outdated Show resolved Hide resolved

evaherrada added 2 commits July 17, 2020 09:44

Cleaned up example

16400df

Formatting

5ea7589

evaherrada requested a review from a team July 17, 2020 18:58

Added exception for trying to bitbang in circuitpython

a554d72

evaherrada requested a review from jerryneedell July 24, 2020 18:52

kerel-fs mentioned this pull request Aug 14, 2020

Unable to set line 4 to input #27

Open

jerryneedell reviewed Aug 19, 2020

View reviewed changes

adafruit_dht.py Show resolved Hide resolved

add docstring for use_pulseio kwarg

a515f92

jerryneedell approved these changes Aug 19, 2020

View reviewed changes

kattni approved these changes Aug 19, 2020

View reviewed changes

kattni merged commit a67ac23 into master Aug 19, 2020

SecT0uch mentioned this pull request Nov 12, 2020

Support for other boards mudpi/mudpi-core#15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

evaherrada commented Jul 16, 2020

evaherrada commented Jul 16, 2020

jerryneedell commented Jul 17, 2020 •

edited

Loading

jerryneedell commented Jul 17, 2020 •

edited

Loading

evaherrada commented Jul 17, 2020

evaherrada commented Jul 17, 2020 •

edited

Loading

jerryneedell commented Jul 17, 2020

evaherrada commented Jul 24, 2020

jerryneedell commented Jul 24, 2020 •

edited

Loading

evaherrada commented Jul 24, 2020

evaherrada commented Jul 24, 2020

goldyfruit commented Aug 14, 2020

kattni commented Aug 17, 2020

jerryneedell commented Aug 17, 2020

jerryneedell commented Aug 18, 2020 •

edited

Loading

kattni commented Aug 19, 2020

kattni commented Aug 19, 2020

jerryneedell commented Aug 19, 2020 •

edited

Loading

jerryneedell commented Aug 19, 2020

makermelissa commented Aug 19, 2020

jerryneedell commented Aug 19, 2020

kattni commented Aug 19, 2020

jerryneedell commented Aug 19, 2020

jerryneedell left a comment

kattni left a comment

SecT0uch commented Nov 7, 2020 •

edited

Loading

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

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

Conversation

evaherrada commented Jul 16, 2020

evaherrada commented Jul 16, 2020

jerryneedell commented Jul 17, 2020 • edited Loading

jerryneedell commented Jul 17, 2020 • edited Loading

evaherrada commented Jul 17, 2020

evaherrada commented Jul 17, 2020 • edited Loading

jerryneedell commented Jul 17, 2020

evaherrada commented Jul 24, 2020

jerryneedell commented Jul 24, 2020 • edited Loading

evaherrada commented Jul 24, 2020

evaherrada commented Jul 24, 2020

goldyfruit commented Aug 14, 2020

kattni commented Aug 17, 2020

jerryneedell commented Aug 17, 2020

jerryneedell commented Aug 18, 2020 • edited Loading

kattni commented Aug 19, 2020

kattni commented Aug 19, 2020

jerryneedell commented Aug 19, 2020 • edited Loading

jerryneedell commented Aug 19, 2020

makermelissa commented Aug 19, 2020

jerryneedell commented Aug 19, 2020

kattni commented Aug 19, 2020

jerryneedell commented Aug 19, 2020

jerryneedell left a comment

Choose a reason for hiding this comment

kattni left a comment

Choose a reason for hiding this comment

SecT0uch commented Nov 7, 2020 • edited Loading

jerryneedell commented Jul 17, 2020 •

edited

Loading

jerryneedell commented Jul 17, 2020 •

edited

Loading

evaherrada commented Jul 17, 2020 •

edited

Loading

jerryneedell commented Jul 24, 2020 •

edited

Loading

jerryneedell commented Aug 18, 2020 •

edited

Loading

jerryneedell commented Aug 19, 2020 •

edited

Loading

SecT0uch commented Nov 7, 2020 •

edited

Loading