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.

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

feature: LED indicators #999

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion app/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ target_sources(app PRIVATE src/behaviors/behavior_reset.c)
target_sources_ifdef(CONFIG_ZMK_EXT_POWER app PRIVATE src/behaviors/behavior_ext_power.c)
if ((NOT CONFIG_ZMK_SPLIT) OR CONFIG_ZMK_SPLIT_ROLE_CENTRAL)
target_sources(app PRIVATE src/hid.c)
target_sources_ifdef(CONFIG_ZMK_USB app PRIVATE src/usb_hid.c)

Choose a reason for hiding this comment

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

This is no longer needed after #2006 was merged since CONFIG_ZMK_USB now depends on the if condition


target_sources(app PRIVATE src/behaviors/behavior_key_press.c)
target_sources_ifdef(CONFIG_ZMK_BEHAVIOR_KEY_TOGGLE app PRIVATE src/behaviors/behavior_key_toggle.c)
target_sources(app PRIVATE src/behaviors/behavior_hold_tap.c)
Expand Down Expand Up @@ -65,6 +67,7 @@ if ((NOT CONFIG_ZMK_SPLIT) OR CONFIG_ZMK_SPLIT_ROLE_CENTRAL)
target_sources(app PRIVATE src/events/layer_state_changed.c)
target_sources(app PRIVATE src/events/modifiers_state_changed.c)
target_sources(app PRIVATE src/events/keycode_state_changed.c)
target_sources(app PRIVATE src/hid_indicators.c)

