diff options
| author | Selene ToyKeeper | 2020-09-14 02:48:34 -0600 |
|---|---|---|
| committer | Selene ToyKeeper | 2020-09-14 02:48:34 -0600 |
| commit | 46db889558b953d5a8831c7912bfff4570ae9305 (patch) | |
| tree | b8ee1f10e58a2472dd00a9f37e770a6cb1c3b8f9 | |
| parent | fixed bug: ramp 2H at floor went up instead of staying at floor (diff) | |
| download | anduril-46db889558b953d5a8831c7912bfff4570ae9305.tar.gz anduril-46db889558b953d5a8831c7912bfff4570ae9305.tar.bz2 anduril-46db889558b953d5a8831c7912bfff4570ae9305.zip | |
fixed bug: ticks_since_last_event wasn't getting reset on button hold release
The visible symptom was: Ramp up for 1s or longer, release, wait more than 1s,
then hold again. It should ramp up, but it would ramp down instead. The
clause for resetting ramp_direction wasn't happening, because the EV_tick
counter started at a value higher than 1s where it would normally trigger.
The underlying cause was a bit complicated. Recent changes in PCINT_inner()
were causing ticks_since_last_event to get set to 0 (at push_event()) and then
back to its previous value (at emit_current_event()). The EV_tick counter would
then start at whatever the button release event used.
The fix involved removing the part of emit_current_event() where it would
set ticks_since_last_event to "arg". That line was a very old bug which simply
hadn't caused any visible issues until recently. Instead, it needs to set
ticks_since_last_event more carefully, at other locations. Specifically, it
resets to 0 now in empty_event_sequence() and one more location in the deferred
WDT handler (when HOLD_TIMEOUT triggers).
Additionally, push_event() was only ever used from PCINT_inner()... so I moved
the tick reset logic to PCINT_inner() instead. This allows us to decrease size
by about 10 bytes, since PCINT_inner() no longer needs to copy the counter before
it gets reset. However, it also means push_event() should never be called from
any other function.
Diffstat (limited to '')
| -rw-r--r-- | spaghetti-monster/fsm-events.c | 7 | ||||
| -rw-r--r-- | spaghetti-monster/fsm-events.h | 2 | ||||
| -rw-r--r-- | spaghetti-monster/fsm-pcint.c | 4 | ||||
| -rw-r--r-- | spaghetti-monster/fsm-wdt.c | 4 |
4 files changed, 9 insertions, 8 deletions
diff --git a/spaghetti-monster/fsm-events.c b/spaghetti-monster/fsm-events.c index ad869a6..93c38c9 100644 --- a/spaghetti-monster/fsm-events.c +++ b/spaghetti-monster/fsm-events.c @@ -25,6 +25,7 @@ void empty_event_sequence() { current_event = EV_none; + ticks_since_last_event = 0; // when the user completes an input sequence, interrupt any running timers // to cancel any delays currently in progress // This eliminates a whole bunch of extra code: @@ -33,8 +34,9 @@ void empty_event_sequence() { interrupt_nice_delays(); } -uint8_t push_event(uint8_t ev_type) { - ticks_since_last_event = 0; // something happened +uint8_t push_event(uint8_t ev_type) { // only for use by PCINT_inner() + // don't do this here; do it in PCINT_inner() instead + //ticks_since_last_event = 0; // something happened // only click events are sent to this function current_event |= B_CLICK; @@ -208,7 +210,6 @@ void emit(Event event, uint16_t arg) { } void emit_current_event(uint16_t arg) { - ticks_since_last_event = arg; emit(current_event, arg); } diff --git a/spaghetti-monster/fsm-events.h b/spaghetti-monster/fsm-events.h index 871c6a1..f83d306 100644 --- a/spaghetti-monster/fsm-events.h +++ b/spaghetti-monster/fsm-events.h @@ -202,7 +202,7 @@ static volatile uint16_t ticks_since_last_event = 0; void empty_event_sequence(); -uint8_t push_event(uint8_t ev_type); +uint8_t push_event(uint8_t ev_type); // only for use by PCINT_inner() #define EMISSION_QUEUE_LEN 16 diff --git a/spaghetti-monster/fsm-pcint.c b/spaghetti-monster/fsm-pcint.c index eacc699..021a076 100644 --- a/spaghetti-monster/fsm-pcint.c +++ b/spaghetti-monster/fsm-pcint.c @@ -101,9 +101,9 @@ void PCINT_inner(uint8_t pressed) { emit_current_event(0); } else { // user released button // how long was the button held? - uint16_t ticks_since_last = ticks_since_last_event; push_event(B_RELEASE); - emit_current_event(ticks_since_last); + emit_current_event(ticks_since_last_event); } + ticks_since_last_event = 0; } #endif diff --git a/spaghetti-monster/fsm-wdt.c b/spaghetti-monster/fsm-wdt.c index 94266c1..9e8d9af 100644 --- a/spaghetti-monster/fsm-wdt.c +++ b/spaghetti-monster/fsm-wdt.c @@ -150,6 +150,7 @@ void WDT_inner() { // (first frame of a "hold" event) else { if (ticks_since_last >= HOLD_TIMEOUT) { + ticks_since_last_event = 0; current_event |= B_HOLD; emit_current_event(0); } @@ -160,9 +161,8 @@ void WDT_inner() { else if (current_event) { // "hold" event just ended // no timeout required when releasing a long-press - // TODO? move this logic to PCINT() and simplify things here? if (current_event & B_HOLD) { - //emit_current_event(0); // should have been emitted by PCINT_inner() + //emit_current_event(ticks_since_last); // should have been emitted by PCINT_inner() empty_event_sequence(); } // end and clear event after release timeout |
