Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Core] Don't send keyboard reports that propagate no changes to the host #14065

Merged
merged 2 commits into from
Dec 14, 2021
Merged

[Core] Don't send keyboard reports that propagate no changes to the host #14065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca13013
Only send keyboard reports if there are changes to the last report
KarlK90 Aug 18, 2021
bad303d
Fix unit-tests with unnecessary keyboard reports
KarlK90 Nov 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion quantum/action_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "action_layer.h"
#include "timer.h"
#include "keycode_config.h"
#include <string.h>

extern keymap_config_t keymap_config;

Expand Down Expand Up @@ -247,7 +248,13 @@ void send_keyboard_report(void) {
keyboard_report->mods |= weak_override_mods;
#endif

host_keyboard_send(keyboard_report);
static report_keyboard_t last_report;

/* Only send the report if there are changes to propagate to the host. */
if (memcmp(keyboard_report, &last_report, sizeof(report_keyboard_t)) != 0) {
drashna marked this conversation as resolved.
Show resolved Hide resolved
memcpy(&last_report, keyboard_report, sizeof(report_keyboard_t));
host_keyboard_send(keyboard_report);
}
}

/** \brief Get mods
Expand Down
3 changes: 0 additions & 3 deletions tests/auto_shift/test_auto_shift.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ TEST_F(AutoShift, key_release_before_timeout) {
/* Release regular key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_A)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
regular_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand All @@ -65,8 +64,6 @@ TEST_F(AutoShift, key_release_after_timeout) {
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_A)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
regular_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand Down
33 changes: 11 additions & 22 deletions tests/basic/test_action_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,12 @@ TEST_F(ActionLayer, MomentaryLayerDoesNothing) {
set_keymap({layer_key});

/* Press and release MO, nothing should happen. */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.press();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand All @@ -153,8 +151,7 @@ TEST_F(ActionLayer, MomentaryLayerWithKeypress) {
set_keymap({layer_key, regular_key, KeymapKey{1, 1, 0, KC_B}});

/* Press MO. */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.press();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(1));
Expand All @@ -175,8 +172,7 @@ TEST_F(ActionLayer, MomentaryLayerWithKeypress) {
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release MO */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(0));
Expand All @@ -199,8 +195,7 @@ TEST_F(ActionLayer, ToggleLayerDoesNothing) {
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release TG. */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(1));
Expand All @@ -223,8 +218,7 @@ TEST_F(ActionLayer, ToggleLayerUpAndDown) {
EXPECT_TRUE(layer_state_is(1));
testing::Mock::VerifyAndClearExpectations(&driver);

/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
toggle_layer_1_on_layer_0.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(1));
Expand All @@ -237,8 +231,7 @@ TEST_F(ActionLayer, ToggleLayerUpAndDown) {
EXPECT_TRUE(layer_state_is(0));
testing::Mock::VerifyAndClearExpectations(&driver);

/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
toggle_layer_0_on_layer_1.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(0));
Expand All @@ -254,14 +247,13 @@ TEST_F(ActionLayer, LayerTapToggleDoesNothing) {
set_keymap({layer_key});

/* Press and release TT. */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(0);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.press();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(1));
testing::Mock::VerifyAndClearExpectations(&driver);

