New and improved lock LED callbacks (#7215)

* New and improved lock LED callbacks

* Include stdbool

* Update documentation

* Use full function signatures and add keyboard-level example

docs/custom_quantum_functions.md

@@ -90,68 +90,110 @@ keyrecord_t record {
 
 # LED Control
 
-QMK provides methods to read the 5 LEDs defined as part of the HID spec:
+QMK provides methods to read 5 of the LEDs defined in the HID spec:
 
-* `USB_LED_NUM_LOCK`
+* Num Lock
-* `USB_LED_CAPS_LOCK`
+* Caps Lock
-* `USB_LED_SCROLL_LOCK`
+* Scroll Lock
-* `USB_LED_COMPOSE`
+* Compose
-* `USB_LED_KANA`
+* Kana
 
-These five constants correspond to the positional bits of the host LED state.
+There are two ways to get the lock LED state:
-There are two ways to get the host LED state:
 
-* by implementing `led_set_user()`
+* by implementing `bool led_update_kb(led_t led_state)` or `_user(led_t led_state)`; or
-* by calling `host_keyboard_leds()`
+* by calling `led_t host_keyboard_led_state()`
 
-## `led_set_user()`
+!> `host_keyboard_led_state()` may already reflect a new value before `led_update_user()` is called.
 
-This function will be called when the state of one of those 5 LEDs changes. It receives the LED state as a parameter.
+Two more deprecated functions exist that provide the LED state as a `uint8_t`:
-Use the `IS_LED_ON(usb_led, led_name)` and `IS_LED_OFF(usb_led, led_name)` macros to check the LED status.
 
-!> `host_keyboard_leds()` may already reflect a new value before `led_set_user()` is called.
+* `uint8_t led_set_kb(uint8_t usb_led)` and `_user(uint8_t usb_led)`
+* `uint8_t host_keyboard_leds()`
 
-### Example `led_set_user()` Implementation
+## `led_update_user()`
+
+This function will be called when the state of one of those 5 LEDs changes. It receives the LED state as a struct parameter.
+
+You must return either `true` or `false` from this function, depending on whether you want to override the keyboard-level implementation.
+
+?> Because the `led_set_*` functions return `void` instead of `bool`, they do not allow for overriding the keyboard LED control, and thus it's recommended to use `led_update_*` instead.
+
+### Example `led_update_kb()` Implementation
 
 ```c
-void led_set_user(uint8_t usb_led) {
+bool led_update_kb(led_t led_state) {
-    if (IS_LED_ON(usb_led, USB_LED_NUM_LOCK)) {
+    if(led_update_user(led_state)) {
+        if (led_state.num_lock) {
             writePinLow(B0);
         } else {
             writePinHigh(B0);
         }
-    if (IS_LED_ON(usb_led, USB_LED_CAPS_LOCK)) {
+        if (led_state.caps_lock) {
             writePinLow(B1);
         } else {
             writePinHigh(B1);
         }
-    if (IS_LED_ON(usb_led, USB_LED_SCROLL_LOCK)) {
+        if (led_state.scroll_lock) {
             writePinLow(B2);
         } else {
             writePinHigh(B2);
         }
-    if (IS_LED_ON(usb_led, USB_LED_COMPOSE)) {
+        if (led_state.compose) {
             writePinLow(B3);
         } else {
             writePinHigh(B3);
         }
-    if (IS_LED_ON(usb_led, USB_LED_KANA)) {
+        if (led_state.kana) {
             writePinLow(B4);
         } else {
             writePinHigh(B4);
         }
+        return true;
+    }
 }
 ```
 
-### `led_set_*` Function Documentation
+### Example `led_update_user()` Implementation
 
-* Keyboard/Revision: `void led_set_kb(uint8_t usb_led)`
+```c
-* Keymap: `void led_set_user(uint8_t usb_led)`
+bool led_update_user(led_t led_state) {
+    if (led_state.num_lock) {
+        writePinLow(B0);
+    } else {
+        writePinHigh(B0);
+    }
+    if (led_state.caps_lock) {
+        writePinLow(B1);
+    } else {
+        writePinHigh(B1);
+    }
+    if (led_state.scroll_lock) {
+        writePinLow(B2);
+    } else {
+        writePinHigh(B2);
+    }
+    if (led_state.compose) {
+        writePinLow(B3);
+    } else {
+        writePinHigh(B3);
+    }
+    if (led_state.kana) {
+        writePinLow(B4);
+    } else {
+        writePinHigh(B4);
+    }
+    return true;
+}
+```
 
-## `host_keyboard_leds()`
+### `led_update_*` Function Documentation
 
-Call this function to get the last received LED state. This is useful for reading the LED state outside `led_set_*`, e.g. in [`matrix_scan_user()`](#matrix-scanning-code).
+* Keyboard/Revision: `bool led_update_kb(led_t led_state)`
-For convenience, you can use the `IS_HOST_LED_ON(led_name)` and `IS_HOST_LED_OFF(led_name)` macros instead of calling and checking `host_keyboard_leds()` directly.
+* Keymap: `bool led_update_user(led_t led_state)`
+
+## `host_keyboard_led_state()`
+
+Call this function to get the last received LED state as a `led_t`. This is useful for reading the LED state outside `led_update_*`, e.g. in [`matrix_scan_user()`](#matrix-scanning-code).
 
 ## Setting Physical LED State
 

keyboards/maartenwut/wasdat/wasdat.c

@@ -33,26 +33,12 @@ void led_init_ports(void) {
     setPinOutput(B2);
 }
 
-void led_set_kb(uint8_t usb_led) {
+bool led_update_kb(led_t led_state) {
-    // put your keyboard LED indicator (ex: Caps Lock LED) toggling code here
+    if(led_update_user(led_state)) {
-
+        writePin(B0, !led_state.caps_lock);
-    if (IS_LED_ON(usb_led, USB_LED_CAPS_LOCK)) {
+        writePin(B1, !led_state.scroll_lock);
-        writePinLow(B0);
+        writePin(B2, !led_state.num_lock);
-    } else {
-        writePinHigh(B0);
     }
 
-    if (IS_LED_ON(usb_led, USB_LED_SCROLL_LOCK)) {
+    return true;
-        writePinLow(B1);
-    } else {
-        writePinHigh(B1);
-    }
-
-    if (IS_LED_ON(usb_led, USB_LED_NUM_LOCK)) {
-        writePinLow(B2);
-    } else {
-        writePinHigh(B2);
-    }
-
-    led_set_user(usb_led);
 }

quantum/quantum.c

@@ -1070,10 +1070,30 @@ void api_send_unicode(uint32_t unicode) {
 #endif
 }
 
+/** \brief Lock LED set callback - keymap/user level
+ *
+ * \deprecated Use led_update_user() instead.
+ */
 __attribute__((weak)) void led_set_user(uint8_t usb_led) {}
 
+/** \brief Lock LED set callback - keyboard level
+ *
+ * \deprecated Use led_update_kb() instead.
+ */
 __attribute__((weak)) void led_set_kb(uint8_t usb_led) { led_set_user(usb_led); }
 
+/** \brief Lock LED update callback - keymap/user level
+ *
+ * \return True if led_update_kb() should run its own code, false otherwise.
+ */
+__attribute__((weak)) bool led_update_user(led_t led_state) { return true; }
+
+/** \brief Lock LED update callback - keyboard level
+ *
+ * \return Ignored for now.
+ */
+__attribute__((weak)) bool led_update_kb(led_t led_state) { return led_update_user(led_state); }
+
 __attribute__((weak)) void led_init_ports(void) {}
 
 __attribute__((weak)) void led_set(uint8_t usb_led) {
@@ -1096,6 +1116,7 @@ __attribute__((weak)) void led_set(uint8_t usb_led) {
 #endif
 
     led_set_kb(usb_led);
+    led_update_kb((led_t) usb_led);
 }
 
 //------------------------------------------------------------------------------

quantum/quantum.h

@@ -289,5 +289,7 @@ uint16_t hex_to_keycode(uint8_t hex);
 
 void led_set_user(uint8_t usb_led);
 void led_set_kb(uint8_t usb_led);
+bool led_update_user(led_t led_state);
+bool led_update_kb(led_t led_state);
 
 void api_send_unicode(uint32_t unicode);

quantum/template/base/keyboard.c

@@ -42,9 +42,9 @@ bool process_record_kb(uint16_t keycode, keyrecord_t *record) {
     return process_record_user(keycode, record);
 }
 
-void led_set_kb(uint8_t usb_led) {
+bool led_update_kb(led_t led_state) {
     // put your keyboard LED indicator (ex: Caps Lock LED) toggling code here
 
-    led_set_user(usb_led);
+    return led_update_user(led_state);
 }
 */

quantum/template/base/keymaps/default/keymap.c

@@ -70,7 +70,7 @@ void matrix_scan_user(void) {
 
 }
 
-void led_set_user(uint8_t usb_led) {
+bool led_update_user(led_t led_state) {
-
+    return true;
 }
 */

tmk_core/common/host.c

@@ -39,6 +39,12 @@ uint8_t host_keyboard_leds(void) {
     if (!driver) return 0;
     return (*driver->keyboard_leds)();
 }
+
+led_t host_keyboard_led_state(void) {
+    if (!driver) return (led_t) {0};
+    return (led_t)((*driver->keyboard_leds)());
+}
+
 /* send report */
 void host_keyboard_send(report_keyboard_t *report) {
     if (!driver) return;

tmk_core/common/host.h

@@ -21,6 +21,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <stdbool.h>
 #include "report.h"
 #include "host_driver.h"
+#include "led.h"
 
 #define IS_LED_ON(leds, led_name) ((leds) & (1 << (led_name)))
 #define IS_LED_OFF(leds, led_name) (~(leds) & (1 << (led_name)))
@@ -41,6 +42,7 @@ host_driver_t *host_get_driver(void);
 
 /* host driver interface */
 uint8_t host_keyboard_leds(void);
+led_t   host_keyboard_led_state(void);
 void    host_keyboard_send(report_keyboard_t *report);
 void    host_mouse_send(report_mouse_t *report);
 void    host_system_send(uint16_t data);

tmk_core/common/led.h

@@ -18,6 +18,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef LED_H
 #define LED_H
 #include "stdint.h"
+#include "stdbool.h"
 
 /* FIXME: Add doxygen comments here. */
 
@@ -32,6 +33,18 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 extern "C" {
 #endif
 
+typedef union {
+    uint8_t raw;
+    struct {
+        bool num_lock    : 1;
+        bool caps_lock   : 1;
+        bool scroll_lock : 1;
+        bool compose     : 1;
+        bool kana        : 1;
+        uint8_t reserved : 3;
+    };
+} led_t;
+
 void led_set(uint8_t usb_led);
 
 void led_init_ports(void);