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

Add wired split support using UART communication #1117

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
11 changes: 10 additions & 1 deletion app/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ target_sources_ifdef(CONFIG_ZMK_BLE app PRIVATE src/events/battery_state_changed
target_sources_ifdef(CONFIG_USB app PRIVATE src/events/usb_conn_state_changed.c)
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_BLE_ROLE_CENTRAL)
if ((NOT CONFIG_ZMK_SPLIT) OR CONFIG_ZMK_SPLIT_BLE_ROLE_CENTRAL OR CONFIG_ZMK_SPLIT_SERIAL_ROLE_CENTRAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

See the refactor in #1314 which will massively simplify things drastically and avoid this change being needed.

target_sources(app PRIVATE src/behaviors/behavior_key_press.c)
target_sources(app PRIVATE src/behaviors/behavior_hold_tap.c)
target_sources(app PRIVATE src/behaviors/behavior_sticky_key.c)
Expand Down Expand Up @@ -75,6 +75,15 @@ endif()
if (CONFIG_ZMK_SPLIT_BLE AND CONFIG_ZMK_SPLIT_BLE_ROLE_CENTRAL)
target_sources(app PRIVATE src/split/bluetooth/central.c)
endif()
if (CONFIG_ZMK_SPLIT_SERIAL AND (NOT CONFIG_ZMK_SPLIT_SERIAL_ROLE_CENTRAL))
target_sources(app PRIVATE src/split_listener.c)
target_sources(app PRIVATE src/split/serial/service.c)
target_sources(app PRIVATE src/split/serial/common.c)
endif()
if (CONFIG_ZMK_SPLIT_SERIAL AND CONFIG_ZMK_SPLIT_SERIAL_ROLE_CENTRAL)
target_sources(app PRIVATE src/split/serial/central.c)
target_sources(app PRIVATE src/split/serial/common.c)
endif()
target_sources_ifdef(CONFIG_USB app PRIVATE src/usb.c)
target_sources_ifdef(CONFIG_ZMK_BLE app PRIVATE src/hog.c)
target_sources_ifdef(CONFIG_ZMK_RGB_UNDERGLOW app PRIVATE src/rgb_underglow.c)
Expand Down
54 changes: 50 additions & 4 deletions app/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,14 @@ config ZMK_SPLIT

if ZMK_SPLIT

choice ZMK_SPLIT_TYPE
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto on #1314

prompt "ZMK split type"

if ZMK_BLE

menuconfig ZMK_SPLIT_BLE
bool "Split keyboard support via BLE transport"
depends on ZMK_BLE
default y
select BT_USER_PHY_UPDATE

if ZMK_SPLIT_BLE
Expand Down Expand Up @@ -199,9 +203,6 @@ config ZMK_SPLIT_BLE_PERIPHERAL_POSITION_QUEUE_SIZE
int "Max number of key position state events to queue to send to the central"
default 10

config ZMK_USB
default n

config BT_MAX_PAIRED
default 1

Expand All @@ -217,6 +218,51 @@ endif
#ZMK_SPLIT_BLE
endif

# ZMK_BLE
endif

if ZMK_USB
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this really needs to be behind this condition, TBH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean we can remove this check ?
I added this check for cases where we have controllers with chips without having USB peripheral like nrf52832 chip.
If we are making an assumption that we support only USB controllers, we can remove this.
Let me know if i mis-understood the question


menuconfig ZMK_SPLIT_SERIAL
bool "Split keyboard support via Serial transport"
depends on ZMK_USB

if ZMK_SPLIT_SERIAL

choice ZMK_SPLIT_SERIAL_ROLE
prompt "ZMK split serial role"

menuconfig ZMK_SPLIT_SERIAL_ROLE_CENTRAL
bool "Central"

menuconfig ZMK_SPLIT_SERIAL_ROLE_PERIPHERAL
bool "Peripheral"

# ZMK_SPLIT_SERIAL_ROLE
endchoice

config ZMK_SPLIT_SERIAL_THREAD_STACK_SIZE
int "Serial split thread stack size"
default 1024

config ZMK_SPLIT_SERIAL_THREAD_PRIORITY
int "Serial split thread priority"
default 5

config ZMK_SPLIT_SERIAL_THREAD_QUEUE_SIZE
int "Max number of queue size for split data buffering"
default 5

# ZMK_SPLIT_SERIAL
endif

# ZMK_USB
endif

# ZMK_SPLIT_TYPE
endchoice


#ZMK_SPLIT
endif

Expand Down
26 changes: 23 additions & 3 deletions app/boards/shields/a_dux/Kconfig.defconfig
Original file line number Diff line number Diff line change
@@ -1,19 +1,39 @@
# Copyright (c) 2021 The ZMK Contributors
# SPDX-License-Identifier: MIT

if SHIELD_A_DUX_LEFT || SHIELD_A_DUX_RIGHT

config ZMK_SPLIT
default y

if SHIELD_A_DUX_LEFT

config ZMK_KEYBOARD_NAME
default "A. Dux"

if ZMK_BLE
config ZMK_SPLIT_BLE_ROLE_CENTRAL
default y
endif
Comment on lines +14 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be easier w/ the agnostic role config in #1314


if ZMK_USB
choice ZMK_SPLIT_SERIAL_ROLE
default ZMK_SPLIT_SERIAL_ROLE_CENTRAL
endchoice
endif

if SHIELD_A_DUX_LEFT || SHIELD_A_DUX_RIGHT
# SHIELD_A_DUX_LEFT
endif

config ZMK_SPLIT
default y
if SHIELD_A_DUX_RIGHT

if ZMK_USB
choice ZMK_SPLIT_SERIAL_ROLE
default ZMK_SPLIT_SERIAL_ROLE_PERIPHERAL
endchoice
endif

# SHIELD_A_DUX_RIGHT
endif

endif
37 changes: 37 additions & 0 deletions app/boards/shields/a_dux/boards/stemcell.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

/ {
chosen {
zmk,split-serial = &usart1;
};
};

&usart1 {
status = "okay";
};

&kscan0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would the pins for a kscan device need overriding for just one board?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Existing split keyboards use any one of four ports D0, D1, D2 and D3 for split communication
Ex: Lily58 uses D2 (promicro RX pin), A_Dux uses D1 (I2C SDA pin) etc

STM32 chips support half duplex UART comm only on TX pins.
So STeMCell has jumpers on back side to swap the TX/RX pins.
This depends completely on the shield which uses the pin for communication.

Other way is to use some KConfig variable to detect and swap pins.
But i have not figured out how to do it exactly using Kconfig and device tree yet,

So in the meantime, i have just juggling the pins inside the shields directly.

input-gpios =
<&pro_micro_d 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 0 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_a 0 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 16 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_a 3 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 6 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 2 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>, /* Changed since swapped pins */
<&pro_micro_a 1 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_a 2 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 7 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 4 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 1 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>, /* TODO: Use 0 if Console is needed */
<&pro_micro_d 15 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 10 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 8 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>,
<&pro_micro_d 9 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>
;
};
26 changes: 23 additions & 3 deletions app/boards/shields/lily58/Kconfig.defconfig
Original file line number Diff line number Diff line change
@@ -1,18 +1,38 @@