if (CONFIG_ZMK_BLE)
target_sources(app PRIVATE src/events/ble_active_profile_changed.c)
Expand All @@ -80,11 +83,12 @@ target_sources_ifdef(CONFIG_ZMK_BACKLIGHT app PRIVATE src/behaviors/behavior_bac
target_sources_ifdef(CONFIG_ZMK_BATTERY_REPORTING app PRIVATE src/events/battery_state_changed.c)
target_sources_ifdef(CONFIG_ZMK_BATTERY_REPORTING app PRIVATE src/battery.c)

target_sources(app PRIVATE src/events/hid_indicators_changed.c)

target_sources_ifdef(CONFIG_ZMK_SPLIT app PRIVATE src/events/split_peripheral_status_changed.c)
add_subdirectory(src/split)

target_sources_ifdef(CONFIG_USB_DEVICE_STACK app PRIVATE src/usb.c)
target_sources_ifdef(CONFIG_ZMK_USB app PRIVATE src/usb_hid.c)
target_sources_ifdef(CONFIG_ZMK_RGB_UNDERGLOW app PRIVATE src/rgb_underglow.c)
target_sources_ifdef(CONFIG_ZMK_BACKLIGHT app PRIVATE src/backlight.c)
target_sources(app PRIVATE src/workqueue.c)
Expand Down
1 change: 1 addition & 0 deletions app/include/zmk/ble.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ int zmk_ble_prof_prev();
int zmk_ble_prof_select(uint8_t index);

int zmk_ble_active_profile_index();
int zmk_ble_profile_index(const bt_addr_le_t *addr);
bt_addr_le_t *zmk_ble_active_profile_addr();
bool zmk_ble_active_profile_is_open();
bool zmk_ble_active_profile_is_connected();
Expand Down
16 changes: 16 additions & 0 deletions app/include/zmk/events/hid_indicators_changed.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#pragma once

#include <zmk/hid_indicators_types.h>
#include <zmk/event_manager.h>

struct zmk_hid_indicators_changed {
zmk_hid_indicators indicators;
};

ZMK_EVENT_DECLARE(zmk_hid_indicators_changed);
31 changes: 29 additions & 2 deletions app/include/zmk/hid.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@

#define COLLECTION_REPORT 0x03

#define HID_REPORT_ID_KEYBOARD 0x01
#define HID_REPORT_ID_LEDS 0x01
#define HID_REPORT_ID_CONSUMER 0x02
Comment on lines +20 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed you had added these here. I had rolled a similar tweak into #1991 with different naming conventions. Let me pull that out into a small refactor PR (where it should be anyways, and we can then build on it for this and that PR as well.


static const uint8_t zmk_hid_report_desc[] = {
HID_USAGE_PAGE(HID_USAGE_GEN_DESKTOP),
HID_USAGE(HID_USAGE_GD_KEYBOARD),
HID_COLLECTION(HID_COLLECTION_APPLICATION),
HID_REPORT_ID(0x01),
HID_REPORT_ID(HID_REPORT_ID_KEYBOARD),
HID_USAGE_PAGE(HID_USAGE_KEY),
HID_USAGE_MIN8(HID_USAGE_KEY_KEYBOARD_LEFTCONTROL),
HID_USAGE_MAX8(HID_USAGE_KEY_KEYBOARD_RIGHT_GUI),
Expand All @@ -39,6 +43,20 @@ static const uint8_t zmk_hid_report_desc[] = {
/* INPUT (Cnst,Var,Abs) */
HID_INPUT(0x03),

HID_USAGE_PAGE(HID_USAGE_LED),
HID_USAGE_MIN8(HID_USAGE_LED_NUM_LOCK),
HID_USAGE_MAX8(HID_USAGE_LED_KANA),
HID_REPORT_SIZE(0x01),
HID_REPORT_COUNT(0x05),
/* OUTPUT (Data,Var,Abs) */
HID_OUTPUT(0x02),

HID_USAGE_PAGE(HID_USAGE_LED),
HID_REPORT_SIZE(0x03),
HID_REPORT_COUNT(0x01),
/* OUTPUT (Cnst,Var,Abs) */
HID_OUTPUT(0x03),

HID_USAGE_PAGE(HID_USAGE_KEY),

#if IS_ENABLED(CONFIG_ZMK_HID_REPORT_TYPE_NKRO)
Expand Down Expand Up @@ -67,7 +85,7 @@ static const uint8_t zmk_hid_report_desc[] = {
HID_USAGE_PAGE(HID_USAGE_CONSUMER),
HID_USAGE(HID_USAGE_CONSUMER_CONSUMER_CONTROL),
HID_COLLECTION(HID_COLLECTION_APPLICATION),
HID_REPORT_ID(0x02),
HID_REPORT_ID(HID_REPORT_ID_CONSUMER),
HID_USAGE_PAGE(HID_USAGE_CONSUMER),

#if IS_ENABLED(CONFIG_ZMK_HID_CONSUMER_REPORT_USAGES_BASIC)
Expand Down Expand Up @@ -113,6 +131,15 @@ struct zmk_hid_keyboard_report {
struct zmk_hid_keyboard_report_body body;
} __packed;

struct zmk_hid_led_report_body {
uint8_t leds;
} __packed;

struct zmk_hid_led_report {
uint8_t report_id;
struct zmk_hid_led_report_body body;
} __packed;

struct zmk_hid_consumer_report_body {
#if IS_ENABLED(CONFIG_ZMK_HID_CONSUMER_REPORT_USAGES_BASIC)
uint8_t keys[CONFIG_ZMK_HID_CONSUMER_REPORT_SIZE];
Expand Down
19 changes: 19 additions & 0 deletions app/include/zmk/hid_indicators.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#pragma once

#include <zmk/endpoints_types.h>
#include <zmk/hid.h>
#include <zmk/hid_indicators_types.h>

zmk_hid_indicators zmk_hid_indicators_get_current_profile(void);
zmk_hid_indicators zmk_hid_indicators_get_profile(struct zmk_endpoint_instance endpoint);
void zmk_hid_indicators_set_profile(zmk_hid_indicators indicators,
struct zmk_endpoint_instance endpoint);

void zmk_hid_indicators_process_report(struct zmk_hid_led_report_body *report,
struct zmk_endpoint_instance endpoint);
9 changes: 9 additions & 0 deletions app/include/zmk/hid_indicators_types.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#pragma once

typedef uint8_t zmk_hid_indicators;
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually follow the convention of using a _t suffix for types like these:

Suggested change
typedef uint8_t zmk_hid_indicators;
typedef uint8_t zmk_hid_indicators_t;

5 changes: 4 additions & 1 deletion app/include/zmk/split/bluetooth/central.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

#include <zephyr/bluetooth/addr.h>
#include <zmk/behavior.h>
#include <zmk/hid_indicators_types.h>

int zmk_split_bt_invoke_behavior(uint8_t source, struct zmk_behavior_binding *binding,
struct zmk_behavior_binding_event event, bool state);
struct zmk_behavior_binding_event event, bool state);

int zmk_split_bt_update_hid_indicator(zmk_hid_indicators indicators);
1 change: 1 addition & 0 deletions app/include/zmk/split/bluetooth/uuid.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@
#define ZMK_SPLIT_BT_CHAR_POSITION_STATE_UUID ZMK_BT_SPLIT_UUID(0x00000001)
#define ZMK_SPLIT_BT_CHAR_RUN_BEHAVIOR_UUID ZMK_BT_SPLIT_UUID(0x00000002)
#define ZMK_SPLIT_BT_CHAR_SENSOR_STATE_UUID ZMK_BT_SPLIT_UUID(0x00000003)
#define ZMK_SPLIT_BT_UPDATE_HID_INDICATORS_UUID ZMK_BT_SPLIT_UUID(0x00000004)
9 changes: 9 additions & 0 deletions app/src/ble.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ int zmk_ble_clear_bonds() {

int zmk_ble_active_profile_index() { return active_profile; }

int zmk_ble_profile_index(const bt_addr_le_t *addr) {
for (int i = 0; i < ZMK_BLE_PROFILE_COUNT; i++) {
if (bt_addr_le_cmp(addr, &profiles[i].peer) == 0) {
return i;
}
}
return -ENODEV;
}

#if IS_ENABLED(CONFIG_SETTINGS)
static void ble_save_profile_work(struct k_work *work) {
settings_save_one("ble/active_profile", &active_profile, sizeof(active_profile));
Expand Down
10 changes: 10 additions & 0 deletions app/src/events/hid_indicators_changed.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#include <zephyr/kernel.h>
#include <zmk/events/hid_indicators_changed.h>

ZMK_EVENT_IMPL(zmk_hid_indicators_changed);
5 changes: 3 additions & 2 deletions app/src/hid.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <dt-bindings/zmk/modifiers.h>

static struct zmk_hid_keyboard_report keyboard_report = {
.report_id = 1, .body = {.modifiers = 0, ._reserved = 0, .keys = {0}}};
.report_id = HID_REPORT_ID_KEYBOARD, .body = {.modifiers = 0, ._reserved = 0, .keys = {0}}};

static struct zmk_hid_consumer_report consumer_report = {.report_id = 2, .body = {.keys = {0}}};
static struct zmk_hid_consumer_report consumer_report = {.report_id = HID_REPORT_ID_CONSUMER,
.body = {.keys = {0}}};

// Keep track of how often a modifier was pressed.
// Only release the modifier if the count is 0.
Expand Down
69 changes: 69 additions & 0 deletions app/src/hid_indicators.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#include <zephyr/kernel.h>
#include <zephyr/logging/log.h>

#include <zmk/ble.h>
#include <zmk/endpoints.h>
#include <zmk/hid_indicators.h>
#include <zmk/events/hid_indicators_changed.h>
#include <zmk/events/endpoint_changed.h>
#include <zmk/split/bluetooth/central.h>

LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);

static zmk_hid_indicators hid_indicators[ZMK_ENDPOINT_COUNT];

zmk_hid_indicators zmk_hid_indicators_get_current_profile(void) {
return zmk_hid_indicators_get_profile(zmk_endpoints_selected());
}

zmk_hid_indicators zmk_hid_indicators_get_profile(struct zmk_endpoint_instance endpoint) {
int profile = zmk_endpoint_instance_to_index(endpoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int profile = zmk_endpoint_instance_to_index(endpoint);
const int profile = zmk_endpoint_instance_to_index(endpoint);

return hid_indicators[profile];
}

static void raise_led_changed_event(struct k_work *_work) {
zmk_hid_indicators indicators = zmk_hid_indicators_get_current_profile();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
zmk_hid_indicators indicators = zmk_hid_indicators_get_current_profile();
const zmk_hid_indicators indicators = zmk_hid_indicators_get_current_profile();


ZMK_EVENT_RAISE(new_zmk_hid_indicators_changed(
(struct zmk_hid_indicators_changed){.indicators = indicators}));

#if IS_ENABLED(CONFIG_ZMK_SPLIT) && IS_ENABLED(CONFIG_ZMK_BLE)
zmk_split_bt_update_hid_indicator(indicators);
#endif
}

static K_WORK_DEFINE(led_changed_work, raise_led_changed_event);

void zmk_hid_indicators_set_profile(zmk_hid_indicators indicators,
struct zmk_endpoint_instance endpoint) {
int profile = zmk_endpoint_instance_to_index(endpoint);

// This write is not happening on the main thread. To prevent potential data races, every
// operation involving hid_indicators must be atomic. Currently, each function either reads
// or writes only one entry at a time, so it is safe to do these operations without a lock.
hid_indicators[profile] = indicators;

k_work_submit(&led_changed_work);
}

void zmk_hid_indicators_process_report(struct zmk_hid_led_report_body *report,
struct zmk_endpoint_instance endpoint) {
uint8_t indicators = report->leds;
zmk_hid_indicators_set_profile(indicators, endpoint);
Comment on lines +57 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uint8_t indicators = report->leds;
zmk_hid_indicators_set_profile(indicators, endpoint);
zmk_hid_indicators_t indicators = (zmk_hid_indicators_t)report->leds;
zmk_hid_indicators_set_profile(indicators, endpoint);


LOG_DBG("Update HID indicators: endpoint=%d, indicators=%x", endpoint.transport, indicators);
}

static int profile_listener(const zmk_event_t *eh) {
raise_led_changed_event(NULL);
return 0;
}

static ZMK_LISTENER(profile_listener, profile_listener);
static ZMK_SUBSCRIPTION(profile_listener, zmk_endpoint_changed);
46 changes: 43 additions & 3 deletions app/src/hog.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zephyr/bluetooth/gatt.h>

#include <zmk/ble.h>
#include <zmk/endpoints_types.h>
#include <zmk/hog.h>
#include <zmk/hid.h>
#include <zmk/hid_indicators.h>

enum {
HIDS_REMOTE_WAKE = BIT(0),
Expand Down Expand Up @@ -47,12 +49,17 @@ enum {
};

static struct hids_report input = {
.id = 0x01,
.id = HID_REPORT_ID_KEYBOARD,
.type = HIDS_INPUT,
};

static struct hids_report led_indicators = {
.id = HID_REPORT_ID_LEDS,
.type = HIDS_OUTPUT,
};

static struct hids_report consumer_input = {
.id = 0x02,
.id = HID_REPORT_ID_CONSUMER,
.type = HIDS_INPUT,
};

Expand Down Expand Up @@ -85,6 +92,31 @@ static ssize_t read_hids_input_report(struct bt_conn *conn, const struct bt_gatt
sizeof(struct zmk_hid_keyboard_report_body));
}

static ssize_t write_hids_leds_report(struct bt_conn *conn, const struct bt_gatt_attr *attr,
const void *buf, uint16_t len, uint16_t offset,
uint8_t flags) {
if (offset != 0) {
return BT_GATT_ERR(BT_ATT_ERR_INVALID_OFFSET);
}
if (len != sizeof(struct zmk_hid_led_report_body)) {
return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN);
}

struct zmk_hid_led_report_body *report = (struct zmk_hid_led_report_body *)buf;
int profile = zmk_ble_profile_index(bt_conn_get_dst(conn));
if (profile < 0) {
return BT_GATT_ERR(BT_ATT_ERR_UNLIKELY);
}

struct zmk_endpoint_instance endpoint = {.transport = ZMK_TRANSPORT_BLE,
.ble = {
.profile_index = profile,
}};
zmk_hid_indicators_process_report(report, endpoint);

return len;
}

static ssize_t read_hids_consumer_input_report(struct bt_conn *conn,
const struct bt_gatt_attr *attr, void *buf,
uint16_t len, uint16_t offset) {
Expand Down Expand Up @@ -134,6 +166,14 @@ BT_GATT_SERVICE_DEFINE(
BT_GATT_CCC(input_ccc_changed, BT_GATT_PERM_READ_ENCRYPT | BT_GATT_PERM_WRITE_ENCRYPT),
BT_GATT_DESCRIPTOR(BT_UUID_HIDS_REPORT_REF, BT_GATT_PERM_READ_ENCRYPT, read_hids_report_ref,
NULL, &input),

BT_GATT_CHARACTERISTIC(BT_UUID_HIDS_REPORT,
BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE | BT_GATT_CHRC_WRITE_WITHOUT_RESP,
BT_GATT_PERM_READ_ENCRYPT | BT_GATT_PERM_WRITE_ENCRYPT, NULL,
write_hids_leds_report, NULL),
BT_GATT_DESCRIPTOR(BT_UUID_HIDS_REPORT_REF, BT_GATT_PERM_READ_ENCRYPT, read_hids_report_ref,
NULL, &led_indicators),

BT_GATT_CHARACTERISTIC(BT_UUID_HIDS_REPORT, BT_GATT_CHRC_READ | BT_GATT_CHRC_NOTIFY,
BT_GATT_PERM_READ_ENCRYPT, read_hids_consumer_input_report, NULL, NULL),
BT_GATT_CCC(input_ccc_changed, BT_GATT_PERM_READ_ENCRYPT | BT_GATT_PERM_WRITE_ENCRYPT),
Expand Down Expand Up @@ -224,7 +264,7 @@ void send_consumer_report_callback(struct k_work *work) {
}

struct bt_gatt_notify_params notify_params = {
.attr = &hog_svc.attrs[10],
.attr = &hog_svc.attrs[12],
.data = &report,
.len = sizeof(report),
};
Expand Down
Loading
Loading