Rotary encoder backend

[Tanmay06]

This PR includes Rotary Encoder low-level driver, high-level protocol and integration updates. It resolve #41, #151, and #133.

[Tanmay06]

I'm in middle of writing tests for the protocol, driver and the updated integration scripts. once, completed this PR would be ready to merge. However, I've discovered a buggy behavior of the encoder where there is a slight delay between the encoder input and the printed output. Additionally, I've also observed that sometimes the encoder recognizes a clockwise turn as a counter-clockwise turn and hence decreases the step rather increasing it. To ensure whether this is a hardware or a software issue, @ethanjli, I would like you to test the implementation on your hardware by running tests\io\trio\rotary_encoder.py script.

[ethanjli]

I am also getting undesired behavior:

• Sometimes when I turn it one click counterclockwise, the step does not change (it gets lost); other times, the step increments by more than one (it gets duplicated).
• Sometimes when I turn it counterclockwise, the step decreases; or when I turn it clockwise, the step sometimes increases.

Since I can also reproduce this undesirable behavior from Sikha's script, the most likely explanation is that we are handling the edges incorrectly (i.e. not robustly). The most probable cause would be bouncing of pins A and B, in which case we'd need to debounce the rotary encoder. I don't know if there are any edge cases which the current algorithm is not detecting, but that's also a possibility.

[ethanjli]

I would suggest seeing if you get better results from an off-the-shelf library: https://pypi.org/project/pigpio-encoder/ (note that some extra setup steps are required, with respect to the pigpio daemon).

[Tanmay06]

@ethanjli would you suggest some test cases for RotaryEncoder driver?

[ethanjli]

Here are some tests:

1. Prompt user to rotate the dial CW one click, get confirmation after they have finished; step should have increased by exactly 1. Repeat this 10 or 20 times (each time with user confirmation?), and step should always increase by exactly 1 each time
2. Prompt user to rotate the dial CCW one click, get confirmation after they have finished; step should have decreased by exactly 1. Repeat this 10 or 20 times (each time with user confirmation?), and step should always decrease by exactly 1 each time
3. Prompt the user to slowly rotate the dial CW 10 clicks, get confirmation after they have finished; step should have increased by exactly 10.
4. Prompt the user to slowly rotate the dial CCW 10 clicks, get confirmation after they have finished; step should have decreased by exactly 10.
5. Prompt the user to quickly rotate the dial CW, get confirmation after they have finished; step should only have increased during that period, never decreased
6. Prompt the user to quickly rotate the dial CCW, get confirmation after they have finished; step should only have decreased during that period, never increased
7. Prompt the user to rotate the dial CCW half a click and then CW half a click (so that the dial ends up where it started), get confirmation after they have finished; step should conform to some reasonable, well-defined behavior (e.g. it always/usually increments by 1, or always/usually decrements by 1, or always/usually stays the same; preferably always stays the same).

[Tanmay06]

This seems good. We can have this with some external/physical testing protocol, but on the software side we need something that can be automated with pytest. Additionally, with our current configuration, I guess, tests 3 and 4 will obviously fail because we are instantly sending any change in the rotary encoder state and the usual increments are most likely to be 1, 2 or, maximum, 3 steps, depending on the speed. I think it is more intuitive to rely on the touch input for single step change rather than fiddling with the dial. Furthermore, we can also add a test to check whether pressing the button resets the steps. And I was wondering, what if the user never uses the button to confirm their action? The number of steps will never reset.

[ethanjli]

Good points. I was thinking that the user should confirm their actions using the keyboard (i.e. using the input function or a KeyboardInterrupt, if the latter would be easier in a trio/async program). We should also have a test to check the behavior of pressing the button.

As for any automated testing, this one is tricky. One thing we can do at least to test the step counting/accumulation logic as a low-fidelity test is to manually call the callback functions and manually modify the states inside the RotaryEncoder driver object, based on what we know about how the pulses should work in a rotary encoder.

For tests 3 and 4, I am only talking about the difference in steps before the user starts rotating and after the user finishes rotating 10 clicks. A correctly-behaving rotary encoder should accumulate the steps increments (whatever their size) so that the sum of all the increments is 10.

[Tanmay06]

@ethanjli would you please check the build test results for the last commit. The errors didn't pop-up when I was testing it on my local machine. Furthermore, I've added test for rotary encoder high-level protocol. While I'm getting done with driver tests we can merge this branch and resolve PR #163 .

[ethanjli]

The errors mypy is complaining about come from running mypy on tests rather than ventserver - that could be why you didn't see the errors on your local machine, if you just ran mypy ventserver. My guess is you may want to make the rotary encoder argument optional so that you can avoid instantiating and supplying it on some of these test scripts where the mypy complaints are coming from.

When I run tox on my machine it fails - pylint complains that it's unable to import RPi.GPIO. I also don't see RPi.GPIO listed among the dependencies in pyproject.toml - where did you add the RPi.GPIO dependency?

[Tanmay06]

I was using mypy tests to check the errors explicitly in tests after I had finished writing the test for the protocol. Interestingly, when I used it again now, the error has started showing up. But, I don't understand what could have caused this when I've not even touched those files. Adding optional rotaryencoder to _trio.process_all and using functools in venstserver.application where the only things that I did that seems to have broken it. I had used poetry add RPi.GPIO to install, automatically add dependency and update the poetry.lock file. But it seems like when it failed to install the library it also didn't update the dependencies, I should've verified that. I'll update the branch by manually adding dependency to toml file and correcting the mypy issues.

[Tanmay06]

I've fixed the issues can you check and let me know if something is missing, apart from driver tests, or not working.

[ethanjli]

Everything generally looks good. When you feel it's ready to merge, please go ahead and answer the questions for records-keeping:

1. This project is licensed under Apache License v2.0 for any software, and Solderpad Hardware License v2.1 for any hardware - do you agree that your contributions to this project will be under these licenses, too?
2. Were any of these contributions also part of work you did for an employer or a client?
3. Does this work include, or is it based on, any third-party work which you did not create?

[Tanmay06]

This project is licensed under Apache License v2.0 for any software, and Solderpad Hardware License v2.1 for any hardware - do you agree that your contributions to this project will be under these licenses, too? Yes Were any of these contributions also part of work you did for an employer or a client? No Does this work include, or is it based on, any third-party work which you did not create? No

[Tanmay06]

The rotary encoder implementation looks good for now. Might need to change the low-level driver algorithm and mostly the GPIO library for better performance.

[Tanmay06]

Okay. I've already raised this in #176. Because there were some limitations to pigpio-encoder, instead of using it directly I've started implementing pigpio-encoder's algo in our driver. Can you also test pigpio-encoder's down_callback() and up_callback(). I tried using them but somehow the cb functions weren't triggered.