bxparks / AceButton

An adjustable, compact, event-driven button library for Arduino that debounces and dispatches events to a user-defined event handler.
MIT License
385 stars 37 forks source link

proposed deprecation of AdjustableButtonConfig into ButtonConfig #13

Closed bxparks closed 6 years ago

bxparks commented 6 years ago

Hello everyone,

Thanks for using the AceButton library. I am thinking of making a change to the ButtonConfig class of the library, and was wondering if anyone has any strong opinions.

Background

When I first created the AceButton library, I was aiming to have a low RAM requirement for the library. Coming from the cloud computing world (where I have access to GiB and TiB of RAM), it was difficult to wrap my head around a programming environment where I have only 2kB of RAM (ATmega328P processor on an UNO or Nano). I made ButtonConfig mostly immutable, containing compile-time constants that seemed reasonable for various timing parameters, and used virtual functions to dispatch down to the AdjustableButtonConfig subclass to allow people to override those parameters if they wanted to.

Proposal

For the next revision of the AceButton library, I am proposing to merge the AdjustableButtonConfig class into the ButtonConfig class, and remove most of those virtual methods that retrieve those timing parameters. The AdjustableButtonConfig class will be kept for backwards compatibility, but it will become just a empty shell subclass of ButtonConfig with no additional features. I can remove that class in later versions.

Reasons

1) In theory, making ButtonConfig adjustable should increase the RAM usage by 12 bytes (because there are 6 uint16_t timing parameters in AdjustableButtonConfig which are not present in ButtonConfig). But in practice, I have found that the compiler optimizes the code in strange ways, and the overal RAM usage does not actually increase in many cases.

2) Removing the virtual method on the various getters allows the compiler to inline those methods, instead of using a virtual dispatch, and makes the AceButton.check() method run faster. Of course, we are talking about a difference between, say, 17 microseconds versus 14 microseconds, so maybe this is micro-optimizing, but I think a simple button library should have as little overhead as possible.

3) Removing AdjustableButtonConfig makes the code simpler, easier to maintain, easier to document, and hopefully easier to use.

Benchmarks

Here are some benchmarks. I used Arduino IDE version 1.8.6, using the arduino:avr:nano:cpu=atmega328old fully-qualified board identifier. Other processors have so much more RAM than the ATMega328P, (e.g. 64kB for Teensy 3.2, 80kB for ESP8266, ~300kB for ESP32), that we will only worry about RAM size for the ATMega328P family of boards.

CPU Time

AutoBenchmark

before:

------------------------+-------------+---------+
button event            | min/avg/max | samples |
------------------------+-------------+---------+
idle                    |  16/ 16/ 24 | 1925    |
press/release           |   8/ 18/ 28 | 1920    |
click                   |   8/ 17/ 28 | 1922    |
double click            |   8/ 16/ 32 | 1921    |
long press/repeat press |   8/ 19/ 28 | 1919    |
------------------------+-------------+---------+

after:

------------------------+-------------+---------+
button event            | min/avg/max | samples |
------------------------+-------------+---------+
idle                    |  12/ 13/ 20 | 1934    |
press/release           |   8/ 14/ 20 | 1925    |
click                   |   8/ 14/ 24 | 1925    |
double click            |   8/ 13/ 24 | 1925    |
long press/repeat press |   8/ 15/ 24 | 1927    |
------------------------+-------------+---------+

Program Size and RAM Usage

HelloWorld

SingleButton

StopWatch

TunerButtons

ClickVersusDoubleClickUsingBoth

CapacitiveButton

Summary

For the most part, the new code consumes less flash memory, but surprisingly, it often uses the same amount of static RAM.

bxparks commented 6 years ago

No objections, so I'm going to do this simplification.