From 8812872794b5fb09e6bbbe47a58cc7dbcaca18a7 Mon Sep 17 00:00:00 2001 From: Drashna Jaelre Date: Sun, 13 Nov 2022 07:51:19 -0800 Subject: [PATCH] Only trigger encoder callbacks on primary side (#18467) Co-authored-by: zvecr --- quantum/encoder.c | 14 +- .../encoder/tests/config_mock_split_role.h | 26 ++++ .../tests/encoder_tests_split_role.cpp | 122 ++++++++++++++++++ quantum/encoder/tests/mock.c | 4 + quantum/encoder/tests/mock_split.c | 4 + quantum/encoder/tests/rules.mk | 10 ++ quantum/encoder/tests/testlist.mk | 3 +- 7 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 quantum/encoder/tests/config_mock_split_role.h create mode 100644 quantum/encoder/tests/encoder_tests_split_role.cpp diff --git a/quantum/encoder.c b/quantum/encoder.c index 1393e34868..3aee340249 100644 --- a/quantum/encoder.c +++ b/quantum/encoder.c @@ -80,6 +80,10 @@ __attribute__((weak)) bool encoder_update_kb(uint8_t index, bool clockwise) { return encoder_update_user(index, clockwise); } +__attribute__((weak)) bool should_process_encoder(void) { + return is_keyboard_master(); +} + void encoder_init(void) { #ifdef SPLIT_KEYBOARD thisHand = isLeftHand ? 0 : NUM_ENCODERS_LEFT; @@ -179,8 +183,11 @@ static bool encoder_update(uint8_t index, uint8_t state) { encoder_value[index]++; changed = true; +#ifdef SPLIT_KEYBOARD + if (should_process_encoder()) +#endif // SPLIT_KEYBOARD #ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_COUNTER_CLOCKWISE); + encoder_exec_mapping(index, ENCODER_COUNTER_CLOCKWISE); #else // ENCODER_MAP_ENABLE encoder_update_kb(index, ENCODER_COUNTER_CLOCKWISE); #endif // ENCODER_MAP_ENABLE @@ -193,8 +200,11 @@ static bool encoder_update(uint8_t index, uint8_t state) { #endif encoder_value[index]--; changed = true; +#ifdef SPLIT_KEYBOARD + if (should_process_encoder()) +#endif // SPLIT_KEYBOARD #ifdef ENCODER_MAP_ENABLE - encoder_exec_mapping(index, ENCODER_CLOCKWISE); + encoder_exec_mapping(index, ENCODER_CLOCKWISE); #else // ENCODER_MAP_ENABLE encoder_update_kb(index, ENCODER_CLOCKWISE); #endif // ENCODER_MAP_ENABLE diff --git a/quantum/encoder/tests/config_mock_split_role.h b/quantum/encoder/tests/config_mock_split_role.h new file mode 100644 index 0000000000..c80ac4d519 --- /dev/null +++ b/quantum/encoder/tests/config_mock_split_role.h @@ -0,0 +1,26 @@ +// Copyright 2022 Nick Brassel (@tzarc) +// SPDX-License-Identifier: GPL-2.0-or-later +#pragma once + +#define MATRIX_ROWS 1 +#define MATRIX_COLS 1 + +/* Here, "pins" from 0 to 31 are allowed. */ +#define ENCODERS_PAD_A \ + { 0, 2 } +#define ENCODERS_PAD_B \ + { 1, 3 } +#define ENCODERS_PAD_A_RIGHT \ + { 4, 6 } +#define ENCODERS_PAD_B_RIGHT \ + { 5, 7 } + +#ifdef __cplusplus +extern "C" { +#endif + +#include "mock_split.h" + +#ifdef __cplusplus +}; +#endif diff --git a/quantum/encoder/tests/encoder_tests_split_role.cpp b/quantum/encoder/tests/encoder_tests_split_role.cpp new file mode 100644 index 0000000000..02264067f4 --- /dev/null +++ b/quantum/encoder/tests/encoder_tests_split_role.cpp @@ -0,0 +1,122 @@ +/* Copyright 2021 Balz Guenat + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "gtest/gtest.h" +#include "gmock/gmock.h" +#include +#include +#include + +extern "C" { +#include "encoder.h" +#include "encoder/tests/mock_split.h" +} + +struct update { + int8_t index; + bool clockwise; +}; + +uint8_t num_updates = 0; + +bool isMaster; +bool isLeftHand; + +bool is_keyboard_master(void) { + return isMaster; +} + +bool encoder_update_kb(uint8_t index, bool clockwise) { + if (!isMaster) { + ADD_FAILURE() << "We shouldn't get here."; + } + num_updates++; + return true; +} + +bool setAndRead(pin_t pin, bool val) { + setPin(pin, val); + return encoder_read(); +} + +class EncoderSplitTestRole : public ::testing::Test { + protected: + void SetUp() override { + num_updates = 0; + for (int i = 0; i < 32; i++) { + pinIsInputHigh[i] = 0; + pins[i] = 0; + } + } +}; + +TEST_F(EncoderSplitTestRole, TestPrimaryLeft) { + isMaster = true; + isLeftHand = true; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(0, false); + setAndRead(1, false); + setAndRead(0, true); + setAndRead(1, true); + + EXPECT_EQ(num_updates, 1); // one update received +} + +TEST_F(EncoderSplitTestRole, TestPrimaryRight) { + isMaster = true; + isLeftHand = false; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(6, false); + setAndRead(7, false); + setAndRead(6, true); + setAndRead(7, true); + + uint8_t slave_state[32] = {0}; + encoder_state_raw(slave_state); + + EXPECT_EQ(num_updates, 1); // one update received +} + +TEST_F(EncoderSplitTestRole, TestNotPrimaryLeft) { + isMaster = false; + isLeftHand = true; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(0, false); + setAndRead(1, false); + setAndRead(0, true); + setAndRead(1, true); + + EXPECT_EQ(num_updates, 0); // zero updates received +} + +TEST_F(EncoderSplitTestRole, TestNotPrimaryRight) { + isMaster = false; + isLeftHand = false; + encoder_init(); + // send 4 pulses. with resolution 4, that's one step and we should get 1 update. + setAndRead(6, false); + setAndRead(7, false); + setAndRead(6, true); + setAndRead(7, true); + + uint8_t slave_state[32] = {0}; + encoder_state_raw(slave_state); + + EXPECT_EQ(num_updates, 0); // zero updates received +} diff --git a/quantum/encoder/tests/mock.c b/quantum/encoder/tests/mock.c index 10a00cb8f2..61f2f8294d 100644 --- a/quantum/encoder/tests/mock.c +++ b/quantum/encoder/tests/mock.c @@ -34,3 +34,7 @@ bool setPin(pin_t pin, bool val) { pins[pin] = val; return val; } + +__attribute__((weak)) bool is_keyboard_master(void) { + return true; +} diff --git a/quantum/encoder/tests/mock_split.c b/quantum/encoder/tests/mock_split.c index dd3c26d958..5cc6cd19e1 100644 --- a/quantum/encoder/tests/mock_split.c +++ b/quantum/encoder/tests/mock_split.c @@ -36,3 +36,7 @@ bool setPin(pin_t pin, bool val) { } void last_encoder_activity_trigger(void) {} + +__attribute__((weak)) bool is_keyboard_master(void) { + return true; +} diff --git a/quantum/encoder/tests/rules.mk b/quantum/encoder/tests/rules.mk index 6a2611952c..d01c1c66ee 100644 --- a/quantum/encoder/tests/rules.mk +++ b/quantum/encoder/tests/rules.mk @@ -56,3 +56,13 @@ encoder_split_no_right_SRC := \ $(QUANTUM_PATH)/encoder/tests/mock_split.c \ $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_no_right.cpp \ $(QUANTUM_PATH)/encoder.c + +encoder_split_role_DEFS := -DENCODER_TESTS -DENCODER_ENABLE -DENCODER_MOCK_SPLIT +encoder_split_role_INC := $(QUANTUM_PATH)/split_common +encoder_split_role_CONFIG := $(QUANTUM_PATH)/encoder/tests/config_mock_split_role.h + +encoder_split_role_SRC := \ + platforms/test/timer.c \ + $(QUANTUM_PATH)/encoder/tests/mock_split.c \ + $(QUANTUM_PATH)/encoder/tests/encoder_tests_split_role.cpp \ + $(QUANTUM_PATH)/encoder.c diff --git a/quantum/encoder/tests/testlist.mk b/quantum/encoder/tests/testlist.mk index 6b2fd84d96..a407f1fadd 100644 --- a/quantum/encoder/tests/testlist.mk +++ b/quantum/encoder/tests/testlist.mk @@ -4,4 +4,5 @@ TEST_LIST += \ encoder_split_left_gt_right \ encoder_split_left_lt_right \ encoder_split_no_left \ - encoder_split_no_right + encoder_split_no_right \ + encoder_split_role \