Studio RPC Infrastructure

[petejohanson]

Initial PR for the RPC infrastructure to be used by ZMK Studio.

This includes the behavior metadata commit from #2231 as it's used by the first really fleshed out behavior subsystem.

To Do

• [x] Document framing and protocol buffer messages.
• [x] Wrap up the sample client library.
• [ ] Testing/validation from others.

[and-elf]

Why not start with a cli tool, that handles the actual rpc calls, and layer studio gui ontop of it later? I'm not sure the gui (library) should be a part of this repo. The binary could be used by https://github.com/nickcoutsos/keymap-editor

I'm not a maintainer, but it seems to me that the software isn't a part of the firmware. When I do RPC, I usually keep protocol definitions in an own repo, which server and client references.

[megafloras]

Nit:

From client to device Perhaps "Studio Client to ZMK Device" or something similar to establish familiarity if folks aren't used to RPC

Browser metadata

Path:      /docs/development/studio-rpc-protocol
Browser:   Chrome 126.0.0.0 on Android 10
Viewport:  360 x 778 @3x
Language:  en-US
Cookies:   Enabled

Open in BrowserStack

Open Deploy Preview · Mark as Resolved

[petejohanson]

Nit:

From client to device Perhaps "Studio Client to ZMK Device" or something similar to establish familiarity if folks aren't used to RPC

Browser metadata

Open Deploy Preview · Mark as Resolved

Updated.

[petejohanson]

My first concern is that we have support for locking from unlocked after a timeout, but not this middle state. What happens if Studio requests an unlock, then without unlocking on the keyboard, you restart Studio? It looks like the keyboard gets stuck in this "unlocking" state unless CONFIG_ZMK_STUDIO_LOCK_ON_DISCONNECT is set? Might be a bit confusing if so.

I think adding a timeout for "unlocking" is a good idea. But yes, it does potentially present some funky states.

My second and probably more concerning concern is that

if (zmk_studio_core_get_lock_state() != ZMK_STUDIO_CORE_LOCK_STATE_LOCKED) { // we're unlocked, so allow things }

looks perfectly reasonable at first glance yet is completely wrong. Would separate bools be any better? That does allow for the not very meaningful state of "is unlocked and unlock is requested", but it would make the above mistake impossible.

This is where I wish for proper tagged enums like Rust, where we could represent this sub-state properly, e.g.:

enum LockedState {
  Locked,
  Unlocking,
}

enum LockState {
  Locked(LockedState),
  Unlocked,
}

I think your idea of separate bools may be the best we can do. Let me think on this a bit though.

[petejohanson]

Thinking about it... I think I will just axe the "Unlocking" state, we don't even need it yet, and there's no point holding this up on that one aspect.

[nmunnich]

Just as a small note, could you either finish (there's a TODO) & add the config and behavior doc pages to the sidebar or remove them from this PR? The RPC page looks good.

[petejohanson]

Just as a small note, could you either finish (there's a TODO) & add the config and behavior doc pages to the sidebar or remove them from this PR? The RPC page looks good.

I will move the config page to #2268 since that PR makes studio actually end user usable. The RPC docs I will add to the sidebar in this PR since it's part of this core work. That work @Nick-Munnich ?

[nmunnich]

I will move the config page to #2268 since that PR makes studio actually end user usable. The RPC docs I will add to the sidebar in this PR since it's part of this core work. That work @Nick-Munnich ?

Moving them is a good solution, but be aware that you've already added the RPC docs, the other part which was added but not listed in the sidebar is "Studio Unlock" behaviour. I'd guess that should be moved too?

[petejohanson]

I will move the config page to #2268 since that PR makes studio actually end user usable. The RPC docs I will add to the sidebar in this PR since it's part of this core work. That work @Nick-Munnich ?

Moving them is a good solution, but be aware that you've already added the RPC docs, the other part which was added but not listed in the sidebar is "Studio Unlock" behaviour. I'd guess that should be moved too?

Yeah, I saw RPC was already in the sidebar. I've left that for now, but moved the config doc to #2268 and added in the sidebar there.

I wanted to be sure I had the unlock behavior documented, but don't really want to "expose it" until it's actually usable for something. I'd like to keep the doc, but add to sidebar in #2268. Any concerns?

[nmunnich]

I wanted to be sure I had the unlock behavior documented, but don't really want to "expose it" until it's actually usable for something. I'd like to keep the doc, but add to sidebar in #2268. Any concerns?

Best practice would probably be to add it to sidebar and then comment it out in this PR, but I think that's not necessary unless you wanted to. All good from me.

[petejohanson]

I wanted to be sure I had the unlock behavior documented, but don't really want to "expose it" until it's actually usable for something. I'd like to keep the doc, but add to sidebar in #2268. Any concerns?

Best practice would probably be to add it to sidebar and then comment it out in this PR, but I think that's not necessary unless you wanted to. All good from me.

Given the type check from Docusaurus is now angry about unlock docs referencing the config docs that don't exist, I'm just going to move that doc to #2268 also for cohesion.