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(split): add support for sensors from peripheral #1841

Merged
merged 3 commits into from Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/include/zmk/keymap.h
Expand Up @@ -8,6 +8,10 @@

#include <zmk/events/position_state_changed.h>

#define ZMK_LAYER_CHILD_LEN_PLUS_ONE(node) 1 +
#define ZMK_KEYMAP_LAYERS_LEN \
(DT_FOREACH_CHILD(DT_INST(0, zmk_keymap), ZMK_LAYER_CHILD_LEN_PLUS_ONE) 0)

typedef uint32_t zmk_keymap_layers_state_t;

uint8_t zmk_keymap_layer_default();
Expand Down
6 changes: 4 additions & 2 deletions app/include/zmk/sensors.h
Expand Up @@ -24,7 +24,9 @@ struct zmk_sensor_config {
uint16_t triggers_per_rotation;
};

// This struct is also used for data transfer for splits, so any changes to the size, layout, etc
// is a breaking change for the split GATT service protocol.
struct zmk_sensor_channel_data {
enum sensor_channel channel;
struct sensor_value value;
};
enum sensor_channel channel;
} __packed;
15 changes: 14 additions & 1 deletion app/include/zmk/split/bluetooth/service.h
Expand Up @@ -6,8 +6,18 @@

#pragma once

#include <zmk/events/sensor_event.h>
#include <zmk/sensors.h>

#define ZMK_SPLIT_RUN_BEHAVIOR_DEV_LEN 9

struct sensor_event {
uint8_t sensor_index;

uint8_t channel_data_size;
struct zmk_sensor_channel_data channel_data[ZMK_SENSOR_EVENT_MAX_CHANNELS];
} __packed;

