Plover HID cleanup. (#26262)

builddefs/common_features.mk

@@ -99,11 +99,6 @@ endif
 VALID_STENO_PROTOCOL_TYPES := geminipr txbolt all
 STENO_PROTOCOL ?= all
 
-ifeq ($(strip $(PLOVER_HID_ENABLE)), yes)
-    OPT_DEFS += -DPLOVER_HID_ENABLE
-    SRC += $(QUANTUM_DIR)/process_keycode/process_plover_hid.c
-endif
-
 ifeq ($(strip $(STENO_ENABLE)), yes)
     ifeq ($(filter $(STENO_PROTOCOL),$(VALID_STENO_PROTOCOL_TYPES)),)
         $(call CATASTROPHIC_ERROR,Invalid STENO_PROTOCOL,STENO_PROTOCOL="$(STENO_PROTOCOL)" is not a valid stenography protocol)

builddefs/generic_features.mk

@@ -45,6 +45,7 @@ GENERIC_FEATURES = \
     MOUSEKEY \
     MUSIC \
     OS_DETECTION \
+    PLOVER_HID \
     PROGRAMMABLE_BUTTON \
     REPEAT_KEY \
     SECURE \

quantum/plover_hid.c

@@ -0,0 +1,46 @@
+/* Copyright 2021 dnaq
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <limits.h>
+
+#include "plover_hid.h"
+#include "report.h"
+#include "host.h"
+#include "compiler_support.h"
+
+// The button index handed to plover_hid_update() is `keycode - QK_PLOVER_HID`, so the report's
+// data field must hold a bit for every keycode in the range or the bit-packing below overflows.
+STATIC_ASSERT(sizeof(((report_plover_hid_t *)0)->data) * CHAR_BIT >= (QK_PLOVER_HID_MAX - QK_PLOVER_HID + 1), "Plover HID report data too small for the QK_PLOVER_HID keycode range");
+
+static report_plover_hid_t plover_hid_report         = {.report_id = REPORT_ID_PLOVER_HID};
+static bool                plover_hid_report_updated = false;
+
+void plover_hid_update(uint8_t button, bool pressed) {
+    if (pressed) {
+        plover_hid_report.data[button / 8] |= (1 << (7 - (button % 8)));
+    } else {
+        plover_hid_report.data[button / 8] &= ~(1 << (7 - (button % 8)));
+    }
+    plover_hid_report_updated = true;
+}
+
+void plover_hid_task(void) {
+    if (!plover_hid_report_updated) {
+        return;
+    }
+    host_plover_hid_send(&plover_hid_report);
+    plover_hid_report_updated = false;
+}

quantum/plover_hid.h

@@ -16,5 +16,8 @@
 
 #pragma once
 
+#include <stdint.h>
+#include <stdbool.h>
+
 void plover_hid_update(uint8_t button, bool pressed);
 void plover_hid_task(void);

tmk_core/protocol/chibios/usb_endpoints.c

@@ -81,11 +81,7 @@ usb_endpoint_in_t usb_endpoints_in[USB_ENDPOINT_IN_COUNT] = {
 #endif
 
 #if defined(PLOVER_HID_ENABLE)
-#    if defined(USB_ENDPOINTS_ARE_REORDERABLE)
-    [USB_ENDPOINT_IN_PLOVER_HID] = QMK_USB_ENDPOINT_IN_SHARED(USB_EP_MODE_TYPE_INTR, PLOVER_HID_EPSIZE, PLOVER_HID_IN_EPNUM, PLOVER_HID_IN_CAPACITY, NULL, QMK_USB_REPORT_STORAGE_DEFAULT(PLOVER_HID_EPSIZE)),
-#    else
     [USB_ENDPOINT_IN_PLOVER_HID] = QMK_USB_ENDPOINT_IN(USB_EP_MODE_TYPE_INTR, PLOVER_HID_EPSIZE, PLOVER_HID_IN_EPNUM, PLOVER_HID_IN_CAPACITY, NULL, QMK_USB_REPORT_STORAGE_DEFAULT(PLOVER_HID_EPSIZE)),
-#    endif
 #endif
 
 #if defined(MIDI_ENABLE)
@@ -115,6 +111,10 @@ usb_endpoint_in_lut_t usb_endpoint_interface_lut[TOTAL_INTERFACES] = {
     [RAW_INTERFACE] = USB_ENDPOINT_IN_RAW,
 #endif
 
+#if defined(PLOVER_HID_ENABLE)
+    [PLOVER_HID_INTERFACE] = USB_ENDPOINT_IN_PLOVER_HID,
+#endif
+
 #if defined(MOUSE_ENABLE) && !defined(MOUSE_SHARED_EP)
     [MOUSE_INTERFACE] = USB_ENDPOINT_IN_MOUSE,
 #endif

tmk_core/protocol/chibios/usb_main.c

@@ -489,6 +489,12 @@ void send_digitizer(report_digitizer_t *report) {
 #endif
 }
 
+void send_plover_hid(report_plover_hid_t *report) {
+#ifdef PLOVER_HID_ENABLE
+    send_report(USB_ENDPOINT_IN_PLOVER_HID, report, sizeof(report_plover_hid_t));
+#endif
+}
+
 /* ---------------------------------------------------------
  *                   Console functions
  * ---------------------------------------------------------
@@ -523,28 +529,6 @@ void raw_hid_task(void) {
 
 #endif
 
-#ifdef PLOVER_HID_ENABLE
-static bool    plover_hid_report_updated                    = false;
-static uint8_t plover_hid_current_report[PLOVER_HID_EPSIZE] = {0x50};
-
-void plover_hid_update(uint8_t button, bool pressed) {
-    if (pressed) {
-        plover_hid_current_report[1 + button / 8] |= (1 << (7 - (button % 8)));
-    } else {
-        plover_hid_current_report[1 + button / 8] &= ~(1 << (7 - (button % 8)));
-    }
-    plover_hid_report_updated = true;
-}
-
-void plover_hid_task(void) {
-    if (!plover_hid_report_updated) {
-        return;
-    }
-    send_report(USB_ENDPOINT_IN_PLOVER_HID, plover_hid_current_report, sizeof(plover_hid_current_report));
-    plover_hid_report_updated = false;
-}
-#endif
-
 #ifdef MIDI_ENABLE
 
 void send_midi_packet(MIDI_EventPacket_t *event) {

tmk_core/protocol/host.c

@@ -342,6 +342,14 @@ void host_programmable_button_send(uint32_t data) {
 
 __attribute__((weak)) void send_programmable_button(report_programmable_button_t *report) {}
 
+#ifdef PLOVER_HID_ENABLE
+void host_plover_hid_send(report_plover_hid_t *report) {
+    send_plover_hid(report);
+}
+#endif
+
+__attribute__((weak)) void send_plover_hid(report_plover_hid_t *report) {}
+
 #ifdef RAW_ENABLE
 void host_raw_hid_send(uint8_t *data, uint8_t length) {
     host_driver_t *driver = host_get_active_driver();

tmk_core/protocol/host.h

@@ -44,6 +44,7 @@ void    host_mouse_send(report_mouse_t *report);
 void    host_system_send(uint16_t usage);
 void    host_consumer_send(uint16_t usage);
 void    host_programmable_button_send(uint32_t data);
+void    host_plover_hid_send(report_plover_hid_t *report);
 void    host_raw_hid_send(uint8_t *data, uint8_t length);
 
 uint16_t host_last_system_usage(void);

tmk_core/protocol/host_driver.h

@@ -37,3 +37,4 @@ typedef struct {
 void send_joystick(report_joystick_t *report);
 void send_digitizer(report_digitizer_t *report);
 void send_programmable_button(report_programmable_button_t *report);
+void send_plover_hid(report_plover_hid_t *report);

tmk_core/protocol/lufa/lufa.c

@@ -64,10 +64,6 @@
 #    include "raw_hid.h"
 #endif
 
-#ifdef PLOVER_HID_ENABLE
-#    include "plover_hid.h"
-#endif
-
 #ifdef WAIT_FOR_USB
 // TODO: Remove backwards compatibility with old define
 #    define USB_WAIT_FOR_ENUMERATION
@@ -183,28 +179,6 @@ void raw_hid_task(void) {
 }
 #endif
 
-#ifdef PLOVER_HID_ENABLE
-static bool    plover_hid_report_updated                    = false;
-static uint8_t plover_hid_current_report[PLOVER_HID_EPSIZE] = {0x50};
-
-void plover_hid_update(uint8_t button, bool pressed) {
-    if (pressed) {
-        plover_hid_current_report[1 + button / 8] |= (1 << (7 - (button % 8)));
-    } else {
-        plover_hid_current_report[1 + button / 8] &= ~(1 << (7 - (button % 8)));
-    }
-    plover_hid_report_updated = true;
-}
-
-void plover_hid_task(void) {
-    if (!plover_hid_report_updated) {
-        return;
-    }
-    send_report(PLOVER_HID_IN_EPNUM, plover_hid_current_report, sizeof(plover_hid_current_report));
-    plover_hid_report_updated = false;
-}
-#endif
-
 /*******************************************************************************
  * Console
  ******************************************************************************/
@@ -602,6 +576,12 @@ void send_digitizer(report_digitizer_t *report) {
 #endif
 }
 
+void send_plover_hid(report_plover_hid_t *report) {
+#ifdef PLOVER_HID_ENABLE
+    send_report(PLOVER_HID_IN_EPNUM, report, sizeof(report_plover_hid_t));
+#endif
+}
+
 /*******************************************************************************
  * sendchar
  ******************************************************************************/
@@ -877,10 +857,6 @@ void protocol_post_task(void) {
     MIDI_Device_USBTask(&USB_MIDI_Interface);
 #endif
 
-#ifdef PLOVER_HID_ENABLE
-    plover_hid_task();
-#endif
-
 #ifdef VIRTSER_ENABLE
     virtser_task();
     CDC_Device_USBTask(&cdc_device);

tmk_core/protocol/report.h

@@ -44,6 +44,11 @@ enum hid_report_ids {
 
 #define IS_VALID_REPORT_ID(id) ((id) >= REPORT_ID_ALL && (id) <= REPORT_ID_COUNT)
 
+// Plover HID has its own dedicated interface rather than the shared endpoint, so its report ID
+// is fixed by the protocol at 0x50 ('P') and is intentionally NOT a member of enum
+// hid_report_ids above (which only enumerates shared-endpoint reports up to REPORT_ID_COUNT).
+#define REPORT_ID_PLOVER_HID 0x50
+
 /* Mouse buttons */
 #define MOUSE_BTN_MASK(n) (1 << (n))
 enum mouse_buttons {
@@ -193,6 +198,11 @@ typedef struct {
     uint32_t usage;
 } PACKED report_programmable_button_t;
 
+typedef struct {
+    uint8_t report_id; // REPORT_ID_PLOVER_HID
+    uint8_t data[8];
+} PACKED report_plover_hid_t;
+
 #ifdef MOUSE_EXTENDED_REPORT
 #    define MOUSE_REPORT_XY_MIN (INT16_MIN + 1)
 #    define MOUSE_REPORT_XY_MAX INT16_MAX

tmk_core/protocol/usb_descriptor.c

@@ -40,6 +40,7 @@
 #include "report.h"
 #include "usb_descriptor.h"
 #include "usb_descriptor_common.h"
+#include "compiler_support.h"
 
 #ifdef JOYSTICK_ENABLE
 #    include "joystick.h"
@@ -494,6 +495,10 @@ const USB_Descriptor_HIDReport_Datatype_t PROGMEM PloverReport[] = {
         HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_VARIABLE | HID_IOF_ABSOLUTE),
     HID_RI_END_COLLECTION(0),
 };
+
+// The Plover HID report is sent with sizeof(report_plover_hid_t), but the endpoint and descriptor
+// are sized with PLOVER_HID_EPSIZE; they must match or reports get truncated/padded.
+STATIC_ASSERT(sizeof(report_plover_hid_t) == PLOVER_HID_EPSIZE, "report_plover_hid_t size must match PLOVER_HID_EPSIZE");
 #endif
 
 #ifdef CONSOLE_ENABLE