Add uart_cmd helpers to deduplicate UART command handlers.

Centralize protobuf decode, response init/send, registration, and common
nanopb encode callbacks; refactor existing cmd_* modules to use them.

Co-authored-by: Cursor <cursoragent@cursor.com>

main/CMakeLists.txt

@@ -13,6 +13,7 @@ idf_component_register(
   "led_ring.c"
   "uart.c"
   "uart_proto.c"
+  "uart_cmd.c"
   "cmd_handler.c"
   "cmd_version.c"
   "cmd_client_info.c"

main/README.md

@@ -275,6 +275,7 @@ Target: ESP32-S3. Close serial monitor on the UART adapter port before running `
 | `uart.c/h` | Framed UART RX/TX |
 | `uart_proto.c/h` | Encode/send `UartMessage` |
 | `cmd_handler.c/h` | Command queue and dispatch |
+| `uart_cmd.c/h` | Shared UART decode/send helpers for handlers |
 | `cmd_version.c/h` | VERSION handler |
 | `cmd_client_info.c/h` | CLIENT_INFO handler |
 | `client_registry.c/h` | Registered slave table |
@@ -288,8 +289,8 @@ Target: ESP32-S3. Close serial monitor on the UART adapter port before running `
 ## Adding a new UART command
 
 1. Add or extend messages in `uart_messages.proto` and regenerate nanopb.
-2. Create `cmd_*.c` with a handler; register with `msg_register_handler(MessageType_…, handler)`.
+2. Create `cmd_*.c` with a handler; register with `uart_cmd_register(MessageType_…, handler)`.
-3. Reply via `uart_send_uart_message()` where needed.
+3. Decode with `uart_cmd_decode()` / `UART_CMD_REQ()`; reply with `uart_cmd_init_response()` + `uart_cmd_send()`.
 4. Extend `goTool` or another host client to send the matching frame.
 
 For ESP-NOW-driven PC updates later: map slave state to `ClientInfo` and send `CLIENT_INFO` over UART from the master.

main/cmd_accel_deadzone.c

@@ -1,29 +1,22 @@
 #include "bosch456.h"
 #include "client_registry.h"
 #include "cmd_accel_deadzone.h"
-#include "cmd_handler.h"
 #include "esp_log.h"
 #include "esp_now_comm.h"
-#include "pb_decode.h"
+#include "uart_cmd.h"
-#include "uart_messages.pb.h"
-#include "uart_proto.h"
-#include <string.h>
 
 static const char *TAG = "[ACCEL_DZ]";
 
-static void send_response(uint32_t deadzone, uint32_t client_id, bool success,
+static void reply(uint32_t deadzone, uint32_t client_id, bool success,
-                          uint32_t slaves_updated) {
+                  uint32_t slaves_updated) {
-  alox_UartMessage response = alox_UartMessage_init_zero;
+  alox_UartMessage response;
-  response.type = alox_MessageType_ACCEL_DEADZONE;
+  uart_cmd_init_response(&response, alox_MessageType_ACCEL_DEADZONE,
-  response.which_payload = alox_UartMessage_accel_deadzone_response_tag;
+                         alox_UartMessage_accel_deadzone_response_tag);
   response.payload.accel_deadzone_response.deadzone = deadzone;
   response.payload.accel_deadzone_response.client_id = client_id;
   response.payload.accel_deadzone_response.success = success;
   response.payload.accel_deadzone_response.slaves_updated = slaves_updated;
-
+  uart_cmd_send(&response, TAG);
-  if (uart_send_uart_message(&response) != ESP_OK) {
-    ESP_LOGE(TAG, "failed to send response");
-  }
 }
 
 static esp_err_t push_deadzone_to_slave(const client_info_t *client,
@@ -32,8 +25,7 @@ static esp_err_t push_deadzone_to_slave(const client_info_t *client,
     return ESP_ERR_INVALID_ARG;
   }
 
-  esp_err_t err =
+  esp_err_t err = client_registry_set_accel_deadzone(client->id, deadzone);
-      client_registry_set_accel_deadzone(client->id, deadzone);
   if (err != ESP_OK) {
     return err;
   }
@@ -42,28 +34,20 @@ static esp_err_t push_deadzone_to_slave(const client_info_t *client,
 }
 
 static void handle_accel_deadzone(const uint8_t *data, size_t len) {
-  alox_UartMessage uart_msg = alox_UartMessage_init_zero;
+  alox_UartMessage uart_msg;
   alox_AccelDeadzoneRequest req = alox_AccelDeadzoneRequest_init_zero;
-  bool have_request = false;
 
-  if (len > 0) {
+  if (uart_cmd_decode(data, len, &uart_msg) != ESP_OK) {
-    pb_istream_t stream = pb_istream_from_buffer(data, len);
+    ESP_LOGW(TAG, "decode failed");
-    if (!pb_decode(&stream, alox_UartMessage_fields, &uart_msg)) {
+    reply(BMA456_DEFAULT_ACCEL_DEADZONE, 0, false, 0);
-      ESP_LOGW(TAG, "decode failed");
+    return;
-      send_response(BMA456_DEFAULT_ACCEL_DEADZONE, 0, false, 0);
-      return;
-    }
-    if (uart_msg.which_payload ==
-        alox_UartMessage_accel_deadzone_request_tag) {
-      req = uart_msg.payload.accel_deadzone_request;
-      have_request = true;
-    }
   }
 
-  if (!have_request) {
+  const alox_AccelDeadzoneRequest *req_ptr = UART_CMD_REQ(
-    req.write = false;
+      &uart_msg, alox_UartMessage_accel_deadzone_request_tag,
-    req.client_id = 0;
+      accel_deadzone_request);
-    req.all_clients = false;
+  if (req_ptr != NULL) {
+    req = *req_ptr;
   }
 
   if (req.write) {
@@ -88,7 +72,7 @@ static void handle_accel_deadzone(const uint8_t *data, size_t len) {
 
       ESP_LOGI(TAG, "set deadzone %lu via unicast to %u/%u slaves",
                (unsigned long)req.deadzone, (unsigned)sent, (unsigned)n);
-      send_response(req.deadzone, 0, sent > 0 || bma456_is_ready(), sent);
+      reply(req.deadzone, 0, sent > 0 || bma456_is_ready(), sent);
       return;
     }
 
@@ -97,7 +81,7 @@ static void handle_accel_deadzone(const uint8_t *data, size_t len) {
       ESP_LOGI(TAG, "set local deadzone %lu (no ESP-NOW; use -client or -all "
                     "for slaves)",
                (unsigned long)req.deadzone);
-      send_response(req.deadzone, 0, true, 0);
+      reply(req.deadzone, 0, true, 0);
       return;
     }
 
@@ -105,30 +89,25 @@ static void handle_accel_deadzone(const uint8_t *data, size_t len) {
     if (client == NULL) {
       ESP_LOGW(TAG, "client id %lu not found",
                (unsigned long)req.client_id);
-      send_response(req.deadzone, req.client_id, false, 0);
+      reply(req.deadzone, req.client_id, false, 0);
       return;
     }
 
     esp_err_t err = push_deadzone_to_slave(client, req.deadzone);
-    send_response(req.deadzone, req.client_id, err == ESP_OK, err == ESP_OK);
+    reply(req.deadzone, req.client_id, err == ESP_OK, err == ESP_OK);
     return;
   }
 
-  /* Read */
   if (req.all_clients || req.client_id == 0) {
-    uint32_t dz = bma456_get_accel_deadzone();
+    reply(bma456_get_accel_deadzone(), 0, true, 0);
-    send_response(dz, 0, true, 0);
     return;
   }
 
   uint32_t dz = 0;
   esp_err_t err = client_registry_get_accel_deadzone(req.client_id, &dz);
-  send_response(dz, req.client_id, err == ESP_OK, 0);
+  reply(dz, req.client_id, err == ESP_OK, 0);
 }
 
 void cmd_accel_deadzone_register(void) {
-  if (msg_register_handler(alox_MessageType_ACCEL_DEADZONE,
+  uart_cmd_register(alox_MessageType_ACCEL_DEADZONE, handle_accel_deadzone);
-                           handle_accel_deadzone) != ESP_OK) {
-    ESP_LOGE(TAG, "register failed");
-  }
 }

main/cmd_client_info.c

@@ -1,27 +1,13 @@
 #include "client_registry.h"
 #include "cmd_client_info.h"
-#include "cmd_handler.h"
 #include "esp_log.h"
 #include "pb_encode.h"
-#include "uart_messages.pb.h"
+#include "uart_cmd.h"
-#include "uart_proto.h"
 
 static const char *TAG = "[CLIENT_INFO]";
 
-static bool encode_client_mac(pb_ostream_t *stream, const pb_field_t *field,
-                              void *const *arg) {
-  const uint8_t *mac = (const uint8_t *)*arg;
-  if (mac == NULL) {
-    return true;
-  }
-  if (!pb_encode_tag_for_field(stream, field)) {
-    return false;
-  }
-  return pb_encode_string(stream, mac, CLIENT_MAC_LEN);
-}
-
 static bool encode_clients_list(pb_ostream_t *stream, const pb_field_t *field,
-                              void *const *arg) {
+                                void *const *arg) {
   (void)arg;
 
   size_t count = client_registry_count();
@@ -31,6 +17,8 @@ static bool encode_clients_list(pb_ostream_t *stream, const pb_field_t *field,
       continue;
     }
 
+    uart_cmd_bytes_t mac = {.data = client->mac, .len = CLIENT_MAC_LEN};
+
     alox_ClientInfo proto = alox_ClientInfo_init_zero;
     proto.id = client->id;
     proto.available = client->available;
@@ -39,8 +27,8 @@ static bool encode_clients_list(pb_ostream_t *stream, const pb_field_t *field,
     proto.last_success_ping =
         client_registry_ms_since(client->last_success_ping_at);
     proto.version = client->version;
-    proto.mac.funcs.encode = encode_client_mac;
+    proto.mac.funcs.encode = uart_cmd_encode_bytes;
-    proto.mac.arg = (void *)client->mac;
+    proto.mac.arg = &mac;
 
     if (!pb_encode_tag_for_field(stream, field)) {
       return false;
@@ -56,23 +44,16 @@ static void handle_client_info(const uint8_t *data, size_t len) {
   (void)data;
   (void)len;
 
-  alox_UartMessage response = alox_UartMessage_init_zero;
+  alox_UartMessage response;
-  response.type = alox_MessageType_CLIENT_INFO;
+  uart_cmd_init_response(&response, alox_MessageType_CLIENT_INFO,
-  response.which_payload = alox_UartMessage_client_info_response_tag;
+                         alox_UartMessage_client_info_response_tag);
-  response.payload.client_info_response.clients.funcs.encode =
+  response.payload.client_info_response.clients.funcs.encode = encode_clients_list;
-      encode_clients_list;
   response.payload.client_info_response.clients.arg = NULL;
 
   ESP_LOGI(TAG, "sending %u clients", (unsigned)client_registry_count());
-
+  uart_cmd_send(&response, TAG);
-  if (uart_send_uart_message(&response) != ESP_OK) {
-    ESP_LOGE(TAG, "failed to send response");
-  }
 }
 
 void cmd_client_info_register(void) {
-  if (msg_register_handler(alox_MessageType_CLIENT_INFO, handle_client_info) !=
+  uart_cmd_register(alox_MessageType_CLIENT_INFO, handle_client_info);
-      ESP_OK) {
-    ESP_LOGE(TAG, "register failed");
-  }
 }

main/cmd_espnow_unicast_test.c

@@ -1,66 +1,51 @@
 #include "client_registry.h"
 #include "cmd_espnow_unicast_test.h"
-#include "cmd_handler.h"
 #include "esp_log.h"
 #include "esp_now_comm.h"
-#include "pb_decode.h"
+#include "uart_cmd.h"
-#include "uart_messages.pb.h"
-#include "uart_proto.h"
 
 static const char *TAG = "[UNICAST_TEST]";
 
-static void send_response(bool success, uint32_t seq) {
+static void reply(bool success, uint32_t seq) {
-  alox_UartMessage response = alox_UartMessage_init_zero;
+  alox_UartMessage response;
-  response.type = alox_MessageType_ESPNOW_UNICAST_TEST;
+  uart_cmd_init_response(&response, alox_MessageType_ESPNOW_UNICAST_TEST,
-  response.which_payload = alox_UartMessage_espnow_unicast_test_response_tag;
+                         alox_UartMessage_espnow_unicast_test_response_tag);
   response.payload.espnow_unicast_test_response.success = success;
   response.payload.espnow_unicast_test_response.seq = seq;
-
+  uart_cmd_send(&response, TAG);
-  if (uart_send_uart_message(&response) != ESP_OK) {
-    ESP_LOGE(TAG, "failed to send response");
-  }
 }
 
 static void handle_espnow_unicast_test(const uint8_t *data, size_t len) {
-  alox_UartMessage uart_msg = alox_UartMessage_init_zero;
+  alox_UartMessage uart_msg;
-  alox_EspNowUnicastTestRequest req = alox_EspNowUnicastTestRequest_init_zero;
-  bool have_request = false;
 
-  if (len > 0) {
+  if (uart_cmd_decode(data, len, &uart_msg) != ESP_OK) {
-    pb_istream_t stream = pb_istream_from_buffer(data, len);
+    ESP_LOGW(TAG, "decode failed");
-    if (!pb_decode(&stream, alox_UartMessage_fields, &uart_msg)) {
+    reply(false, 0);
-      ESP_LOGW(TAG, "decode failed");
-      send_response(false, 0);
-      return;
-    }
-    if (uart_msg.which_payload ==
-        alox_UartMessage_espnow_unicast_test_request_tag) {
-      req = uart_msg.payload.espnow_unicast_test_request;
-      have_request = true;
-    }
-  }
-
-  if (!have_request || req.client_id == 0) {
-    ESP_LOGW(TAG, "need client_id in request");
-    send_response(false, 0);
     return;
   }
 
-  const client_info_t *client = client_registry_find_by_id(req.client_id);
+  const alox_EspNowUnicastTestRequest *req = UART_CMD_REQ(
+      &uart_msg, alox_UartMessage_espnow_unicast_test_request_tag,
+      espnow_unicast_test_request);
+  if (req == NULL || req->client_id == 0) {
+    ESP_LOGW(TAG, "need client_id in request");
+    reply(false, 0);
+    return;
+  }
+
+  const client_info_t *client = client_registry_find_by_id(req->client_id);
   if (client == NULL) {
     ESP_LOGW(TAG, "client id %lu not in registry",
-             (unsigned long)req.client_id);
+             (unsigned long)req->client_id);
-    send_response(false, req.seq);
+    reply(false, req->seq);
     return;
   }
 
-  esp_err_t err = esp_now_comm_send_unicast_test(client->mac, req.seq);
+  esp_err_t err = esp_now_comm_send_unicast_test(client->mac, req->seq);
-  send_response(err == ESP_OK, req.seq);
+  reply(err == ESP_OK, req->seq);
 }
 
 void cmd_espnow_unicast_test_register(void) {
-  if (msg_register_handler(alox_MessageType_ESPNOW_UNICAST_TEST,
+  uart_cmd_register(alox_MessageType_ESPNOW_UNICAST_TEST,
-                           handle_espnow_unicast_test) != ESP_OK) {
+                    handle_espnow_unicast_test);
-    ESP_LOGE(TAG, "register failed");
-  }
 }

main/cmd_version.c

@@ -1,10 +1,5 @@
-#include "cmd_handler.h"
 #include "cmd_version.h"
-#include "esp_log.h"
+#include "uart_cmd.h"
-#include "pb_encode.h"
-#include "uart_messages.pb.h"
-#include "uart_proto.h"
-#include <string.h>
 
 #ifndef POWERPOD_FW_VERSION
 #define POWERPOD_FW_VERSION 1u
@@ -16,36 +11,19 @@
 
 static const char *TAG = "[VERSION]";
 
-static bool encode_git_hash(pb_ostream_t *stream, const pb_field_t *field,
-                            void *const *arg) {
-  const char *str = (const char *)*arg;
-  if (str == NULL) {
-    str = "";
-  }
-  if (!pb_encode_tag_for_field(stream, field)) {
-    return false;
-  }
-  return pb_encode_string(stream, (const pb_byte_t *)str, strlen(str));
-}
-
 static void handle_version(const uint8_t *data, size_t len) {
   (void)data;
   (void)len;
 
-  alox_UartMessage response = alox_UartMessage_init_zero;
+  alox_UartMessage response;
-  response.type = alox_MessageType_VERSION;
+  uart_cmd_init_response(&response, alox_MessageType_VERSION,
-  response.which_payload = alox_UartMessage_version_response_tag;
+                         alox_UartMessage_version_response_tag);
   response.payload.version_response.version = POWERPOD_FW_VERSION;
-  response.payload.version_response.git_hash.funcs.encode = encode_git_hash;
+  response.payload.version_response.git_hash.funcs.encode = uart_cmd_encode_string;
   response.payload.version_response.git_hash.arg = (void *)POWERPOD_GIT_HASH;
-
+  uart_cmd_send(&response, TAG);
-  if (uart_send_uart_message(&response) != ESP_OK) {
-    ESP_LOGE(TAG, "failed to send response");
-  }
 }
 
 void cmd_version_register(void) {
-  if (msg_register_handler(alox_MessageType_VERSION, handle_version) != ESP_OK) {
+  uart_cmd_register(alox_MessageType_VERSION, handle_version);
-    ESP_LOGE(TAG, "register failed");
-  }
 }

main/uart_cmd.c

@@ -0,0 +1,82 @@
+#include "uart_cmd.h"
+#include "esp_log.h"
+#include "pb_decode.h"
+#include "pb_encode.h"
+#include "uart_proto.h"
+#include <string.h>
+
+static const char *TAG = "[UART_CMD]";
+
+esp_err_t uart_cmd_decode(const uint8_t *data, size_t len, alox_UartMessage *out) {
+  if (out == NULL) {
+    return ESP_ERR_INVALID_ARG;
+  }
+
+  alox_UartMessage zero = alox_UartMessage_init_zero;
+  *out = zero;
+  if (len == 0) {
+    return ESP_OK;
+  }
+  if (data == NULL) {
+    return ESP_ERR_INVALID_ARG;
+  }
+
+  pb_istream_t stream = pb_istream_from_buffer(data, len);
+  if (!pb_decode(&stream, alox_UartMessage_fields, out)) {
+    return ESP_FAIL;
+  }
+  return ESP_OK;
+}
+
+void uart_cmd_init_response(alox_UartMessage *msg, alox_MessageType type,
+                            pb_size_t which_payload) {
+  if (msg == NULL) {
+    return;
+  }
+  alox_UartMessage zero = alox_UartMessage_init_zero;
+  *msg = zero;
+  msg->type = type;
+  msg->which_payload = which_payload;
+}
+
+esp_err_t uart_cmd_send(const alox_UartMessage *msg, const char *log_tag) {
+  esp_err_t err = uart_send_uart_message(msg);
+  if (err != ESP_OK && log_tag != NULL) {
+    ESP_LOGE(log_tag, "failed to send UART response");
+  }
+  return err;
+}
+
+esp_err_t uart_cmd_register(alox_MessageType type, msg_callback_t cb) {
+  esp_err_t err = msg_register_handler((uint16_t)type, cb);
+  if (err != ESP_OK) {
+    ESP_LOGE(TAG, "register handler for type %u failed", (unsigned)type);
+  }
+  return err;
+}
+
+bool uart_cmd_encode_string(pb_ostream_t *stream, const pb_field_t *field,
+                            void *const *arg) {
+  const char *str = (const char *)*arg;
+  if (str == NULL) {
+    str = "";
+  }
+  if (!pb_encode_tag_for_field(stream, field)) {
+    return false;
+  }
+  return pb_encode_string(stream, (const pb_byte_t *)str, strlen(str));
+}
+
+bool uart_cmd_encode_bytes(pb_ostream_t *stream, const pb_field_t *field,
+                           void *const *arg) {
+  const uart_cmd_bytes_t *blob = (const uart_cmd_bytes_t *)*arg;
+
+  (void)field;
+  if (blob == NULL || blob->data == NULL) {
+    return true;
+  }
+  if (!pb_encode_tag_for_field(stream, field)) {
+    return false;
+  }
+  return pb_encode_string(stream, blob->data, blob->len);
+}

main/uart_cmd.h

@@ -0,0 +1,37 @@
+#ifndef UART_CMD_H
+#define UART_CMD_H
+
+#include "cmd_handler.h"
+#include "esp_err.h"
+#include "pb.h"
+#include "uart_messages.pb.h"
+
+/** Decode framed UART body (protobuf UartMessage without the leading type byte). */
+esp_err_t uart_cmd_decode(const uint8_t *data, size_t len, alox_UartMessage *out);
+
+/** Set type + oneof tag on an outbound UartMessage. */
+void uart_cmd_init_response(alox_UartMessage *msg, alox_MessageType type,
+                            pb_size_t which_payload);
+
+/** Encode and send; logs on failure when log_tag is non-NULL. */
+esp_err_t uart_cmd_send(const alox_UartMessage *msg, const char *log_tag);
+
+esp_err_t uart_cmd_register(alox_MessageType type, msg_callback_t cb);
+
+/** Nanopb encode callback for a NUL-terminated C string. */
+bool uart_cmd_encode_string(pb_ostream_t *stream, const pb_field_t *field,
+                            void *const *arg);
+
+typedef struct {
+  const uint8_t *data;
+  size_t len;
+} uart_cmd_bytes_t;
+
+/** Nanopb encode callback; arg is uart_cmd_bytes_t *. */
+bool uart_cmd_encode_bytes(pb_ostream_t *stream, const pb_field_t *field,
+                           void *const *arg);
+
+#define UART_CMD_REQ(msg, tag, field)                                          \
+  ((msg)->which_payload == (tag) ? &(msg)->payload.field : NULL)
+
+#endif