/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(2);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(0));
Expand All @@ -279,7 +271,6 @@ TEST_F(ActionLayer, LayerTapToggleWithKeypress) {
set_keymap({layer_key, regular_key, KeymapKey{1, 1, 0, KC_B}});

/* Press TT. */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(0);
layer_key.press();
run_one_scan_loop();
Expand All @@ -298,8 +289,7 @@ TEST_F(ActionLayer, LayerTapToggleWithKeypress) {
EXPECT_TRUE(layer_state_is(1));
testing::Mock::VerifyAndClearExpectations(&driver);

/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_key.release();
run_one_scan_loop();
EXPECT_TRUE(layer_state_is(0));
Expand All @@ -317,8 +307,7 @@ TEST_F(ActionLayer, LayerTapToggleWithToggleWithKeypress) {
set_keymap({layer_key, regular_key, KeymapKey{1, 1, 0, KC_B}});

/* Tap TT five times . */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(9);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);

layer_key.press();
run_one_scan_loop();
Expand Down
55 changes: 26 additions & 29 deletions tests/basic/test_keypress.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ TEST_F(KeyPress, CorrectKeysAreReportedWhenTwoKeysArePressed) {
TEST_F(KeyPress, LeftShiftIsReportedCorrectly) {
TestDriver driver;
auto key_a = KeymapKey(0, 0, 0, KC_A);
auto key_lsft = KeymapKey(0, 3, 0, KC_LSFT);
auto key_lsft = KeymapKey(0, 3, 0, KC_LEFT_SHIFT);

set_keymap({key_a, key_lsft});

Expand All @@ -110,8 +110,8 @@ TEST_F(KeyPress, LeftShiftIsReportedCorrectly) {

TEST_F(KeyPress, PressLeftShiftAndControl) {
TestDriver driver;
auto key_lsft = KeymapKey(0, 3, 0, KC_LSFT);
auto key_lctrl = KeymapKey(0, 5, 0, KC_LCTRL);
auto key_lsft = KeymapKey(0, 3, 0, KC_LEFT_SHIFT);
auto key_lctrl = KeymapKey(0, 5, 0, KC_LEFT_CTRL);

set_keymap({key_lctrl, key_lsft});

Expand All @@ -138,8 +138,8 @@ TEST_F(KeyPress, PressLeftShiftAndControl) {

TEST_F(KeyPress, LeftAndRightShiftCanBePressedAtTheSameTime) {
TestDriver driver;
auto key_lsft = KeymapKey(0, 3, 0, KC_LSFT);
auto key_rsft = KeymapKey(0, 4, 0, KC_RSFT);
auto key_lsft = KeymapKey(0, 3, 0, KC_LEFT_SHIFT);
auto key_rsft = KeymapKey(0, 4, 0, KC_RIGHT_SHIFT);

set_keymap({key_rsft, key_lsft});

Expand Down Expand Up @@ -175,12 +175,12 @@ TEST_F(KeyPress, RightShiftLeftControlAndCharWithTheSameKey) {
// The underlying cause is that we use only one bit to represent the right hand
// modifiers.
combo_key.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RSFT, KC_RCTRL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RSFT, KC_RCTRL, KC_O)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RIGHT_SHIFT, KC_RIGHT_CTRL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RIGHT_SHIFT, KC_RIGHT_CTRL, KC_O)));
keyboard_task();

combo_key.release();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RSFT, KC_RCTRL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_RIGHT_SHIFT, KC_RIGHT_CTRL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
keyboard_task();
}
Expand All @@ -189,18 +189,18 @@ TEST_F(KeyPress, PressPlusEqualReleaseBeforePress) {
TestDriver driver;
InSequence s;
auto key_plus = KeymapKey(0, 1, 1, KC_PLUS);
auto key_eql = KeymapKey(0, 0, 1, KC_EQL);
auto key_eql = KeymapKey(0, 0, 1, KC_EQUAL);

set_keymap({key_plus, key_eql});

key_plus.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT, KC_EQUAL)));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

key_plus.release();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand All @@ -220,13 +220,13 @@ TEST_F(KeyPress, PressPlusEqualDontReleaseBeforePress) {
TestDriver driver;
InSequence s;
auto key_plus = KeymapKey(0, 1, 1, KC_PLUS);
auto key_eql = KeymapKey(0, 0, 1, KC_EQL);
auto key_eql = KeymapKey(0, 0, 1, KC_EQUAL);

set_keymap({key_plus, key_eql});

key_plus.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT, KC_EQUAL)));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

