-
Notifications
You must be signed in to change notification settings - Fork 33
implement timeout in read_pulses() #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
BTW, read_pulses() documentation does not specify the units as microseconds (us). It's very easy to make mistakes with seconds vs milliseconds vs microsecond timeouts, more so when undocumented. One could argue that input_pulses object is determining the units and it may be documented there but ultimately that's not as useful to the casual programmer. |
I tested a third remote, one that's identical to https://www.adafruit.com/product/389 in appearance. It exhibits the same behaviour with the final long pulse missing for very short duration button touches. |
Possible same issue (not the |
#18 changes things in this space... |
@kevinjwalters have you tried out PR #18? I would be happy to help you if you need additional support of some kind. You might also be interested in https://github.com/fmorton/Makers_CircuitPython_remote_control for more simple irremote access. |
I've not had a chance to use #18. I'm working on a few other things at the moment but when I get back to IR I'll have a look. I did mention #18 in the forum post discussing IR problems that sounded like what I had experienced. |
I'd like to revive this thread, if possible. As per #32, read_pulses() still blocks even with the non-blocking keyword. This means one can't integrate ANY other bits onto a microcontroller that one plans to use with an IR sensor, which is a bummer. I've tried working with @fmorton 's work too, but it has similar but different problems. It certainly doesn't block itself, and thereby allows for repeating ir codes, but it seems to block anything else in a while loop that might want access to the hardware. Even on/off digital buttons become unreliable and don't seem to get access to happen. I'm a software guy, not a hardware guy, so I don't know how to help much. I'd love to chip in if I may, and I can provide debug stuff and sample code. |
See #42 for a proposed solution that I believe is actually nonblocking. |
I think it is safe to close this now that #42 is in. |
Thanks @danielballan! |
I've been writing some code based on https://learn.adafruit.com/infrared-ir-receive-transmit-circuit-playground-express-circuit-python/ir-test-with-remote to read/decode the NEC codes from a humble little remote control. I wanted to be able to match the codes with the button pressed to partially generate the code.
I hit a problem where decoder.read_pulses(pulsein) sometimes does not return after I have pressed a button on the remote. I'm new to IR so it took me a while to work out what's going on. I am sometimes missing a final pulse, occasionally there's a 40ms long pulse but more often it's missing. The long pulse (> max_pulse, default 10000us) causes read_pulses() to return but most of the time it's absent (not clear if it's not sent xor not received).
I've made this more robust for the remote I'm using by some simple additions to read_pulses to implemented a timeout. Would this be useful for all?
I've tried a LG remote from an older commercial product and it does the same. Sometimes there's an end 45ms pulse, sometimes not. Actually, I may have found out the difference here, on the both remotes short duration depress (stabbing at buttons) appear to yield no final 40ms/45ms pulse.
From my empirical testing it certainly looks like read_pulses would benefit from a timeout after receiving some pulses to allow the data to be returned and decoded.
I'll post my code in a few days and I can do a PR against the library if you like the idea of this feature. Current implementation uses a new named parameter called timeout with default of None. Value is either microsecond value or "max_pulse" to link it to the max_pulse value.
The text was updated successfully, but these errors were encountered: