Debounce cleanups #365
Merged
Debounce cleanups #365
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I need Debounce<SystemStatus> for the wifi disconnection watchdog and it wasn't possibly to instantiate with this setup. I made a quick test with PlatformIO's inspection tool. I had a random test program which used DebounceBool and DebounceInt of some input. The RAM use was 51.0 KB and Flash use was 1015.4 KB. After changing the transform to header-only templates, RAM use remained the same but flash use dropped to 1011.0 KB.
Merged
ba58smith
reviewed
Jan 25, 2021
mairas
force-pushed
the
debounce_cleanups
branch
from
69dd8e9 to
780da88
Compare
|
Since this PR includes a change in the behavior logic, if @joelkoz has any doubt, it'd be great if you could test it before merging so that you're not left in the dark (pun absolutely intended!). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I did a bit of reworking on the networking logic and removed the "reboot on wifi disconnect" from the Networking class and made it a consumer of SystemState in SensESPApp. For this, I required Debounce to only trigger the reset if the state had been stable for 3 minutes.
First, I noticed that due to the Debounce template implementation, it was impossibly to instantiate Debounce. I did a quick benchmark to verify the claim in Issue #287 that "if you put the entire implementation in the header file (esp. the configuration stuff) the compiler will generate a TON of redundant code." (The issue sounded a bit peculiar because basically all of STL is, well, templates.)
I have a random SensESP test program that takes some changing AnalogInput, applies hysteresis and rounding and then Debounces the boolean and integer values. Compiled with the original implementation, the RAM footprint was 51.0 KB and flash footprint 1015.4 KB. I then refactored the code into a standard headers-only template class. The RAM footprint remained at 51.0 KB while the flash footprint dropped to 1011.0 KB. So, that is the first major change in this PR.
The second change relates to the Debouncing logic. I am debouncing the system state which gets regular WifiDisconnected updates (the same value all over again). The original logic reset the timer for every update, even if they repeated the same value. That didn't quite make sense to me - it would be impossible to debounce any polled values with this logic. I shuffled the code a bit around but the only change in the end was that repeated values are considered stable instead of resetting the timer every time.