adafruit / Adafruit_CircuitPython_seesaw

seesaw helper IC driver for circuitPython
MIT License
62 stars 36 forks source link

Propagate delay param from read() to analog_read() #90

Closed todbot closed 2 years ago

todbot commented 2 years ago

The seesaw.analog_read() function has a fixed time.sleep(0.001) in addition to the default time.sleep(0.008) in seesaw.read(). This PR removes that fixed delay, allows the user to specify the delay used in read(), and allows the delay to be specified in the creation of AnalogInput objects.

Tested by @jedgarpark where he was experiencing an accumulated >40 milliseconds of UI lag when reading four NeoSliders.

todbot commented 2 years ago

I don't understand what pre-commit is complaining about in the CI. When I run black --check --target-version=py35 . locally I get:

% black --check --target-version=py35 .
All done! ✨ 🍰 ✨
13 files would be left unchanged.
jedgarpark commented 2 years ago

I just tested this on a Feather RP2040 with four NeoSliders and a NeoKey 1x4 all over I2C. This new library improves the problem I have with latency, setting my analog read delay down to about 0.001 or 0.0005. No difference (nor problems) going lower than that.

caternuson commented 2 years ago

This seems fine. The defaults retain the current timings, except for removing the 1ms delay that was probably safe guarding attempts to read too fast on the SAMD09 based boards. The reference guide mentions this requirement here: https://learn.adafruit.com/adafruit-seesaw-atsamd09-breakout/analog-to-digital-converter I think OK to remove and put that requirement on user to insure in their code.

And with this change, can override defaults for ATtiny817.

Would like @ladyada to take a look also though. Could be Reasons™ for these I'm not aware of.

caternuson commented 2 years ago

thanks!