Expand All @@ -237,14 +237,13 @@ TEST_F(KeyPress, PressPlusEqualDontReleaseBeforePress) {
testing::Mock::VerifyAndClearExpectations(&driver);

key_plus.release();
// BUG: Should really still return KC_EQL, but this is fine too
// It's also called twice for some reason
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(2);
// BUG: Should really still return KC_EQUAL, but this is fine too
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(1);
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

key_eql.release();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
}
Expand All @@ -253,12 +252,12 @@ TEST_F(KeyPress, PressEqualPlusReleaseBeforePress) {
TestDriver driver;
InSequence s;
auto key_plus = KeymapKey(0, 1, 1, KC_PLUS);
auto key_eql = KeymapKey(0, 0, 1, KC_EQL);
auto key_eql = KeymapKey(0, 0, 1, KC_EQUAL);

set_keymap({key_plus, key_eql});

key_eql.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQUAL)));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

Expand All @@ -268,13 +267,13 @@ TEST_F(KeyPress, PressEqualPlusReleaseBeforePress) {
testing::Mock::VerifyAndClearExpectations(&driver);

key_plus.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT, KC_EQL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT, KC_EQUAL)));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

key_plus.release();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LSFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand All @@ -284,12 +283,12 @@ TEST_F(KeyPress, PressEqualPlusDontReleaseBeforePress) {
TestDriver driver;
InSequence s;
auto key_plus = KeymapKey(0, 1, 1, KC_PLUS);
auto key_eql = KeymapKey(0, 0, 1, KC_EQL);
auto key_eql = KeymapKey(0, 0, 1, KC_EQUAL);

set_keymap({key_plus, key_eql});

key_eql.press();
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQL)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_EQUAL)));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

Expand All @@ -309,9 +308,7 @@ TEST_F(KeyPress, PressEqualPlusDontReleaseBeforePress) {
testing::Mock::VerifyAndClearExpectations(&driver);

key_plus.release();
// This report is not needed
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_LEFT_SHIFT)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
}
}
8 changes: 3 additions & 5 deletions tests/basic/test_one_shot_keys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,20 @@ TEST_F(OneShot, OSLWithAdditionalKeypress) {
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release OSL key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(2);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
osl_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

/* Press regular key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(regular_key.report_code))).Times(2);
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(regular_key.report_code))).Times(1);
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
regular_key.press();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release regular key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
regular_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand Down
3 changes: 1 addition & 2 deletions tests/tap_hold_configurations/permissive_hold/test_tap_hold.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,14 @@ TEST_F(PermissiveHold, tap_regular_key_while_layer_tap_key_is_held) {
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release regular key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(layer_key.report_code)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
regular_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release layer-tap-hold key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_tap_hold_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand Down
3 changes: 1 addition & 2 deletions tests/tap_hold_configurations/permissive_hold_ignore_mod_tap_interrupt/test_tap_hold.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,14 @@ TEST_F(PermissiveHold_IgnoreModTapInterrupt, tap_regular_key_while_layer_tap_key
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release regular key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport(KC_B)));
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
regular_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);

/* Release layer-tap-hold key */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport()));
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);
layer_tap_hold_key.release();
run_one_scan_loop();
testing::Mock::VerifyAndClearExpectations(&driver);
Expand Down
3 changes: 1 addition & 2 deletions tests/tap_hold_configurations/tapping_force_hold/test_action_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ TEST_F(ActionLayer, LayerTapToggleWithToggleWithKeypress) {
set_keymap({layer_key, regular_key, KeymapKey{1, 1, 0, KC_B}});

/* Tap TT five times . */
/* TODO: QMK currently sends an empty report even if nothing needs to be reported to the host! */
/* TODO: Tapping Force Hold breaks TT */
EXPECT_CALL(driver, send_keyboard_mock(KeyboardReport())).Times(10);
EXPECT_CALL(driver, send_keyboard_mock(_)).Times(0);

layer_key.press();
run_one_scan_loop();
Expand Down
Loading