if SHIELD_LILY58_LEFT || SHIELD_LILY58_RIGHT

config ZMK_SPLIT
default y

if SHIELD_LILY58_LEFT

config ZMK_KEYBOARD_NAME
default "Lily58"

if ZMK_BLE
config ZMK_SPLIT_BLE_ROLE_CENTRAL
default y
endif

if ZMK_USB
choice ZMK_SPLIT_SERIAL_ROLE
default ZMK_SPLIT_SERIAL_ROLE_CENTRAL
endchoice
endif

if SHIELD_LILY58_LEFT || SHIELD_LILY58_RIGHT
#SHIELD_LILY58_LEFT
endif

config ZMK_SPLIT
default y
if SHIELD_LILY58_RIGHT

if ZMK_USB
choice ZMK_SPLIT_SERIAL_ROLE
default ZMK_SPLIT_SERIAL_ROLE_PERIPHERAL
endchoice
endif

#SHIELD_LILY58_LEFT
endif

if ZMK_DISPLAY

Expand Down
16 changes: 16 additions & 0 deletions app/boards/shields/lily58/boards/stemcell.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

/ {
chosen {
zmk,split-serial = &usart2;
};
};

&usart2 {
status = "okay";
};

2 changes: 1 addition & 1 deletion app/include/zmk/split/bluetooth/central.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
#include <zmk/behavior.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);
3 changes: 0 additions & 3 deletions app/include/zmk/split/bluetooth/service.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,3 @@ struct zmk_split_run_behavior_payload {
struct zmk_split_run_behavior_data data;
char behavior_dev[ZMK_SPLIT_RUN_BEHAVIOR_DEV_LEN];
} __packed;

