Add wired split support using UART communication #1117

megamind4089 · 2022-02-04T01:47:52Z

Initial wired split support. Raised this PR to get feedback on initial changes and design

Changes:

Updated Kconfig to allow configuring split communication type using Kconfig choice
Type can either be BLE or SERIAL for now
BLE is selected as default when board supports BLE
Serial is selected when there is only USB support
Used choice for central/peripheral instead of using single flag. This is experimental. Want to get feedback on this
Use chosen node to select UART dev for serial split driver
Serial central & service behaves similarly to the current ble split comm
Only addition is the CRC to split position packet which i added as a defensive check
Please look at the a_dux Kconfig.defconfig changes and let me know whether it makes sense to use like this.

Note:

I tried to make changes to be not intrusive, i.e does not change/affect existing design to make PR merge smoother
Ignore the west.yml change. Cherrypicked one PR from zephyr main for STM32 single wire UART and will revert it
Ignore the stemcell.overlay file. I added this to give an idea about the change needed. Will remove this later.
Will update other split keyboard's defconfig once the design seems acceptable

SteMCell board config:
https://github.com/megamind4089/zmk-config/tree/main/config/boards/arm/stemcell

Cherrypicked zephyr PR:
zephyrproject-rtos/zephyr#40525

petejohanson

First pass w/ some initial questions. Thanks for the work on this!

petejohanson · 2022-02-06T04:42:24Z

app/Kconfig

+# ZMK_BLE
+endif
+
+if ZMK_USB


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

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

petejohanson · 2022-02-06T04:43:56Z

app/boards/shields/a_dux/Kconfig.defconfig

@@ -1,19 +1,41 @@
 # Copyright (c) 2021 The ZMK Contributors
 # SPDX-License-Identifier: MIT

+if SHIELD_A_DUX_LEFT || SHIELD_A_DUX_RIGHT
+
+if ZMK_BLE || ZMK_USB


Why isn't this just defaulted on? Why behind this check?

Yes, does not makes much sense. No idea why i added this.
Will remove this check.

petejohanson · 2022-02-06T04:45:17Z

app/boards/shields/a_dux/boards/stemcell.overlay

+	status = "okay";
+};
+
+&kscan0 {


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

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.

petejohanson · 2022-02-06T04:47:31Z

app/src/split/bluetooth/service.c

@@ -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) {


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.

petejohanson · 2022-02-06T04:48:44Z

app/src/split/serial/central.c

+#include <zmk/events/position_state_changed.h>
+#include <zmk/matrix.h>
+
+#if DT_HAS_CHOSEN(zmk_split_serial)


If we don't have a chosen, we should just #error instead of wrapping this all in a conditional.

petejohanson · 2022-02-06T04:49:13Z

app/src/split/serial/central.c

+#define CONFIG_ZMK_SPLIT_SERIAL_CENTRAL_POSITION_QUEUE_SIZE 5
+
+#define CONFIG_SERIAL_THREAD_STACK_SIZE 128
+#define CONFIG_SERIAL_THREAD_PRIORITY 5


Why not actual Kconfig settings?

petejohanson · 2022-02-06T04:53:25Z

app/src/split/serial/central.c

+    uart_ready = 1;
+    LOG_DBG("UART device:%s ready", serial_dev->name);
+
+    while(true) {


Do we really need to hard loop here? Can't we just enable the RX here, and then in the notify callback, do more? Not familiar w/ the UART API in detail, but this seems off.

petejohanson · 2022-02-06T04:54:02Z

app/src/split/serial/service.c

+
+#include <zmk/matrix.h>
+
+#if DT_HAS_CHOSEN(zmk_split_serial)


Ditto, should #error if no chosen node, not wrap all this in the #if.

petejohanson · 2022-02-06T04:57:16Z

app/src/split/serial/service.c

+    }
+
+    for (int i=0; i < len; i++) {
+        uart_poll_out(serial_dev, data[i]);


Why not uart_tx for the whole buffer?

I have done the TX and RX part first using sync mode and then updated RX to async using DMA.
Will update TX also to use same apis

petejohanson · 2022-02-06T04:58:17Z

app/west.yml

+      remote: megamind4089
+      revision: v2.5.0+zmk-fixes-stm32f4


Anything here actually relevant to this work that this is needed?

No, please ignore this file. Will remove this before final cleanup.
Just added here to show that i had to cherry-pick this PR to make it work in half duplex mode.
zephyrproject-rtos/zephyr#40525

Refactor the code to common code

petejohanson · 2022-05-23T03:30:57Z

app/CMakeLists.txt

@@ -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)


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

petejohanson · 2022-05-23T03:45:43Z

app/Kconfig

@@ -155,10 +155,14 @@ config ZMK_SPLIT

 if ZMK_SPLIT

+choice ZMK_SPLIT_TYPE


Ditto on #1314

petejohanson · 2022-05-23T03:46:46Z

app/boards/shields/a_dux/Kconfig.defconfig

+if ZMK_BLE
 config ZMK_SPLIT_BLE_ROLE_CENTRAL
 	default y
+endif


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

marcoster · 2022-06-04T18:32:23Z

Should really the Async-API of zephyr's uart be used here?
As far as I've seen the API is only supported by STM32, NRF and sam0.
Or is the API the future 'standard' for uart in zephyr.

At least for now I'm not able to 'simply' use this on a Pi Pico... maybe I'll looker deeper on how the async api has to be implemented or find a workaround for this.

Maybe at least a Kconfig error should be generated if SERIAL_SUPPORT_ASYNC of a board is set to n?

petejohanson · 2022-06-04T18:50:17Z

I do think using the async API is probably preferred, which means we should work on the RP2040 UART driver for Zephyr to add support for it.

marcoster · 2022-06-07T14:01:31Z

Yeah, thought so... maybe I'll find some time soon. So for now I'll have to stick with KMK for my KB2040 / Sofle combo ;-)

