update read speeds when using the properties - fix #2

[jfurcean]

This commit should help with the issue raised in #2. You should notice improved performance using the examples/nunchuk_analog_mouse.py example.

[caternuson]

Look at the approach I've pushed here:
https://github.com/caternuson/Adafruit_CircuitPython_Nunchuk/tree/speedup

The nunchuk_analog_mouse.py example has also been changed to demonstrate how the new approach would be used.

[jfurcean]

You asked to separate the issue out, so I was trying to fix the data read issue first. I don't think there is an issue with the property method once the data read issue is solved, which I believe the proposed method in this PR solves that. I think the your proposed approach will still cause the slowness in the data read if a user uses nunchuck.button_c throughout their code instead of unpacking the tuple at the top of their while loop. I think they should be able to use the nunchuck.button_c through out their code.

Have you tested the two versions of the code with the original examples/nunchuk_analog_mouse.py?

Here is mine with the Classic Controller as well. https://github.com/jfurcean/CircuitPython_WiiChuck

It seems to me to be much cleaner without the values property and the tuple unpacking

[caternuson]

Yep, thanks for making this new PR. Here we are just focused on the slowness issue. The current library definitely has issues with that.

I did try the mouse example with the existing library, your PR, and the version I linked above. The current library is slow. Your PR is noticeably faster. Same for the version I linked. The version I linked is an example that takes the approach suggested here:
#17 (comment)

While your PR does improve the performance relative to the existing library, it introduces new issues. Consider this simple example:

import board
import adafruit_nunchuk

nc = adafruit_nunchuk.Nunchuk(board.I2C())

while True:
    print(nc.joystick)

Using your PR code, it behaves like this:

(202, 58)
(202, 58)
(202, 58)
(202, 58)
(202, 58)
(197, 44)
(197, 44)
(197, 44)
(133, 35)
(133, 35)
(133, 35)
(133, 35)
(129, 47)
(129, 47)
(129, 47)
(129, 47)
(129, 47)
(129, 47)

Yes it is faster. But it returns lots of duplicate data. For the analog mouse example, that generally doesn't matter, since it doesn't show up in any noticeable way. But that behavior could be an issue for other use cases.

Using the version of the code I linked above, it behaves like this:

(225, 126)
(212, 93)
(206, 35)
(80, 53)
(59, 59)
(57, 59)
(56, 59)
(50, 69)
(49, 191)
(201, 207)
(202, 206)
(203, 205)
(207, 205)
(209, 203)
(217, 196)
(230, 146)
(231, 136)
(228, 132)

Which is the more expected behavior.

I think it will be overly complex to provide both property access and speed and make it happen automagically. More logic would be needed to prevent the behavior shown above. Another option might be to have a manual update function that users would be required to call to actually update the buffer contents. The properties just return the current buffer contents. That would look something like:

while True:
    nc.update()
    x, y = nc.joystick
    if nc.button_C:
        print("button C pressed")
    # etc

Needing to call update() is a bit clunky, but the code after that could use the familiar property access.

[jfurcean]

I am not sure I agree about that the 'duplicate data' is an issue. This same things happens with buttons in any circuit python code. I think this actually producing what is expected. Individuals using the libraries would need to handle the issue just like the c and z buttons are handled. The last approach is exactly how the Arduino library handles it and I believe it will end up producing the same results as this PR. I agree that it is clunky though and why I didn't suggest it.

[edit] I think the update() approach might be the best solution.

[dglaude]

There might be an issue with the use of time.monotonic() as in CircuitPython, with some MCU, the precision of that function change over time... https://learn.adafruit.com/clue-sensor-plotter-circuitpython/time-in-circuitpython
Maybe for mouse usage, this will become a problem.
Not that I have a solution.

[jfurcean]

Yeah, the update() method is probably the best current solution. This will align with the similar Arduino library as well.