From 78d325e1d470d222038c5f790163541b692e526d Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Mon, 10 Sep 2018 23:12:25 +0800 Subject: [PATCH 1/8] fix: #120 bad keyreport issue --- src/HID-APIs/DefaultKeyboardAPI.hpp | 74 ++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 06a1cabf..05ff9a65 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -25,7 +25,7 @@ THE SOFTWARE. #pragma once -size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) +size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) { // It's a modifier key if(k >= KEY_LEFT_CTRL && k <= KEY_RIGHT_GUI) @@ -42,26 +42,56 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) } // Its a normal key else{ - // Add k to the key report only if it's not already present - // and if there is an empty slot. Remove the first available key. - for (uint8_t i = 0; i < sizeof(_keyReport.keycodes); i++) - { - auto key = _keyReport.keycodes[i]; - - // Is key already in the list or did we found an empty slot? - if (s && (key == uint8_t(k) || key == KEY_RESERVED)) { - _keyReport.keycodes[i] = k; - return 1; + // my code starts + + // get size of keycodes during compile time + const uint8_t keycodesSize = sizeof(_keyReport.keycodes); + + + // if we are removing keys + if (!s) { + // iterate through the keycodes + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + // if target key is found + if (key == k) { + // remove target and exit + _keyReport.keycodes[i] = KEY_RESERVED; + return 1; + } } - - // Test the key report to see if k is present. Clear it if it exists. - if (!s && (key == k)) { - _keyReport.keycodes[i] = KEY_RESERVED; - return 1; + } // if we are adding an element to keycodes + else { + // iterate through the keycodes + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + // if target key is found + if (key == uint8_t(k)) { + // do nothing and exit + return 1; + } + } + // iterate through the keycodes again, this only happens if no existing + // keycodes matches k + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + // if first instance of empty slot is found + if (key == KEY_RESERVED) { + // change empty slot to k and exit + _keyReport.keycodes[i] = k; + return 1; + } } } + + + + // my code ends } - + // No empty/pressed key was found return 0; } @@ -104,7 +134,7 @@ size_t DefaultKeyboardAPI::press(ConsumerKeycode k) } -size_t DefaultKeyboardAPI::release(ConsumerKeycode k) +size_t DefaultKeyboardAPI::release(ConsumerKeycode k) { // Release key and send report to host auto ret = remove(k); @@ -115,14 +145,14 @@ size_t DefaultKeyboardAPI::release(ConsumerKeycode k) } -size_t DefaultKeyboardAPI::add(ConsumerKeycode k) +size_t DefaultKeyboardAPI::add(ConsumerKeycode k) { // No 2 byte keys are supported if(k > 0xFF){ setWriteError(); return 0; } - + // Place the key inside the reserved keyreport position. // This does not work on Windows. _keyReport.reserved = k; @@ -130,13 +160,13 @@ size_t DefaultKeyboardAPI::add(ConsumerKeycode k) } -size_t DefaultKeyboardAPI::remove(ConsumerKeycode k) +size_t DefaultKeyboardAPI::remove(ConsumerKeycode k) { // No 2 byte keys are supported if(k > 0xFF){ return 0; } - + // Always release the key, to make it simpler releasing a consumer key // without releasing all other normal keyboard keys. _keyReport.reserved = HID_CONSUMER_UNASSIGNED; From 615df66196299cf45a0f1f61fb406aaacb9387a2 Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Tue, 11 Sep 2018 13:57:04 +0800 Subject: [PATCH 2/8] add: reordered code --- src/HID-APIs/DefaultKeyboardAPI.hpp | 36 +++++++++++------------------ 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 05ff9a65..b92ab909 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -42,27 +42,11 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) } // Its a normal key else{ - // my code starts - // get size of keycodes during compile time const uint8_t keycodesSize = sizeof(_keyReport.keycodes); - - // if we are removing keys - if (!s) { - // iterate through the keycodes - for (uint8_t i = 0; i < keycodesSize; i++) - { - auto key = _keyReport.keycodes[i]; - // if target key is found - if (key == k) { - // remove target and exit - _keyReport.keycodes[i] = KEY_RESERVED; - return 1; - } - } - } // if we are adding an element to keycodes - else { + // if we are adding an element to keycodes + if (s){ // iterate through the keycodes for (uint8_t i = 0; i < keycodesSize; i++) { @@ -85,11 +69,19 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) return 1; } } + } else { // we are removing k from keycodes + // iterate through the keycodes + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + // if target key is found + if (key == k) { + // remove target and exit + _keyReport.keycodes[i] = KEY_RESERVED; + return 1; + } + } } - - - - // my code ends } // No empty/pressed key was found From e2145e8017de4aff0edd0aa789fc05b76158e295 Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Wed, 12 Sep 2018 01:06:20 +0800 Subject: [PATCH 3/8] change: releasing a key reorders the keycodes list instead --- src/HID-APIs/DefaultKeyboardAPI.hpp | 56 +++++++++++++++++++---------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index b92ab909..1a40459b 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -51,39 +51,57 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; - // if target key is found - if (key == uint8_t(k)) { - // do nothing and exit - return 1; - } - } - // iterate through the keycodes again, this only happens if no existing - // keycodes matches k - for (uint8_t i = 0; i < keycodesSize; i++) - { - auto key = _keyReport.keycodes[i]; - // if first instance of empty slot is found - if (key == KEY_RESERVED) { - // change empty slot to k and exit + + // write to keycodes on first instance of k or null in keycodes + if (key == uint8_t(k) || key == KEY_RESERVED) { _keyReport.keycodes[i] = k; return 1; } } } else { // we are removing k from keycodes + // this will replace the first instance of k with the last entry in the pseudo-linkedlist + + uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes + // iterate through the keycodes for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; + // if target key is found - if (key == k) { - // remove target and exit - _keyReport.keycodes[i] = KEY_RESERVED; - return 1; + if (key == uint8_t(k)) { + // record target key index to keyIndex; + keyIndex = i; + break; } } + + // if the target key is found (AKA if there is a key to remove) + if (keyIndex != 255) + { + // iterate through the keycodes from the back + KeyboardKeycode lastElement; + for (uint8_t i = keycodesSize; i > 0; --i) + { + // slot is occupied + if (_keyReport.keycodes[i] != KEY_RESERVED) + { + _keyReport.keycodes[i] = KEY_RESERVED; // remove last element + + // target key is the last non-null element in the pseudo-linkedlist + if (i == keyIndex) + { + return 1; // no further operations needed, exit now + } + lastElement = _keyReport.keycodes[i]; // copy current element to temp + break; + } + } + _keyReport.keycodes[keyIndex] = lastElement; // replace k with last element + return 1; + } } } - // No empty/pressed key was found return 0; } From b733010ed0507d1ee249bb9168c091a0709145cd Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Wed, 12 Sep 2018 04:49:04 +0800 Subject: [PATCH 4/8] refactor --- src/HID-APIs/DefaultKeyboardAPI.hpp | 55 +++++++++++------------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 1a40459b..98d64b82 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -60,45 +60,32 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) } } else { // we are removing k from keycodes // this will replace the first instance of k with the last entry in the pseudo-linkedlist - - uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - - // iterate through the keycodes - for (uint8_t i = 0; i < keycodesSize; i++) - { - auto key = _keyReport.keycodes[i]; - - // if target key is found - if (key == uint8_t(k)) { - // record target key index to keyIndex; - keyIndex = i; - break; - } - } - - // if the target key is found (AKA if there is a key to remove) - if (keyIndex != 255) - { - // iterate through the keycodes from the back + // if list is not empty + if (_keyReport.keycodes[0] != KEY_RESERVED) { + uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes KeyboardKeycode lastElement; - for (uint8_t i = keycodesSize; i > 0; --i) + for (uint8_t i = 0; i < keycodesSize; i++) { - // slot is occupied - if (_keyReport.keycodes[i] != KEY_RESERVED) - { - _keyReport.keycodes[i] = KEY_RESERVED; // remove last element - - // target key is the last non-null element in the pseudo-linkedlist - if (i == keyIndex) - { - return 1; // no further operations needed, exit now - } - lastElement = _keyReport.keycodes[i]; // copy current element to temp + auto key = _keyReport.keycodes[i]; + + if (key == uint8_t(k)) { + keyIndex = i; + continue; + } + + // if the next element is null or if this is the last element + if ((i < keycodesSize-1 && _keyReport.keycodes[i+1] == KEY_RESERVED) || i == keycodesSize-1) { + lastElement = key; // set temp key to last filled slot + _keyReport.keycodes[i] = KEY_RESERVED; // clear last filled slot break; } } - _keyReport.keycodes[keyIndex] = lastElement; // replace k with last element - return 1; + + // if target key found + if (keyIndex != 255 && k != lastElement) { + _keyReport.keycodes[keyIndex] = lastElement; // replace target key + return 1; + } } } } From 9660e2fa1700f4c2260905810659e136d164a085 Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Wed, 12 Sep 2018 16:32:05 +0800 Subject: [PATCH 5/8] bugfix: releasing a non-existant key removes last element in keycodes --- src/HID-APIs/DefaultKeyboardAPI.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 98d64b82..43cca5b9 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -63,7 +63,7 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) // if list is not empty if (_keyReport.keycodes[0] != KEY_RESERVED) { uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - KeyboardKeycode lastElement; + uint8_t lastIndex; for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; @@ -75,15 +75,15 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) // if the next element is null or if this is the last element if ((i < keycodesSize-1 && _keyReport.keycodes[i+1] == KEY_RESERVED) || i == keycodesSize-1) { - lastElement = key; // set temp key to last filled slot - _keyReport.keycodes[i] = KEY_RESERVED; // clear last filled slot + lastIndex = i; // set temp key to last filled slot index break; } } - // if target key found - if (keyIndex != 255 && k != lastElement) { - _keyReport.keycodes[keyIndex] = lastElement; // replace target key + // replace target key with last element, replace last element with null + if (keyIndex != 255) { + _keyReport.keycodes[keyIndex] = _keyReport.keycodes[lastIndex]; // replace target key + _keyReport.keycodes[lastIndex] = KEY_RESERVED; return 1; } } From 5f4cad98eee891fbe8d9bf1adae24a8eab3b8f16 Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Thu, 13 Sep 2018 04:57:01 +0800 Subject: [PATCH 6/8] Revert "bugfix: releasing a non-existant key removes last element in keycodes" This reverts commit 9660e2fa1700f4c2260905810659e136d164a085. --- src/HID-APIs/DefaultKeyboardAPI.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 43cca5b9..98d64b82 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -63,7 +63,7 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) // if list is not empty if (_keyReport.keycodes[0] != KEY_RESERVED) { uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - uint8_t lastIndex; + KeyboardKeycode lastElement; for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; @@ -75,15 +75,15 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) // if the next element is null or if this is the last element if ((i < keycodesSize-1 && _keyReport.keycodes[i+1] == KEY_RESERVED) || i == keycodesSize-1) { - lastIndex = i; // set temp key to last filled slot index + lastElement = key; // set temp key to last filled slot + _keyReport.keycodes[i] = KEY_RESERVED; // clear last filled slot break; } } - // replace target key with last element, replace last element with null - if (keyIndex != 255) { - _keyReport.keycodes[keyIndex] = _keyReport.keycodes[lastIndex]; // replace target key - _keyReport.keycodes[lastIndex] = KEY_RESERVED; + // if target key found + if (keyIndex != 255 && k != lastElement) { + _keyReport.keycodes[keyIndex] = lastElement; // replace target key return 1; } } From e659e13dddf0426ccbbf6ee76835268a325adfae Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Thu, 13 Sep 2018 04:57:09 +0800 Subject: [PATCH 7/8] Revert "refactor" This reverts commit b733010ed0507d1ee249bb9168c091a0709145cd. --- src/HID-APIs/DefaultKeyboardAPI.hpp | 55 ++++++++++++++++++----------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 98d64b82..1a40459b 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -60,32 +60,45 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) } } else { // we are removing k from keycodes // this will replace the first instance of k with the last entry in the pseudo-linkedlist - // if list is not empty - if (_keyReport.keycodes[0] != KEY_RESERVED) { - uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - KeyboardKeycode lastElement; - for (uint8_t i = 0; i < keycodesSize; i++) - { - auto key = _keyReport.keycodes[i]; - if (key == uint8_t(k)) { - keyIndex = i; - continue; - } + uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - // if the next element is null or if this is the last element - if ((i < keycodesSize-1 && _keyReport.keycodes[i+1] == KEY_RESERVED) || i == keycodesSize-1) { - lastElement = key; // set temp key to last filled slot - _keyReport.keycodes[i] = KEY_RESERVED; // clear last filled slot - break; - } + // iterate through the keycodes + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + + // if target key is found + if (key == uint8_t(k)) { + // record target key index to keyIndex; + keyIndex = i; + break; } + } - // if target key found - if (keyIndex != 255 && k != lastElement) { - _keyReport.keycodes[keyIndex] = lastElement; // replace target key - return 1; + // if the target key is found (AKA if there is a key to remove) + if (keyIndex != 255) + { + // iterate through the keycodes from the back + KeyboardKeycode lastElement; + for (uint8_t i = keycodesSize; i > 0; --i) + { + // slot is occupied + if (_keyReport.keycodes[i] != KEY_RESERVED) + { + _keyReport.keycodes[i] = KEY_RESERVED; // remove last element + + // target key is the last non-null element in the pseudo-linkedlist + if (i == keyIndex) + { + return 1; // no further operations needed, exit now + } + lastElement = _keyReport.keycodes[i]; // copy current element to temp + break; + } } + _keyReport.keycodes[keyIndex] = lastElement; // replace k with last element + return 1; } } } From cf22f94a81a57a51385bb3a81ca68d103c72bb50 Mon Sep 17 00:00:00 2001 From: Blahlicus Date: Thu, 13 Sep 2018 04:57:20 +0800 Subject: [PATCH 8/8] Revert "change: releasing a key reorders the keycodes list instead" This reverts commit e2145e8017de4aff0edd0aa789fc05b76158e295. --- src/HID-APIs/DefaultKeyboardAPI.hpp | 56 ++++++++++------------------- 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/src/HID-APIs/DefaultKeyboardAPI.hpp b/src/HID-APIs/DefaultKeyboardAPI.hpp index 1a40459b..b92ab909 100644 --- a/src/HID-APIs/DefaultKeyboardAPI.hpp +++ b/src/HID-APIs/DefaultKeyboardAPI.hpp @@ -51,57 +51,39 @@ size_t DefaultKeyboardAPI::set(KeyboardKeycode k, bool s) for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; - - // write to keycodes on first instance of k or null in keycodes - if (key == uint8_t(k) || key == KEY_RESERVED) { + // if target key is found + if (key == uint8_t(k)) { + // do nothing and exit + return 1; + } + } + // iterate through the keycodes again, this only happens if no existing + // keycodes matches k + for (uint8_t i = 0; i < keycodesSize; i++) + { + auto key = _keyReport.keycodes[i]; + // if first instance of empty slot is found + if (key == KEY_RESERVED) { + // change empty slot to k and exit _keyReport.keycodes[i] = k; return 1; } } } else { // we are removing k from keycodes - // this will replace the first instance of k with the last entry in the pseudo-linkedlist - - uint8_t keyIndex = 255; // if keyIndex == 255 then k does not exist in keycodes - // iterate through the keycodes for (uint8_t i = 0; i < keycodesSize; i++) { auto key = _keyReport.keycodes[i]; - // if target key is found - if (key == uint8_t(k)) { - // record target key index to keyIndex; - keyIndex = i; - break; - } - } - - // if the target key is found (AKA if there is a key to remove) - if (keyIndex != 255) - { - // iterate through the keycodes from the back - KeyboardKeycode lastElement; - for (uint8_t i = keycodesSize; i > 0; --i) - { - // slot is occupied - if (_keyReport.keycodes[i] != KEY_RESERVED) - { - _keyReport.keycodes[i] = KEY_RESERVED; // remove last element - - // target key is the last non-null element in the pseudo-linkedlist - if (i == keyIndex) - { - return 1; // no further operations needed, exit now - } - lastElement = _keyReport.keycodes[i]; // copy current element to temp - break; - } + if (key == k) { + // remove target and exit + _keyReport.keycodes[i] = KEY_RESERVED; + return 1; } - _keyReport.keycodes[keyIndex] = lastElement; // replace k with last element - return 1; } } } + // No empty/pressed key was found return 0; }