From a55de934f9fb1cbcdd8072781f09fc6c04786984 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Wed, 13 Mar 2024 07:15:04 +0000 Subject: [PATCH] refactor: Move studio RPC to nanopb messages. * Protocol Buffers are way more widely supported CBOR with CDDL, so switch to that message encoding. --- app/CMakeLists.txt | 15 +++++++- app/include/zmk/studio/rpc.h | 59 ++++++++++++++++------------- app/proto/CMakeLists.txt | 6 --- app/proto/zmk/behaviors.options | 2 + app/proto/zmk/behaviors.proto | 43 +++++++++++++++++++++ app/proto/zmk/core.proto | 21 ++++++++++ app/proto/zmk/keymap.proto | 9 +++++ app/proto/zmk/studio-msgs.cmake | 33 ---------------- app/proto/zmk/studio.proto | 55 +++++++++++++++++++++++++++ app/src/studio/Kconfig | 9 ++++- app/src/studio/core_subsystem.c | 11 +++--- app/src/studio/gatt_rpc_interface.c | 33 +++++++++++----- app/src/studio/rpc.c | 17 +++++---- app/src/studio/uart_rpc_interface.c | 36 ++++++++++++------ 14 files changed, 246 insertions(+), 103 deletions(-) delete mode 100644 app/proto/CMakeLists.txt create mode 100644 app/proto/zmk/behaviors.options create mode 100644 app/proto/zmk/behaviors.proto create mode 100644 app/proto/zmk/core.proto create mode 100644 app/proto/zmk/keymap.proto delete mode 100644 app/proto/zmk/studio-msgs.cmake create mode 100644 app/proto/zmk/studio.proto diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 50df50fa3f7..0d211ef7493 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -107,8 +107,19 @@ target_sources(app PRIVATE src/main.c) add_subdirectory(src/display/) -add_subdirectory(proto/) -if(CONFIG_ZMK_STUDIO) +if (CONFIG_ZMK_STUDIO) + # For some reason this is failing if run from a different sub-file. + list(APPEND CMAKE_MODULE_PATH ${ZEPHYR_BASE}/modules/nanopb) + + include(nanopb) + + zephyr_nanopb_sources(app + proto/zmk/studio.proto + proto/zmk/core.proto + proto/zmk/behaviors.proto + proto/zmk/keymap.proto + ) + add_subdirectory(src/studio) endif() diff --git a/app/include/zmk/studio/rpc.h b/app/include/zmk/studio/rpc.h index 11d50991959..c7c0b5d350a 100644 --- a/app/include/zmk/studio/rpc.h +++ b/app/include/zmk/studio/rpc.h @@ -2,14 +2,15 @@ #pragma once #include -#include + +#include struct zmk_rpc_subsystem; -typedef struct response_r(subsystem_func)(const struct zmk_rpc_subsystem *subsys, - const struct request *req); +typedef zmk_Response(subsystem_func)(const struct zmk_rpc_subsystem *subsys, + const zmk_Request *req); -typedef struct response_r(rpc_func)(const struct request *req); +typedef zmk_Response(rpc_func)(const zmk_Request *neq); struct zmk_rpc_subsystem { subsystem_func *func; @@ -25,48 +26,54 @@ struct zmk_rpc_subsystem_handler { }; #define ZMK_RPC_SUBSYSTEM(prefix) \ - struct response_r subsystem_func_##prefix(const struct zmk_rpc_subsystem *subsys, \ - const struct request *req) { \ - uint8_t choice = req->prefix##_subsystem_m.prefix##_request_m.prefix##_request_choice; \ + zmk_Response subsystem_func_##prefix(const struct zmk_rpc_subsystem *subsys, \ + const zmk_Request *req) { \ + uint8_t which_req = req->subsystem.prefix.which_request_type; \ + LOG_DBG("Got subsystem func for %d", subsys->subsystem_choice); \ \ for (int i = subsys->handlers_start_index; i <= subsys->handlers_end_index; i++) { \ struct zmk_rpc_subsystem_handler *sub_handler; \ STRUCT_SECTION_GET(zmk_rpc_subsystem_handler, i, &sub_handler); \ - if (sub_handler->request_choice == choice) { \ + if (sub_handler->request_choice == which_req) { \ return sub_handler->func(req); \ } \ } \ - LOG_ERR("No handler func found for %d", choice); \ - return (struct response_r){}; \ + LOG_ERR("No handler func found for %d", which_req); \ + zmk_Response fallback_resp = zmk_Response_init_zero; \ + return fallback_resp; \ } \ STRUCT_SECTION_ITERABLE(zmk_rpc_subsystem, prefix##_subsystem) = { \ .func = subsystem_func_##prefix, \ - .subsystem_choice = request_union_##prefix##_subsystem_m_c, \ + .subsystem_choice = zmk_Request_##prefix##_tag, \ }; -#define ZMK_RPC_SUBSYSTEM_HANDLER(prefix, request_choice_val, func_val) \ +#define ZMK_RPC_SUBSYSTEM_HANDLER(prefix, request_id) \ STRUCT_SECTION_ITERABLE(zmk_rpc_subsystem_handler, \ - prefix##_subsystem_handler_##request_choice_val) = { \ - .func = func_val, \ - .subsystem_choice = request_union_##prefix##_subsystem_m_c, \ - .request_choice = request_choice_val, \ + prefix##_subsystem_handler_##request_id) = { \ + .func = request_id, \ + .subsystem_choice = zmk_Request_##prefix##_tag, \ + .request_choice = zmk_##prefix##_response_##request_id##_tag, \ }; -#define ZMK_RPC_RESPONSE(subsys, type, ...) \ - ((struct response_r){ \ - .response_choice = request_response_m_c, \ - .request_response_m = \ +#define ZMK_RPC_RESPONSE(subsys, _type, ...) \ + ((zmk_Response){ \ + .which_type = zmk_Response_request_response_tag, \ + .type = \ { \ - .response_payload_m = \ + .request_response = \ { \ - .response_payload_choice = response_payload_##subsys##_response_m_c, \ - .subsys##_response_m = \ + .which_subsystem = zmk_RequestResponse_##subsys##_tag, \ + .subsystem = \ { \ - .union_choice = subsys##_response_union_##type##_m_c, \ - .type##_m = __VA_ARGS__, \ + .subsys = \ + { \ + .which_response_type = \ + zmk_##subsys##_response_##_type##_tag, \ + .response_type = {._type = __VA_ARGS__}, \ + }, \ }, \ }, \ }, \ }) -struct response_r zmk_rpc_handle_request(const struct request *req); +zmk_Response zmk_rpc_handle_request(const zmk_Request *req); diff --git a/app/proto/CMakeLists.txt b/app/proto/CMakeLists.txt deleted file mode 100644 index a39f6dfc6bf..00000000000 --- a/app/proto/CMakeLists.txt +++ /dev/null @@ -1,6 +0,0 @@ - - - -include(${CMAKE_CURRENT_LIST_DIR}/zmk/studio-msgs.cmake) -target_include_directories(app PRIVATE ${CMAKE_CURRENT_LIST_DIR}/zmk/include) -target_link_libraries(app PRIVATE studio-msgs) diff --git a/app/proto/zmk/behaviors.options b/app/proto/zmk/behaviors.options new file mode 100644 index 00000000000..377bc718c84 --- /dev/null +++ b/app/proto/zmk/behaviors.options @@ -0,0 +1,2 @@ +zmk.behaviors.list_all_behaviors_response.behaviors max_count:20 +zmk.behaviors.get_behavior_details_response.friendly_name max_size:20 \ No newline at end of file diff --git a/app/proto/zmk/behaviors.proto b/app/proto/zmk/behaviors.proto new file mode 100644 index 00000000000..7fb5ea3186b --- /dev/null +++ b/app/proto/zmk/behaviors.proto @@ -0,0 +1,43 @@ +syntax = "proto3"; + +package zmk.behaviors; + +message request { + oneof request_type { + bool list_all_behaviors = 1; + get_behavior_details_request get_behavior_details = 2; + } +} + +message get_behavior_details_request { + uint32 behavior_id = 1; +} + +message response { + oneof response_type { + list_all_behaviors_response list_all_behaviors = 1; + get_behavior_details_response get_behavior_details = 2; + } +} + +message list_all_behaviors_response { + repeated uint32 behaviors = 1; +} + +message get_behavior_details_response { + uint32 id = 1; + string friendly_name = 2; + behavior_binding_parameter_details param1 = 3; + behavior_binding_parameter_details param2 = 4; +} + +message behavior_binding_parameter_details { + behavior_binding_parameter_domain domain = 1; + string name = 2; +} + +enum behavior_binding_parameter_domain { + Nil = 0; + HIDUsage = 1; + LayerIndex = 2; +} \ No newline at end of file diff --git a/app/proto/zmk/core.proto b/app/proto/zmk/core.proto new file mode 100644 index 00000000000..db8ad06afb8 --- /dev/null +++ b/app/proto/zmk/core.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package zmk.core; + +message request { + oneof request_type { + bool get_lock_status = 1; + bool unlock_request = 2; + bool lock_request = 3; + } +} + +message get_lock_status_response { + bool locked = 1; +} + +message response { + oneof response_type { + get_lock_status_response get_lock_status = 1; + } +} \ No newline at end of file diff --git a/app/proto/zmk/keymap.proto b/app/proto/zmk/keymap.proto new file mode 100644 index 00000000000..9ae37788e5b --- /dev/null +++ b/app/proto/zmk/keymap.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package zmk.keymap; + +message request { + oneof request_type { + bool get_layer_summaries = 1; + } +} \ No newline at end of file diff --git a/app/proto/zmk/studio-msgs.cmake b/app/proto/zmk/studio-msgs.cmake deleted file mode 100644 index 27ac6df0e38..00000000000 --- a/app/proto/zmk/studio-msgs.cmake +++ /dev/null @@ -1,33 +0,0 @@ - -set (zcbor_command - python3 ${ZEPHYR_ZCBOR_MODULE_DIR}/zcbor/zcbor.py code - --decode --encode - --short-names - -c ${CMAKE_CURRENT_LIST_DIR}/studio-msgs.cddl - -t request response - --output-c ${CMAKE_CURRENT_LIST_DIR}/src/studio-msgs.c - --output-h ${CMAKE_CURRENT_LIST_DIR}/include/studio-msgs.h - ) - -add_custom_command( - OUTPUT "${CMAKE_CURRENT_LIST_DIR}/src/studio-msgs_decode.c" - "${CMAKE_CURRENT_LIST_DIR}/src/studio-msgs_encode.c" - "${CMAKE_CURRENT_LIST_DIR}/include/studio-msgs_encode.h" - "${CMAKE_CURRENT_LIST_DIR}/include/studio-msgs_decode.h" - "${CMAKE_CURRENT_LIST_DIR}/include/studio-msgs_types.h" - COMMAND ${zcbor_command} - DEPENDS "${CMAKE_CURRENT_LIST_DIR}/studio-msgs.cddl" - VERBATIM) - -zephyr_library_named(studio-msgs) -zephyr_library_sources( - ${CMAKE_CURRENT_LIST_DIR}/../../../modules/lib/zcbor/src/zcbor_decode.c - ${CMAKE_CURRENT_LIST_DIR}/../../../modules/lib/zcbor/src/zcbor_encode.c - ${CMAKE_CURRENT_LIST_DIR}/../../../modules/lib/zcbor/src/zcbor_common.c - ${CMAKE_CURRENT_LIST_DIR}/src/studio-msgs_decode.c - ${CMAKE_CURRENT_LIST_DIR}/src/studio-msgs_encode.c - ) -zephyr_library_include_directories( - ${CMAKE_CURRENT_LIST_DIR}/include - ${CMAKE_CURRENT_LIST_DIR}/../../../modules/lib/zcbor/include - ) diff --git a/app/proto/zmk/studio.proto b/app/proto/zmk/studio.proto new file mode 100644 index 00000000000..a2e71c87852 --- /dev/null +++ b/app/proto/zmk/studio.proto @@ -0,0 +1,55 @@ +// Requests +syntax = "proto3"; + +package zmk; + +import "core.proto"; +import "behaviors.proto"; +import "keymap.proto"; + +message Request { + uint32 request_id = 1; + + oneof subsystem { + zmk.core.request core = 2; + zmk.keymap.request keymap = 3; + zmk.behaviors.request behaviors = 4; + } +} + + +// keymap_request = { +// (0: get_layers_summary) +// } + +// behavior_request = { +// (0: list_all_behaviors) / +// (1: get_behavior_details) +// } + +// get_layers_summary = nil + +// list_all_behaviors = nil +// get_behavior_details = [ behavior_id: uint ] + +// ; Responses + +message Response { + oneof type { + RequestResponse request_response = 1; + Notification notification = 2; + } +} + +message RequestResponse { + uint32 request_id = 1; + oneof subsystem { + zmk.core.response core = 2; + zmk.behaviors.response behaviors = 4; + } +} + +message Notification { + uint32 notification_type = 1; + +} diff --git a/app/src/studio/Kconfig b/app/src/studio/Kconfig index 43b7636ca7c..f2e2e10c25c 100644 --- a/app/src/studio/Kconfig +++ b/app/src/studio/Kconfig @@ -1,6 +1,7 @@ menuconfig ZMK_STUDIO bool "Studio Support" + select NANOPB help Add firmware support for realtime keymap updates (ZMK Studio @@ -10,12 +11,16 @@ menu "Interfaces" config ZMK_STUDIO_INTERFACE_UART bool "Serial" + select SERIAL + select NET_BUF default y if ZMK_USB config ZMK_STUDIO_INTERFACE_BLE bool "BLE (GATT)" - default ZMK_BLE + select NET_BUF + depends on ZMK_BLE + default y endmenu -endif \ No newline at end of file +endif diff --git a/app/src/studio/core_subsystem.c b/app/src/studio/core_subsystem.c index 05f7e62d4e6..8c869ade9a6 100644 --- a/app/src/studio/core_subsystem.c +++ b/app/src/studio/core_subsystem.c @@ -8,10 +8,11 @@ ZMK_RPC_SUBSYSTEM(core) #define CORE_RESPONSE(type, ...) ZMK_RPC_RESPONSE(core, type, __VA_ARGS__) -struct response_r get_lock_status(const struct request *req) { - return CORE_RESPONSE(get_lock_state_response, { - .locked = false, - }); +zmk_Response get_lock_status(const zmk_Request *req) { + zmk_core_get_lock_status_response resp = zmk_core_get_lock_status_response_init_zero; + resp.locked = true; + + return CORE_RESPONSE(get_lock_status, resp); } -ZMK_RPC_SUBSYSTEM_HANDLER(core, core_request_get_lock_state_m_c, get_lock_status); +ZMK_RPC_SUBSYSTEM_HANDLER(core, get_lock_status); diff --git a/app/src/studio/gatt_rpc_interface.c b/app/src/studio/gatt_rpc_interface.c index a278047725d..9329462a2fe 100644 --- a/app/src/studio/gatt_rpc_interface.c +++ b/app/src/studio/gatt_rpc_interface.c @@ -16,7 +16,10 @@ #include "uuid.h" #include "common.h" -#include +#include +#include + +#include #include @@ -54,7 +57,7 @@ static ssize_t read_rpc_resp(struct bt_conn *conn, const struct bt_gatt_attr *at // return bt_gatt_attr_read(conn, attr, buf, len, offset, &level, sizeof(uint8_t)); } -static void send_response(struct bt_conn *conn, const struct response_r *response); +static void send_response(struct bt_conn *conn, const zmk_Response *response); static ssize_t write_rpc_req(struct bt_conn *conn, const struct bt_gatt_attr *attr, const void *buf, uint16_t len, uint16_t offset, uint8_t flags) { @@ -65,12 +68,14 @@ static ssize_t write_rpc_req(struct bt_conn *conn, const struct bt_gatt_attr *at if (rpc_framing_state == FRAMING_STATE_IDLE && rx_buf.len > 0) { LOG_DBG("Got a full message"); - struct request req; - size_t req_decoded; - cbor_decode_request(rx_buf.data, rx_buf.len, &req, &req_decoded); + zmk_Request req = zmk_Request_init_zero; + + pb_istream_t stream = pb_istream_from_buffer(rx_buf.data, rx_buf.len); - if (req_decoded == rx_buf.len) { - struct response_r resp = zmk_rpc_handle_request(&req); + bool status = pb_decode(&stream, &zmk_Request_msg, &req); + + if (status) { + zmk_Response resp = zmk_rpc_handle_request(&req); send_response(conn, &resp); } @@ -90,11 +95,19 @@ BT_GATT_SERVICE_DEFINE( write_rpc_req, NULL), BT_GATT_CCC(rpc_ccc_cfg_changed, BT_GATT_PERM_READ_ENCRYPT | BT_GATT_PERM_WRITE_ENCRYPT)); -static void send_response(struct bt_conn *conn, const struct response_r *response) { +static void send_response(struct bt_conn *conn, const zmk_Response *response) { uint8_t resp_data[128]; - size_t out_size = sizeof(resp_data); + pb_ostream_t stream = pb_ostream_from_buffer(resp_data, ARRAY_SIZE(resp_data)); + + /* Now we are ready to encode the message! */ + bool status = pb_encode(&stream, &zmk_Response_msg, response); + + if (!status) { + LOG_ERR("Failed to encode a response message"); + return; + } - cbor_encode_response(resp_data, sizeof(resp_data), response, &out_size); + size_t out_size = stream.bytes_written; uint8_t resp_frame[128]; diff --git a/app/src/studio/rpc.c b/app/src/studio/rpc.c index 6f352453ccd..61adef7b710 100644 --- a/app/src/studio/rpc.c +++ b/app/src/studio/rpc.c @@ -7,13 +7,13 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); #include -struct response_r zmk_rpc_handle_request(const struct request *req) { - LOG_WRN("Got a req of union type: %d", req->union_choice); +zmk_Response zmk_rpc_handle_request(const zmk_Request *req) { + LOG_WRN("Got a req of union type: %d", req->which_subsystem); STRUCT_SECTION_FOREACH(zmk_rpc_subsystem, sub) { - if (sub->subsystem_choice == req->union_choice) { + if (sub->subsystem_choice == req->which_subsystem) { LOG_WRN("Found a func!"); - struct response_r resp = sub->func(sub, req); - resp.request_response_m.request_id = req->request_id; + zmk_Response resp = sub->func(sub, req); + resp.type.request_response.request_id = req->request_id; return resp; } @@ -21,9 +21,8 @@ struct response_r zmk_rpc_handle_request(const struct request *req) { LOG_WRN("No HANDLER FOUND"); // TODO: NOT SUPPORTED - return (struct response_r){ - - }; + zmk_Response resp = zmk_Response_init_zero; + return resp; } static struct zmk_rpc_subsystem *find_subsystem_for_choice(uint8_t choice) { @@ -47,6 +46,8 @@ static int zmk_rpc_init(void) { __ASSERT(sub != NULL, "RPC Handler for unknown subsystem choice %d", handler->subsystem_choice); + LOG_DBG("Init handler %d sub %d", handler->request_choice, sub->subsystem_choice); + if (prev_choice < 0) { sub->handlers_start_index = i; } else if ((prev_choice != handler->subsystem_choice) && prev_sub) { diff --git a/app/src/studio/uart_rpc_interface.c b/app/src/studio/uart_rpc_interface.c index ddaedbad7fa..2efa58218b2 100644 --- a/app/src/studio/uart_rpc_interface.c +++ b/app/src/studio/uart_rpc_interface.c @@ -14,8 +14,13 @@ #include "common.h" -#include -#include +// #include +// #include + +#include +#include + +#include LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); @@ -31,19 +36,23 @@ static const struct device *const uart_dev = DEVICE_DT_GET(UART_DEVICE_NODE); NET_BUF_SIMPLE_DEFINE_STATIC(rx_buf, MSG_SIZE); /* queue to store up to 10 messages (aligned to 4-byte boundary) */ -K_MSGQ_DEFINE(req_msgq, sizeof(struct request), 10, 4); +K_MSGQ_DEFINE(req_msgq, sizeof(zmk_Request), 10, 4); static enum studio_framing_state rpc_framing_state; void rpc_cb(struct k_work *work) { - struct request req; + zmk_Request req; while (k_msgq_get(&req_msgq, &req, K_NO_WAIT) >= 0) { - struct response_r resp = zmk_rpc_handle_request(&req); + zmk_Response resp = zmk_rpc_handle_request(&req); uint8_t payload[32]; - size_t how_much; - cbor_encode_response(payload, ARRAY_SIZE(payload), &resp, &how_much); + pb_ostream_t stream = pb_ostream_from_buffer(payload, ARRAY_SIZE(payload)); + + /* Now we are ready to encode the message! */ + bool status = pb_encode(&stream, &zmk_Response_msg, &resp); + + size_t how_much = stream.bytes_written; LOG_HEXDUMP_DBG(payload, how_much, "Encoded payload"); @@ -87,13 +96,18 @@ static void serial_cb(const struct device *dev, void *user_data) { LOG_DBG("GOT %d", c); if (rpc_framing_state == FRAMING_STATE_IDLE && rx_buf.len > 0) { - struct request req; - size_t req_decoded; - cbor_decode_request(rx_buf.data, rx_buf.len, &req, &req_decoded); - if (req_decoded == rx_buf.len) { + zmk_Request req = zmk_Request_init_zero; + + pb_istream_t stream = pb_istream_from_buffer(rx_buf.data, rx_buf.len); + + bool status = pb_decode(&stream, &zmk_Request_msg, &req); + + if (status) { k_msgq_put(&req_msgq, &req, K_MSEC(1)); k_work_submit(&rpc_work); } + + // TODO: Just clear the bits that stream read! net_buf_simple_reset(&rx_buf); } }