RotaryDial icon indicating copy to clipboard operation
RotaryDial copied to clipboard

Incorrect digit can be read from a partial pulse train

Open gerchikov opened this issue 2 years ago • 2 comments

Nice library! Very succinct and efficient. Truly a pleasure to read and use.

I suspect there is one failure mode though. If a client delays read()ing out the latest dialed digit, and if the next digit is being dialed when client read()s, then the next digit may have some but not all of its pulses skipped. This will result in a digit that is lower than what's been actually dialed returned next. And the user would have no way to detect this and reject corrupted input.

One potential solution is to track when a new pulse train begins -- even if the previous digit is still available(). If that happens, discard either the previous (complete) digit or the entire new pulse train(s) until the last available digit is read. Lossy! But at least no false/partial digits. Another possibility is to queue digits as they are dialed, and read them, first-in-first-out, form the queue. Still lossy, but not until the queue overflows. Finally, adding/taking into account the third (dial/ready) wire could help, too.

Either solution is probably too much trouble, and -- realistically -- of little value in real life. But I thought I'd mention this to warn users of this failure mode, so that they make sure to read() timely, or else risk getting false digits.

gerchikov avatar Jan 04 '23 09:01 gerchikov

Thanks for your ideas. Made this mostly for fun, so i do not often think about improving it. But i will surely review PR if you propose one :-)

Harvie avatar Jan 04 '23 10:01 Harvie

Fair enough! :) Here is an attempt. (I took the liberty to also update the lib version. It may need a new tag (in your repo) in order to be picked up by Arduino lib manager. I do not know if the tag that I added in my fork will make it over when merged.)

gerchikov avatar Jan 16 '23 06:01 gerchikov