mbrgm · 2023-03-01T18:53:03Z

@marcoster Can this be used for two Nice!Nanos in its current state?

@petejohanson What would have to be done to move this forward?

nov1n · 2023-03-23T18:57:20Z

I would also be interested in the answers to @mbrgm 's questions. Also, I wonder what the main benefits are of wiring the two halves. Does it reduce power consumption or improve latency?

And would it be possible to connect with Bluetooth to the host, and use uart between halves, or would this only be supported when connected over USB?

marcoster · 2023-04-13T16:20:39Z

@mbrgm Sorry, never wrote an answer - as I don't have any nice!nanos around I don't really know, but since the nRF52840 in Zephyr supports async uart it normally should work.
@nov1n Well I kinda don't need wireless split and some quite cheap and available controller-options (RP2040 boards i.e.) don't have any wireless capabilities.
If done right I guess it should be possible to connect to the host per BLE.

jesseleite · 2023-09-04T15:33:32Z

Am I right to assume that UART support in Zephyr would allow for wired split communication between peripheral and central over TRRS? 🙏

petejohanson · 2023-09-04T16:53:47Z

Am I right to assume that UART support in Zephyr would allow for wired split communication between peripheral and central over TRRS? 🙏

This PR definitely needs a rebase into latest ZMK main, but yes*.

this assumes a proper hardware uart, not any single wire bit-banging approach, etc as is often used in many three wire TRRS splits.

M1cha · 2023-09-20T11:57:12Z

I have a corne cherry with sparkfun_pro_micro_rp2040 and will make this work with the latest main branch within the next few days. Shall I open a new PR once done?

petejohanson · 2023-09-21T18:59:21Z

I have a corne cherry with sparkfun_pro_micro_rp2040 and will make this work with the latest main branch within the next few days. Shall I open a new PR once done?

Sure! See my previous note about the current state needing a working UART device (in terms of Zephyr device w/ associated UART driver) so any single wire work will need to either:

Adapt this code to use a higher level abstraction than the Zephyr UART driver one, OR
Create a UART driver that works for single wire hardware, maybe using PIO, etc. Doing this likely requires experimental Zephyr version that recently added PIO driver support.

jonathanfv · 2024-04-06T02:41:13Z

Hey there, I was looking at implementing "just this" (wired connection between halves, likely using TRRS and UART TTL) on the Glove80 (dual nRF52840, harware UART). Any way I can help?

thomasbaart · 2024-04-09T13:00:11Z