int zmk_split_bt_position_pressed(uint8_t position);
int zmk_split_bt_position_released(uint8_t position);
23 changes: 23 additions & 0 deletions app/include/zmk/split/common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#pragma once

#include <zephyr/types.h>

#define SPLIT_DATA_LEN 16

#define SPLIT_TYPE_KEYPOSITION 0

typedef struct _split_data_t {
uint16_t type;
uint8_t data[SPLIT_DATA_LEN];
uint16_t crc;
} split_data_t;

int zmk_split_position_pressed(uint8_t position);

int zmk_split_position_released(uint8_t position);
22 changes: 22 additions & 0 deletions app/include/zmk/split/serial/common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2022 The ZMK Contributors
*
* SPDX-License-Identifier: MIT
*/

#pragma once

#include <zephyr/types.h>
#include <init.h>

/* Caller does/should not need to free `data`
* Data will be freed immediately after calling this callback */
typedef int (*rx_complete_t)(const uint8_t *data, size_t length);

void split_serial_async_init(rx_complete_t complete_fn);

void split_serial_async_send(uint8_t *data, size_t length);

uint8_t *alloc_split_serial_buffer(k_timeout_t timeout);

void free_split_serial_buffer(const uint8_t *data);
4 changes: 2 additions & 2 deletions app/src/activity.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ void activity_work_handler(struct k_work *work) {
} else
#endif /* IS_ENABLED(CONFIG_ZMK_SLEEP) */
if (inactive_time > MAX_IDLE_MS) {
set_state(ZMK_ACTIVITY_IDLE);
}
set_state(ZMK_ACTIVITY_IDLE);
}
}

K_WORK_DEFINE(activity_work, activity_work_handler);
Expand Down
5 changes: 3 additions & 2 deletions app/src/split/bluetooth/service.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zmk/behavior.h>
#include <zmk/matrix.h>
#include <zmk/split/bluetooth/uuid.h>
#include <zmk/split/common.h>
#include <zmk/split/bluetooth/service.h>

#define POS_STATE_LEN 16
Expand Down Expand Up @@ -141,12 +142,12 @@ int send_position_state() {
return 0;
}

int zmk_split_bt_position_pressed(uint8_t position) {
int zmk_split_position_pressed(uint8_t position) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It this is common, it shouldn't, IMHO, be duplicated for both, but should really be a reusable common core that then delegates to the transport to send the state.

WRITE_BIT(position_state[position / 8], position % 8, true);
return send_position_state();
}

int zmk_split_bt_position_released(uint8_t position) {
int zmk_split_position_released(uint8_t position) {
WRITE_BIT(position_state[position / 8], position % 8, false);
return send_position_state();
}
Expand Down
Loading