Fixed random input bug & restructured queue flushing

I had failed to check that the queue length was non-zero before checking release delay
timeouts, causing reading past the end of the `key_queue_` array an repeatedly sending
essentially random input to the host.

In the process of fixing that bug, I realized that I was also assuming that layer changes
weren't happening earlier in the queue and checking whether or not a key is a qukey when
it wasn't the head of the queue. Now we only enact a release delay when `flushKey()` is
called, and always call `setQukeyState()` when enqueuing new keys so that we can
distinguish between keys that should be immediately flushed in the primary state, and ones
that should have keypresses delayed.
pull/389/head
Michael Richters 7 years ago
parent 639f16984a
commit 4e624afb33

@ -115,9 +115,13 @@ int8_t Qukeys::lookupQukey(uint8_t key_addr) {
void Qukeys::enqueue(uint8_t key_addr) { void Qukeys::enqueue(uint8_t key_addr) {
if (key_queue_length_ == QUKEYS_QUEUE_MAX) { if (key_queue_length_ == QUKEYS_QUEUE_MAX) {
setQukeyState(key_queue_[0].addr, QUKEY_STATE_PRIMARY);
flushKey(QUKEY_STATE_PRIMARY, IS_PRESSED | WAS_PRESSED); flushKey(QUKEY_STATE_PRIMARY, IS_PRESSED | WAS_PRESSED);
flushQueue(); flushQueue();
} }
// default to alternate state to stop keys being flushed from the queue before the grace
// period timeout
setQukeyState(key_addr, QUKEY_STATE_ALTERNATE);
key_queue_[key_queue_length_].addr = key_addr; key_queue_[key_queue_length_].addr = key_addr;
key_queue_[key_queue_length_].start_time = millis(); key_queue_[key_queue_length_].start_time = millis();
key_queue_length_++; key_queue_length_++;
@ -133,7 +137,7 @@ int8_t Qukeys::searchQueue(uint8_t key_addr) {
} }
// flush a single entry from the head of the queue // flush a single entry from the head of the queue
void Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) { bool Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) {
addr::unmask(key_queue_[0].addr); addr::unmask(key_queue_[0].addr);
int8_t qukey_index = lookupQukey(key_queue_[0].addr); int8_t qukey_index = lookupQukey(key_queue_[0].addr);
bool is_qukey = (qukey_index != QUKEY_NOT_FOUND); bool is_qukey = (qukey_index != QUKEY_NOT_FOUND);
@ -142,6 +146,15 @@ void Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) {
bool is_dual_use = isDualUse(Layer.lookup(row, col)); bool is_dual_use = isDualUse(Layer.lookup(row, col));
Key keycode = Key_NoKey; Key keycode = Key_NoKey;
if (is_qukey || is_dual_use) { if (is_qukey || is_dual_use) {
if (qukey_state == QUKEY_STATE_PRIMARY &&
getQukeyState(key_queue_[0].addr) == QUKEY_STATE_ALTERNATE) {
// If there's a release delay in effect, and there's at least one key after it in
// the queue, delay this key's release event:
if (release_delay_ > 0 && key_queue_length_ > 1) {
key_queue_[0].start_time = millis() + QUKEYS_RELEASE_DELAY_OFFSET;
return false;
}
}
setQukeyState(key_queue_[0].addr, qukey_state); setQukeyState(key_queue_[0].addr, qukey_state);
if (qukey_state == QUKEY_STATE_ALTERNATE) { if (qukey_state == QUKEY_STATE_ALTERNATE) {
if (is_dual_use) { if (is_dual_use) {
@ -189,6 +202,7 @@ void Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) {
key_queue_[i] = key_queue_[i + 1]; key_queue_[i] = key_queue_[i + 1];
} }
key_queue_length_--; key_queue_length_--;
return true;
} }
// flushQueue() is called when a key that's in the key_queue is // flushQueue() is called when a key that's in the key_queue is
@ -214,7 +228,8 @@ void Qukeys::flushQueue(int8_t index) {
void Qukeys::flushQueue() { void Qukeys::flushQueue() {
// flush keys until we find a qukey: // flush keys until we find a qukey:
while (key_queue_length_ > 0 && !isQukey(key_queue_[0].addr)) { while (key_queue_length_ > 0 && !isQukey(key_queue_[0].addr)) {
flushKey(QUKEY_STATE_PRIMARY, IS_PRESSED | WAS_PRESSED); if (flushKey(QUKEY_STATE_PRIMARY, IS_PRESSED | WAS_PRESSED) == false)
break;
} }
} }
@ -289,14 +304,6 @@ Key Qukeys::keyScanHook(Key mapped_key, byte row, byte col, uint8_t key_state) {
} }
return mapped_key; return mapped_key;
} }
// If there's a release delay in effect, the released key is a qukey, and there's at
// least one key after it in the queue, delay this key's release event:
if (release_delay_ > 0 &&
isQukey(key_addr) &&
queue_index < (key_queue_length_ - 1)) {
key_queue_[queue_index].start_time = millis() + QUKEYS_RELEASE_DELAY_OFFSET;
return Key_NoKey;
}
flushQueue(queue_index); flushQueue(queue_index);
flushQueue(); flushQueue();
return Key_NoKey; return Key_NoKey;
@ -335,12 +342,13 @@ void Qukeys::preReportHook(void) {
uint16_t current_time = millis(); uint16_t current_time = millis();
if (release_delay_ > 0) { if (release_delay_ > 0 && key_queue_length_ > 0) {
int16_t diff_time = key_queue_[0].start_time - current_time; int16_t diff_time = key_queue_[0].start_time - current_time;
if (diff_time > 0) { if (diff_time > 0) {
int16_t delay_window = QUKEYS_RELEASE_DELAY_OFFSET - release_delay_; int16_t delay_window = QUKEYS_RELEASE_DELAY_OFFSET - release_delay_;
if (diff_time < delay_window) { if (diff_time < delay_window) {
flushKey(QUKEY_STATE_ALTERNATE, WAS_PRESSED); setQukeyState(key_queue_[0].addr, QUKEY_STATE_PRIMARY);
flushKey(QUKEY_STATE_PRIMARY, WAS_PRESSED);
flushQueue(); flushQueue();
} }
} }

@ -121,7 +121,7 @@ class Qukeys : public KaleidoscopePlugin {
static int8_t lookupQukey(uint8_t key_addr); static int8_t lookupQukey(uint8_t key_addr);
static void enqueue(uint8_t key_addr); static void enqueue(uint8_t key_addr);
static int8_t searchQueue(uint8_t key_addr); static int8_t searchQueue(uint8_t key_addr);
static void flushKey(bool qukey_state, uint8_t keyswitch_state); static bool flushKey(bool qukey_state, uint8_t keyswitch_state);
static void flushQueue(int8_t index); static void flushQueue(int8_t index);
static void flushQueue(void); static void flushQueue(void);
static bool isQukey(uint8_t addr); static bool isQukey(uint8_t addr);

Loading…
Cancel
Save