struct zmk_split_run_behavior_data {
uint8_t position;
uint8_t state;
Expand All @@ -21,4 +31,7 @@ struct zmk_split_run_behavior_payload {
} __packed;

int zmk_split_bt_position_pressed(uint8_t position);
int zmk_split_bt_position_released(uint8_t position);
int zmk_split_bt_position_released(uint8_t position);
int zmk_split_bt_sensor_triggered(uint8_t sensor_index,
const struct zmk_sensor_channel_data channel_data[],
size_t channel_data_size);
1 change: 1 addition & 0 deletions app/include/zmk/split/bluetooth/uuid.h
Expand Up @@ -16,3 +16,4 @@
#define ZMK_SPLIT_BT_SERVICE_UUID ZMK_BT_SPLIT_UUID(0x00000000)
#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)
15 changes: 8 additions & 7 deletions app/src/behaviors/behavior_sensor_rotate_common.c
Expand Up @@ -28,7 +28,7 @@ int zmk_behavior_sensor_rotate_common_accept_data(
if (value.val1 == 0) {
triggers = value.val2;
} else {
struct sensor_value remainder = data->remainder[sensor_index];
struct sensor_value remainder = data->remainder[sensor_index][event.layer];

remainder.val1 += value.val1;
remainder.val2 += value.val2;
Expand All @@ -42,15 +42,16 @@ int zmk_behavior_sensor_rotate_common_accept_data(
triggers = remainder.val1 / trigger_degrees;
remainder.val1 %= trigger_degrees;

data->remainder[sensor_index] = remainder;
data->remainder[sensor_index][event.layer] = remainder;
}

LOG_DBG(
"val1: %d, val2: %d, remainder: %d/%d triggers: %d inc keycode 0x%02X dec keycode 0x%02X",
value.val1, value.val2, data->remainder[sensor_index].val1,
data->remainder[sensor_index].val2, triggers, binding->param1, binding->param2);
value.val1, value.val2, data->remainder[sensor_index][event.layer].val1,
data->remainder[sensor_index][event.layer].val2, triggers, binding->param1,
binding->param2);

data->triggers[sensor_index] = triggers;
data->triggers[sensor_index][event.layer] = triggers;
return 0;
}

Expand All @@ -64,11 +65,11 @@ int zmk_behavior_sensor_rotate_common_process(struct zmk_behavior_binding *bindi
const int sensor_index = ZMK_SENSOR_POSITION_FROM_VIRTUAL_KEY_POSITION(event.position);

if (mode != BEHAVIOR_SENSOR_BINDING_PROCESS_MODE_TRIGGER) {
data->triggers[sensor_index] = 0;
data->triggers[sensor_index][event.layer] = 0;
return ZMK_BEHAVIOR_TRANSPARENT;
}

int triggers = data->triggers[sensor_index];
int triggers = data->triggers[sensor_index][event.layer];

struct zmk_behavior_binding triggered_binding;
if (triggers > 0) {
Expand Down
5 changes: 3 additions & 2 deletions app/src/behaviors/behavior_sensor_rotate_common.h
Expand Up @@ -6,6 +6,7 @@

#include <drivers/behavior.h>
#include <zmk/behavior.h>
#include <zmk/keymap.h>
#include <zmk/sensors.h>

struct behavior_sensor_rotate_config {
Expand All @@ -16,8 +17,8 @@ struct behavior_sensor_rotate_config {
};

struct behavior_sensor_rotate_data {
struct sensor_value remainder[ZMK_KEYMAP_SENSORS_LEN];
int triggers[ZMK_KEYMAP_SENSORS_LEN];
struct sensor_value remainder[ZMK_KEYMAP_SENSORS_LEN][ZMK_KEYMAP_LAYERS_LEN];
int triggers[ZMK_KEYMAP_SENSORS_LEN][ZMK_KEYMAP_LAYERS_LEN];
};

int zmk_behavior_sensor_rotate_common_accept_data(
Expand Down
4 changes: 0 additions & 4 deletions app/src/keymap.c
Expand Up @@ -31,10 +31,6 @@ static uint8_t _zmk_keymap_layer_default = 0;

#define DT_DRV_COMPAT zmk_keymap

#define LAYER_CHILD_LEN(node) 1 +
#define ZMK_KEYMAP_NODE DT_DRV_INST(0)
#define ZMK_KEYMAP_LAYERS_LEN (DT_INST_FOREACH_CHILD(0, LAYER_CHILD_LEN) 0)

#define BINDING_WITH_COMMA(idx, drv_inst) ZMK_KEYMAP_EXTRACT_BINDING(idx, drv_inst)

#define TRANSFORMED_LAYER(node) \
Expand Down
6 changes: 2 additions & 4 deletions app/src/sensors.c
Expand Up @@ -29,7 +29,7 @@ struct sensors_item_cfg {
{ \
.dev = DEVICE_DT_GET_OR_NULL(node), \
.trigger = {.type = SENSOR_TRIG_DATA_READY, .chan = SENSOR_CHAN_ROTATION}, \
.config = &configs[idx] \
.config = &configs[idx], .sensor_index = idx \
}
#define SENSOR_ITEM(idx, _i) _SENSOR_ITEM(idx, ZMK_KEYMAP_SENSORS_BY_IDX(idx))

Expand Down Expand Up @@ -112,7 +112,7 @@ static void zmk_sensors_trigger_handler(const struct device *dev,
int sensor_index = test_item - sensors;

if (sensor_index < 0 || sensor_index >= ARRAY_SIZE(sensors)) {
LOG_ERR("Invalid sensor item triggered our callback");
LOG_ERR("Invalid sensor item triggered our callback (%d)", sensor_index);
return;
}

Expand All @@ -127,8 +127,6 @@ static void zmk_sensors_trigger_handler(const struct device *dev,
static void zmk_sensors_init_item(uint8_t i) {
LOG_DBG("Init sensor at index %d", i);

sensors[i].sensor_index = i;

if (!sensors[i].dev) {
LOG_DBG("No local device for %d", i);
return;
Expand Down
92 changes: 78 additions & 14 deletions app/src/split/bluetooth/central.c
Expand Up @@ -21,10 +21,12 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zmk/stdlib.h>
#include <zmk/ble.h>
#include <zmk/behavior.h>
#include <zmk/sensors.h>
#include <zmk/split/bluetooth/uuid.h>
#include <zmk/split/bluetooth/service.h>
#include <zmk/event_manager.h>
#include <zmk/events/position_state_changed.h>
#include <zmk/events/sensor_event.h>

static int start_scanning(void);

Expand All @@ -41,6 +43,7 @@ struct peripheral_slot {
struct bt_conn *conn;
struct bt_gatt_discover_params discover_params;
struct bt_gatt_subscribe_params subscribe_params;
struct bt_gatt_subscribe_params sensor_subscribe_params;
struct bt_gatt_discover_params sub_discover_params;
uint16_t run_behavior_handle;
uint8_t position_state[POSITION_STATE_DATA_LEN];
Expand Down Expand Up @@ -165,6 +168,52 @@ int confirm_peripheral_slot_conn(struct bt_conn *conn) {
return 0;
}

#if ZMK_KEYMAP_HAS_SENSORS
K_MSGQ_DEFINE(peripheral_sensor_event_msgq, sizeof(struct zmk_sensor_event),
CONFIG_ZMK_SPLIT_BLE_CENTRAL_POSITION_QUEUE_SIZE, 4);

void peripheral_sensor_event_work_callback(struct k_work *work) {
struct zmk_sensor_event ev;
while (k_msgq_get(&peripheral_sensor_event_msgq, &ev, K_NO_WAIT) == 0) {
LOG_DBG("Trigger sensor change for %d", ev.sensor_index);
ZMK_EVENT_RAISE(new_zmk_sensor_event(ev));
}
}

K_WORK_DEFINE(peripheral_sensor_event_work, peripheral_sensor_event_work_callback);

static uint8_t split_central_sensor_notify_func(struct bt_conn *conn,
struct bt_gatt_subscribe_params *params,
const void *data, uint16_t length) {
if (!data) {
LOG_DBG("[UNSUBSCRIBED]");
params->value_handle = 0U;
return BT_GATT_ITER_STOP;
}

LOG_DBG("[SENSOR NOTIFICATION] data %p length %u", data, length);

if (length < offsetof(struct sensor_event, channel_data)) {
LOG_WRN("Ignoring sensor notify with insufficient data length (%d)", length);
return BT_GATT_ITER_STOP;
}

struct sensor_event sensor_event;
memcpy(&sensor_event, data, MIN(length, sizeof(sensor_event)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be a good idea to verify that length >= offsetof(struct sensor_event, channel_data). Otherwise, it would be possible to not write anything to channel_data_size, and then the array size will be a garbage value which may cause other code to read past the end of the array.

Also, we are truncating the array data if given more channels than we support, but we aren't clamping channel_data_size accordingly. This means we're protecting against writing past the end of the array, but not reading past it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a check there.

struct zmk_sensor_event ev = {
.sensor_index = sensor_event.sensor_index,
.channel_data_size = MIN(sensor_event.channel_data_size, ZMK_SENSOR_EVENT_MAX_CHANNELS),
.timestamp = k_uptime_get()};

memcpy(ev.channel_data, sensor_event.channel_data,
sizeof(struct zmk_sensor_channel_data) * sensor_event.channel_data_size);
k_msgq_put(&peripheral_sensor_event_msgq, &ev, K_NO_WAIT);
k_work_submit(&peripheral_sensor_event_work);

return BT_GATT_ITER_CONTINUE;
}
#endif /* ZMK_KEYMAP_HAS_SENSORS */

static uint8_t split_central_notify_func(struct bt_conn *conn,
struct bt_gatt_subscribe_params *params, const void *data,
uint16_t length) {
Expand Down Expand Up @@ -209,14 +258,8 @@ static uint8_t split_central_notify_func(struct bt_conn *conn,
return BT_GATT_ITER_CONTINUE;
}

static void split_central_subscribe(struct bt_conn *conn) {
struct peripheral_slot *slot = peripheral_slot_for_conn(conn);
if (slot == NULL) {
LOG_ERR("No peripheral state found for connection");
return;
}

int err = bt_gatt_subscribe(conn, &slot->subscribe_params);
static int split_central_subscribe(struct bt_conn *conn, struct bt_gatt_subscribe_params *params) {
int err = bt_gatt_subscribe(conn, params);
switch (err) {
case -EALREADY:
LOG_DBG("[ALREADY SUBSCRIBED]");
Expand All @@ -228,6 +271,8 @@ static void split_central_subscribe(struct bt_conn *conn) {
LOG_ERR("Subscribe failed (err %d)", err);
break;
}

return err;
}

static uint8_t split_central_chrc_discovery_func(struct bt_conn *conn,
Expand All @@ -250,9 +295,9 @@ static uint8_t split_central_chrc_discovery_func(struct bt_conn *conn,
}

LOG_DBG("[ATTRIBUTE] handle %u", attr->handle);
const struct bt_uuid *chrc_uuid = ((struct bt_gatt_chrc *)attr->user_data)->uuid;

if (bt_uuid_cmp(((struct bt_gatt_chrc *)attr->user_data)->uuid,
BT_UUID_DECLARE_128(ZMK_SPLIT_BT_CHAR_POSITION_STATE_UUID)) == 0) {
if (bt_uuid_cmp(chrc_uuid, BT_UUID_DECLARE_128(ZMK_SPLIT_BT_CHAR_POSITION_STATE_UUID)) == 0) {
LOG_DBG("Found position state characteristic");
slot->discover_params.uuid = NULL;
slot->discover_params.start_handle = attr->handle + 2;
Expand All @@ -263,14 +308,33 @@ static uint8_t split_central_chrc_discovery_func(struct bt_conn *conn,
slot->subscribe_params.value_handle = bt_gatt_attr_value_handle(attr);
slot->subscribe_params.notify = split_central_notify_func;
slot->subscribe_params.value = BT_GATT_CCC_NOTIFY;
split_central_subscribe(conn);
} else if (bt_uuid_cmp(((struct bt_gatt_chrc *)attr->user_data)->uuid,
BT_UUID_DECLARE_128(ZMK_SPLIT_BT_CHAR_RUN_BEHAVIOR_UUID)) == 0) {
split_central_subscribe(conn, &slot->subscribe_params);
#if ZMK_KEYMAP_HAS_SENSORS
} else if (bt_uuid_cmp(chrc_uuid, BT_UUID_DECLARE_128(ZMK_SPLIT_BT_CHAR_SENSOR_STATE_UUID)) ==
0) {
slot->discover_params.uuid = NULL;
slot->discover_params.start_handle = attr->handle + 2;
slot->discover_params.type = BT_GATT_DISCOVER_CHARACTERISTIC;

slot->sensor_subscribe_params.disc_params = &slot->sub_discover_params;
slot->sensor_subscribe_params.end_handle = slot->discover_params.end_handle;
slot->sensor_subscribe_params.value_handle = bt_gatt_attr_value_handle(attr);
slot->sensor_subscribe_params.notify = split_central_sensor_notify_func;
slot->sensor_subscribe_params.value = BT_GATT_CCC_NOTIFY;
split_central_subscribe(conn, &slot->sensor_subscribe_params);
#endif /* ZMK_KEYMAP_HAS_SENSORS */
} else if (bt_uuid_cmp(chrc_uuid, BT_UUID_DECLARE_128(ZMK_SPLIT_BT_CHAR_RUN_BEHAVIOR_UUID)) ==
0) {
LOG_DBG("Found run behavior handle");
slot->discover_params.uuid = NULL;
slot->discover_params.start_handle = attr->handle + 2;
slot->run_behavior_handle = bt_gatt_attr_value_handle(attr);
}

bool subscribed = (slot->run_behavior_handle && slot->subscribe_params.value_handle);
bool subscribed = slot->run_behavior_handle && slot->subscribe_params.value_handle;
#if ZMK_KEYMAP_HAS_SENSORS
subscribed = subscribed && slot->sensor_subscribe_params.value_handle;
#endif /* ZMK_KEYMAP_HAS_SENSORS */

return subscribed ? BT_GATT_ITER_STOP : BT_GATT_ITER_CONTINUE;
}
Expand Down