Am also interested in seeing this happen. If there's a contributor who wants to pick this up but needs hardware to do so, please post a comment and I'll sponsor the needful :)

Update Apr 12 2024, my Dilemma is wired and I've learned that ZMK does not currently support wired splits. Without wanting to get new microcontrollers, I've decided to either wait this out or pivot to a Sugar Glider or Reviung 41. Currently following [lesshonor's Mechhwild ZMK work](https://github.com/lesshonor/mechwild-zmk-keyboards/tree/cirque) and [PR 1117 of ZMK](zmkfirmware/zmk#1117)

megamind4089 force-pushed the wired_support branch from 9072f10 to a92c968 Compare February 4, 2022 04:00

petejohanson requested changes Feb 6, 2022

View reviewed changes

megamind4089 force-pushed the wired_support branch from 71bbdc9 to 8c8be22 Compare February 6, 2022 08:23

megamind4089 added 6 commits February 16, 2022 11:56

Add wired split support using UART communication

e92a986

Address comments

f357aed

Clang formatter changes

ace8cc3

Update TX to async and fix RX thread support

528fc96

Refactor the code to common code

nitpicks

8e6b5f5

add support for lily58

5a8744f

megamind4089 force-pushed the wired_support branch from 1b27605 to 5a8744f Compare February 16, 2022 03:56

cgoates mentioned this pull request Feb 20, 2022

Wired split keyboard support #1110

Open

petejohanson mentioned this pull request May 23, 2022

Split Kconfig/CMake refactor #1314

Merged

petejohanson reviewed May 23, 2022

View reviewed changes

stevexyz mentioned this pull request Aug 8, 2023

rp2040 (raspberry pi pico) support tracking issue #1085

Open

M1cha mentioned this pull request Oct 5, 2023

Split serial #1954

Draft

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wired split support using UART communication #1117

Add wired split support using UART communication #1117

megamind4089 commented Feb 4, 2022 •

edited

Loading

petejohanson left a comment

petejohanson Feb 6, 2022

megamind4089 Feb 6, 2022

petejohanson Feb 6, 2022

megamind4089 Feb 6, 2022

petejohanson Feb 6, 2022

megamind4089 Feb 6, 2022

petejohanson Feb 6, 2022

petejohanson Feb 6, 2022

petejohanson Feb 6, 2022

petejohanson Feb 6, 2022

petejohanson Feb 6, 2022

petejohanson Feb 6, 2022

megamind4089 Feb 6, 2022

petejohanson Feb 6, 2022

megamind4089 Feb 6, 2022

petejohanson May 23, 2022

petejohanson May 23, 2022

petejohanson May 23, 2022

marcoster commented Jun 4, 2022

petejohanson commented Jun 4, 2022

marcoster commented Jun 7, 2022

mbrgm commented Mar 1, 2023

nov1n commented Mar 23, 2023 •

edited

Loading

marcoster commented Apr 13, 2023

jesseleite commented Sep 4, 2023

petejohanson commented Sep 4, 2023

M1cha commented Sep 20, 2023

petejohanson commented Sep 21, 2023

jonathanfv commented Apr 6, 2024

thomasbaart commented Apr 9, 2024

		@@ -155,10 +155,14 @@ config ZMK_SPLIT

		if ZMK_SPLIT

		choice ZMK_SPLIT_TYPE


		#include <zmk/matrix.h>

		#if DT_HAS_CHOSEN(zmk_split_serial)

Add wired split support using UART communication #1117

Are you sure you want to change the base?

Add wired split support using UART communication #1117

Conversation

megamind4089 commented Feb 4, 2022 • edited Loading

petejohanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoster commented Jun 4, 2022

petejohanson commented Jun 4, 2022

marcoster commented Jun 7, 2022

mbrgm commented Mar 1, 2023

nov1n commented Mar 23, 2023 • edited Loading

marcoster commented Apr 13, 2023

jesseleite commented Sep 4, 2023

petejohanson commented Sep 4, 2023

M1cha commented Sep 20, 2023

petejohanson commented Sep 21, 2023

jonathanfv commented Apr 6, 2024

thomasbaart commented Apr 9, 2024

megamind4089 commented Feb 4, 2022 •

edited

Loading

nov1n commented Mar 23, 2023 •

edited

Loading