Feature: Full-Duplex Wired Split #2766

petejohanson · 2025-01-14T01:59:00Z

~~Opening this as a draft for now, pending more extensive testing, documentation, etc.~~

~~I've tested this with a very basic ZMK Uno setup, but nothing extensive dailying on a keyboard, etc.~~

Preview docs: https://deploy-preview-2766--zmk.netlify.app/docs/config/split#wired-splits

Has gotten a fair amount of testing at this point, so moving this out of Draft.

PR check-list

Branch has a clean commit history
Additional tests are included, if changing behaviors/core code that is testable.
Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
Pre-commit used to check formatting of files, commit messages, etc.
Includes any necessary documentation changes.

Other To-Dos

Investigate report of stuck/broken taps on peripheral Cirque. Recovered by restarting left side only.
Address stack size issues for RP2040 exacerbated by this PR?

caksoylar · 2025-02-01T21:05:37Z

Related: #1110.

Nick-Munnich

Couple of notes on the first commit. Haven't dealt with the split ble code bits enough to be a trustworthy reviewer there, but those bits look fine at a glance.

Not looked at the second commit at all.

app/include/linker/zmk-split-transport-central.ld

app/include/zmk/split/central.h

app/include/zmk/split/bluetooth/central.h

app/include/zmk/split/central.h

app/include/zmk/split/transport/types.h

Nick-Munnich · 2025-02-14T18:35:20Z

app/src/split/central.c

