Skip to content
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

Update Button class to match debouncer logic #38

Merged
merged 3 commits into from
Mar 21, 2022

Conversation

Neradoc
Copy link
Contributor

@Neradoc Neradoc commented Mar 17, 2022

  • Button.short_count and Button.long_press now return a consistent value that only changes when Button.update() is called, like rose and fell do. This is important for the logic of the debouncer to work, where the status of the button should be stable during a "frame".

  • The active_down parameter is changed to value_when_pressed, a clearer name with the opposite value (also used in the keypad module). With active_downset to True, the active value when the button is pressed is False.

  • Expose Button.pressed and Button.released, while rose/fell remain the lower level signal direction of the edge.

Note that short_count changes value after the short_duration_ms delay, so as to not report a double click as a simple click, so you would either use pressed/released if you don't look for double clicks or short_count == 1 to detect a click, 2 for a double-click and more. Is that confusing ?

Maybe value_when_pressed and pressed/released could be moved into the Debouncer class.

  • Improve the documentation of the Button class, with init parameters description.
  • Update the gitignore to current cookiecutter while I'm here doing tests with sphinx (.env, _build, etc.)

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I tested these changes successfully with RP2040 and NeoKey Featherwing.

I do think the name value_when_pressed is more clear. and the way that short presses work does make sense to me as well.

Thanks @Neradoc

@FoamyGuy FoamyGuy merged commit 4b0f67f into adafruit:main Mar 21, 2022
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 22, 2022
Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 4.1.0 from 4.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#159 from Neradoc/staticIP-hostname-and-dns

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8563 to 1.0.2 from 1.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8563#4 from tekktrik/doc/add-typing
  > Fixed readthedocs build
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.10.12 from 3.10.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#101 from makermelissa/master
  > Fixed readthedocs build
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_Debouncer to 2.0.0 from 1.6.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Debouncer#38 from Neradoc/better-button-class

Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 0.1.2 from 0.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#1 from askpatrickw/fix-install-commmand

Updating https://github.com/adafruit/Adafruit_CircuitPython_MacroPad to 2.1.0 from 2.0.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_MacroPad#39 from Neradoc/setup-layout-on-init

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 2.1.9 from 2.1.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#83 from UnicycleDumpTruck/add-type-annotations

Updating https://github.com/adafruit/Adafruit_CircuitPython_Slideshow to 1.7.4 from 1.7.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_Slideshow#42 from tekktrik/doc/add-typing
  > Fixed readthedocs build
  > Consolidate Documentation sections of README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants