Skip to content

Commit

Permalink
Defer sending keyboard report to host until after matrix loop
Browse files Browse the repository at this point in the history
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop.

This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost.

Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host.

The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details.

I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues.

Exact changes include:
- In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host
- Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean
  • Loading branch information
hongaaronc authored and KarlK90 committed Oct 14, 2021
1 parent 84f54ec commit 90c6297
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
16 changes: 14 additions & 2 deletions quantum/action_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static uint8_t suppressed_mods = 0;
// TODO: pointer variable is not needed
// report_keyboard_t keyboard_report = {};
report_keyboard_t *keyboard_report = &(report_keyboard_t){};
static bool keyboard_report_dirty = false;

extern inline void add_key(uint8_t key);
extern inline void del_key(uint8_t key);
Expand Down Expand Up @@ -217,7 +218,7 @@ bool is_oneshot_enabled(void) { return keymap_config.oneshot_disable; }

/** \brief Send keyboard report
*
* FIXME: needs doc
* Flags the keyboard report to be sent at the soonest availability
*/
void send_keyboard_report(void) {
keyboard_report->mods = real_mods;
Expand Down Expand Up @@ -246,7 +247,18 @@ void send_keyboard_report(void) {
keyboard_report->mods |= weak_override_mods;
#endif

host_keyboard_send(keyboard_report);
keyboard_report_dirty = true;
}

/** \brief Send keyboard report immediate
*
* Checks if the keyboard report is different from the last one sent, and if so, sends the updated one
*/
void send_keyboard_report_immediate(void) {
if (keyboard_report_dirty) {
host_keyboard_send(keyboard_report);
keyboard_report_dirty = false;
}
}

/** \brief Get mods
Expand Down
1 change: 1 addition & 0 deletions quantum/action_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ extern "C" {
extern report_keyboard_t *keyboard_report;

void send_keyboard_report(void);
void send_keyboard_report_immediate(void);

/* key */
inline void add_key(uint8_t key) { add_key_to_report(keyboard_report, key); }
Expand Down
3 changes: 3 additions & 0 deletions quantum/keyboard.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "sendchar.h"
#include "eeconfig.h"
#include "action_layer.h"
#include "action_util.h"
#ifdef BACKLIGHT_ENABLE
# include "backlight.h"
#endif
Expand Down Expand Up @@ -425,6 +426,8 @@ void keyboard_task(void) {

MATRIX_LOOP_END:

send_keyboard_report_immediate();

#ifdef DEBUG_MATRIX_SCAN_RATE
matrix_scan_perf_task();
#endif
Expand Down

2 comments on commit 90c6297

@alex-ong
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great! Hopefully this gets picked up by the team

@KarlK90
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I will continue with this.

Please sign in to comment.