+    }
+}
+
+int zmk_split_central_invoke_behavior(uint8_t source, struct zmk_behavior_binding *binding,


Suggested change

int zmk_split_central_invoke_behavior(uint8_t source, struct zmk_behavior_binding *binding,

int zmk_split_central_invoke_behavior(uint8_t target, struct zmk_behavior_binding *binding,

Minor suggestion, but I just remembered thinking the other day that this name for this variable (and similar variables in functions around the transport) would be nicer. Feel free to ignore and hit resolve.

I'm on the fence on this one... It requires just a tad bit more mental load to understand source/target are the same thing, just from different perspectives, but I can see where you are coming from. Leaving unresolved for now to sleep on it. Thanks.

petejohanson · 2025-02-14T23:00:48Z

Ok, taking this out of draft at this point, since it's gotten a fair amount of use and testing by myself and a few others. I just pushed some of the initial docs work as well, which needs review still.

Nick-Munnich

Minor comments on the docs.

docs/docs/config/split.md

docs/docs/features/split-keyboards.md

docs/docs/config/split.md

Nick-Munnich

Some comments. Still working my way through codey-bits slowly, but can't seem to write two reviews at the same time.

app/src/split/central.c

app/include/zmk/split/transport/types.h

docs/docs/config/split.md

app/src/split/wired/wired.h

docs/docs/features/split-keyboards.md

app/dts/bindings/zmk,wired-split.yaml

docs/docs/development/hardware-integration/new-shield.mdx

docs/docs/config/split.md

docs/docs/features/split-keyboards.md

docs/docs/config/split.md

caksoylar · 2025-02-20T00:13:22Z

Note for docs: https://zmk.dev/docs/faq#does-zmk-support-wired-split needs to be updated (in this PR or a future one)

petejohanson · 2025-02-20T00:30:10Z

Note for docs: https://zmk.dev/docs/faq#does-zmk-support-wired-split needs to be updated (in this PR or a future one)

Tweaked that note, would appreciate feedback. Thanks!

docs/docs/config/split.md

docs/docs/faq.md

caksoylar · 2025-02-20T06:21:03Z

docs/docs/features/split-keyboards.md

+
+:::warning[Hot Plugging Cables]
+
+Many popular cables, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.


Suggested change

Many popular cables, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.

Many popular wired interconnects used between split keyboards, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.

I'm on the fence using "interconnect" here since we use that term elsewhere to describe board/shield abstractions.

Fair, maybe something like "physical connectors" instead of "wired interconnects"?

Personally I think this particular warning should be made more specific, not less, as this problem specifically affects TRS-style cables. Things like USB, RJ45, MIDI, etc are all exempt.

Personally I think this particular warning should be made more specific, not less, as this problem specifically affects TRS-style cables. Things like USB, RJ45, MIDI, etc are all exempt.

Beyond my original wording?

Ideally yes, but I was thinking about it a bunch yesterday and I couldn't find a phrasing I liked. Essentially making the warning sound more explicit towards the "bad" cable types, rather than warning against all. I think it's fine as-is though.

What I was also considering is a "unless explicitly stated otherwise by the designer" because I think you can create a circuit which doesn't have this sort of flaw. I worry that might overcomplicate the warning and invite lots of questions from inexperienced folks though, opening up a can of worms we dont want to tackle (yet)

docs/docs/development/hardware-integration/new-shield.mdx

docs/docs/config/split.md

docs/docs/development/hardware-integration/new-shield.mdx

caksoylar

One last note, otherwise docs LGTM.

docs/docs/development/hardware-integration/new-shield.mdx

Nick-Munnich

Last pair of minor queries from me. I think it's unlikely I'll notice any further things from looking at it more.

I will say I don't think this should be merged until we've had another release, which I consider #2729 to be a prerequisite for.

app/include/zmk/split/transport/types.h

paulshir

Leaving comments here as opposed to discord as it's easier to point to the issues. Tested and debugged the latest changes in polling mode with a rpi pico. Events and commands working with the changes I commented on here.

app/src/split/wired/central.c

app/src/split/wired/peripheral.c

app/src/split/wired/wired.c

Extract central/peripheral code to allow for plugging in alternate transports, instead of tying all split logic to BT.

* Depends on full-duplex hardware UART for communication. * Supports all existing central commands/peripheral events, including sensors/inputs from peripherals. * Only one wired split peripheral supported (for now) * Relies on chosen `zmk,split-uart` referencing the UART device.

Migrate split to its own dedicated config file, and add details on wired split config. Co-authored-by: Nicolas Munnich <[email protected]>

Move the system work queue stack size override on RP2040 ouf of a `ZMK_BLE` conditional so it is properly applied generally for that SoC.

petejohanson added enhancement New feature or request split labels Jan 14, 2025

petejohanson self-assigned this Jan 14, 2025

petejohanson force-pushed the split/wired-split-first-pass branch 6 times, most recently from ffd742c to 47b7001 Compare January 17, 2025 22:07

This was referenced Jan 20, 2025

Split Refactor Part 1 #2086

Closed

feat(split): wired split over serial support #2080

Draft

petejohanson force-pushed the split/wired-split-first-pass branch from 45d95af to be7ded8 Compare January 21, 2025 23:52

petejohanson force-pushed the split/wired-split-first-pass branch from 3fea122 to 6baa960 Compare February 1, 2025 03:54

petejohanson force-pushed the split/wired-split-first-pass branch 5 times, most recently from 64ce23f to acd57d2 Compare February 10, 2025 18:32

Nick-Munnich reviewed Feb 14, 2025

View reviewed changes

petejohanson force-pushed the split/wired-split-first-pass branch 3 times, most recently from b365a19 to c10e8fe Compare February 14, 2025 22:51

petejohanson marked this pull request as ready for review February 14, 2025 22:56

petejohanson requested review from a team as code owners February 14, 2025 22:56

Nick-Munnich reviewed Feb 14, 2025

View reviewed changes

caksoylar reviewed Feb 14, 2025

View reviewed changes

docs/docs/config/split.md Outdated Show resolved Hide resolved

Nick-Munnich reviewed Feb 15, 2025

View reviewed changes

docs/docs/config/split.md Outdated Show resolved Hide resolved

petejohanson force-pushed the split/wired-split-first-pass branch from fb46879 to 6b09605 Compare February 18, 2025 20:51

caksoylar reviewed Feb 18, 2025

View reviewed changes

docs/docs/config/split.md Outdated Show resolved Hide resolved

Nick-Munnich reviewed Feb 19, 2025

View reviewed changes

docs/docs/features/split-keyboards.md Outdated Show resolved Hide resolved

docs/docs/config/split.md Show resolved Hide resolved

petejohanson force-pushed the split/wired-split-first-pass branch from 484ce4e to 3e71bc8 Compare February 19, 2025 23:18

caksoylar reviewed Feb 20, 2025

View reviewed changes

petejohanson force-pushed the split/wired-split-first-pass branch from 339ad0d to 76f97ef Compare February 20, 2025 18:17

caksoylar reviewed Feb 20, 2025

View reviewed changes

docs/docs/development/hardware-integration/new-shield.mdx Outdated Show resolved Hide resolved

caksoylar approved these changes Feb 20, 2025

View reviewed changes

docs/docs/development/hardware-integration/new-shield.mdx Show resolved Hide resolved

petejohanson force-pushed the split/wired-split-first-pass branch from f165bbd to c72ec43 Compare February 21, 2025 18:30

Nick-Munnich reviewed Feb 21, 2025

View reviewed changes

app/include/zmk/split/transport/types.h Show resolved Hide resolved

app/include/zmk/split/transport/types.h Show resolved Hide resolved

paulshir reviewed Feb 22, 2025

View reviewed changes

app/src/split/wired/central.c Show resolved Hide resolved

app/src/split/wired/central.c Outdated Show resolved Hide resolved

app/src/split/wired/peripheral.c Outdated Show resolved Hide resolved

app/src/split/wired/wired.c Outdated Show resolved Hide resolved

petejohanson and others added 6 commits March 2, 2025 17:34

refactor(split): Refactor split code for extension

2823b16

Extract central/peripheral code to allow for plugging in alternate transports, instead of tying all split logic to BT.

docs: Add wired split config docs.

a02eca7

Migrate split to its own dedicated config file, and add details on wired split config. Co-authored-by: Nicolas Munnich <[email protected]>

fix: Properly override stack size on RP2040

c05f327

Move the system work queue stack size override on RP2040 ouf of a `ZMK_BLE` conditional so it is properly applied generally for that SoC.

fixup! feat(split): Add full-duplex wired split support

f15acf1

fixup! feat(split): Add full-duplex wired split support

4843ce1

petejohanson force-pushed the split/wired-split-first-pass branch from c00c7c6 to 4843ce1 Compare March 2, 2025 17:34

petejohanson added 2 commits March 2, 2025 10:55

fixup! refactor(split): Refactor split code for extension

8719d5c

fixup! feat(split): Add full-duplex wired split support

b71a1a3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Full-Duplex Wired Split #2766

Feature: Full-Duplex Wired Split #2766

petejohanson commented Jan 14, 2025 •

edited

Loading

caksoylar commented Feb 1, 2025

Nick-Munnich left a comment

Nick-Munnich Feb 14, 2025

petejohanson Feb 14, 2025

petejohanson commented Feb 14, 2025

Nick-Munnich left a comment

Nick-Munnich left a comment

caksoylar commented Feb 20, 2025

petejohanson commented Feb 20, 2025

caksoylar Feb 20, 2025

petejohanson Feb 20, 2025

caksoylar Feb 20, 2025

Nick-Munnich Feb 20, 2025

petejohanson Feb 20, 2025

Nick-Munnich Feb 20, 2025

caksoylar left a comment

Nick-Munnich left a comment

paulshir left a comment

	int zmk_split_central_invoke_behavior(uint8_t source, struct zmk_behavior_binding *binding,
	int zmk_split_central_invoke_behavior(uint8_t target, struct zmk_behavior_binding *binding,


		:::warning[Hot Plugging Cables]

		Many popular cables, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.

	Many popular cables, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.
	Many popular wired interconnects used between split keyboards, in particular, TRRS/TRS cables, can cause irreparable damage to controllers if they are inserted or removed when power is already present on them. Whether or not you are using the wired split functionality or not, _never_ insert or remove such a cable when a controller is powered by USB _or_ battery.

Feature: Full-Duplex Wired Split #2766

Are you sure you want to change the base?

Feature: Full-Duplex Wired Split #2766

Conversation

petejohanson commented Jan 14, 2025 • edited Loading

PR check-list

Other To-Dos

caksoylar commented Feb 1, 2025

Nick-Munnich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petejohanson commented Feb 14, 2025

Nick-Munnich left a comment

Choose a reason for hiding this comment

Nick-Munnich left a comment

Choose a reason for hiding this comment

caksoylar commented Feb 20, 2025

petejohanson commented Feb 20, 2025

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

caksoylar left a comment

Choose a reason for hiding this comment

Nick-Munnich left a comment

Choose a reason for hiding this comment

paulshir left a comment

Choose a reason for hiding this comment

petejohanson commented Jan 14, 2025 •

edited

Loading