Merge pull request #127 from someweisguy:bugfix/rdm-tx-discovery-cb

Update rdm_send_request()

src/rdm/controller/device_control.c

@@ -17,15 +17,16 @@ size_t rdm_send_get_identify_device(dmx_port_t dmx_num,
   DMX_CHECK(identify != NULL, 0, "identify is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const rdm_cc_t cc = RDM_CC_GET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_IDENTIFY_DEVICE;
-  size_t pdl = rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, NULL,
-                                NULL, 0, ack);
-  if (pdl == sizeof(uint8_t)) {
-    const char *format = "b$";
-    rdm_read_pd(dmx_num, format, identify, sizeof(uint8_t));
-  }
-  return pdl;
+  const rdm_request_t request = {
+    .dest_uid = dest_uid,
+    .sub_device = sub_device,
+    .cc = RDM_CC_GET_COMMAND,
+    .pid = RDM_PID_IDENTIFY_DEVICE
+  };
+
+  const char *format = "b$";
+  return rdm_send_request(dmx_num, &request, format, identify,
+                          sizeof(*identify), ack);
 }
 
 bool rdm_send_set_identify_device(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
@@ -38,16 +39,15 @@ bool rdm_send_set_identify_device(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
   DMX_CHECK(identify == 0 || identify == 1, 0, "identify is invalid");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const char *format = "b$";
-  const rdm_cc_t cc = RDM_CC_SET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_IDENTIFY_DEVICE;
-  rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, format, &identify,
-                   sizeof(identify), ack);
-  if (ack != NULL) {
-    return ack->type == RDM_RESPONSE_TYPE_ACK;
-  } else {
-    rdm_header_t header;
-    return rdm_read_header(dmx_num, &header) &&
-           header.response_type == RDM_RESPONSE_TYPE_ACK;
-  }
+  const rdm_request_t request = {
+    .dest_uid = dest_uid,
+    .sub_device = sub_device,
+    .cc = RDM_CC_SET_COMMAND, 
+    .pid = RDM_PID_IDENTIFY_DEVICE,
+    .pd = &identify, 
+    .pdl = sizeof(identify),
+    .format = "b$"
+  };
+
+  return rdm_send_request(dmx_num, &request, NULL, NULL, 0, ack);
 }

src/rdm/controller/discovery.c

@@ -6,36 +6,22 @@
 #include "rdm/include/driver.h"
 #include "rdm/include/uid.h"
 
-static bool rdm_send_mute_static(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
-                                 rdm_pid_t pid, rdm_disc_mute_t *mute,
-                                 rdm_ack_t *ack) {
-  const rdm_sub_device_t sub_device = RDM_SUB_DEVICE_ROOT;
-  const rdm_cc_t cc = RDM_CC_DISC_COMMAND;
-  bool success = rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, NULL,
-                                  NULL, 0, ack);
-  if (success && mute != NULL) {
-    const char *format = "wv";
-    rdm_read_pd(dmx_num, format, mute, sizeof(*mute));
-  }
-  return (ack != NULL && ack->type == RDM_RESPONSE_TYPE_ACK);
-}
-
 bool rdm_send_disc_unique_branch(dmx_port_t dmx_num,
                                  const rdm_disc_unique_branch_t *branch,
                                  rdm_ack_t *ack) {
   DMX_CHECK(dmx_num < DMX_NUM_MAX, 0, "dmx_num error");
   DMX_CHECK(branch != NULL, 0, "branch is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const rdm_uid_t *dest_uid = &RDM_UID_BROADCAST_ALL;
-  const rdm_sub_device_t sub_device = RDM_SUB_DEVICE_ROOT;
-  const rdm_pid_t pid = RDM_PID_DISC_UNIQUE_BRANCH;
-  const rdm_cc_t cc = RDM_CC_DISC_COMMAND;
-  const char *format = "uu$";
+  const rdm_request_t request = {.dest_uid = &RDM_UID_BROADCAST_ALL,
+                                 .sub_device = RDM_SUB_DEVICE_ROOT,
+                                 .cc = RDM_CC_DISC_COMMAND,
+                                 .pid = RDM_PID_DISC_UNIQUE_BRANCH,
+                                 .format = "uu$",
+                                 .pd = branch,
+                                 .pdl = sizeof(*branch)};
 
-  rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, format, branch,
-                   sizeof(*branch), ack);
-  return (ack != NULL && ack->type == RDM_RESPONSE_TYPE_ACK);
+  return rdm_send_request(dmx_num, &request, NULL, NULL, 0, ack);
 }
 
 bool rdm_send_disc_mute(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
@@ -44,8 +30,13 @@ bool rdm_send_disc_mute(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
   DMX_CHECK(dest_uid != NULL, 0, "dest_uid is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  rdm_pid_t pid = RDM_PID_DISC_MUTE;
-  return rdm_send_mute_static(dmx_num, dest_uid, pid, mute, ack);
+  const rdm_request_t request = {.dest_uid = dest_uid,
+                                 .sub_device = RDM_SUB_DEVICE_ROOT,
+                                 .cc = RDM_CC_DISC_COMMAND,
+                                 .pid = RDM_PID_DISC_MUTE};
+
+  const char *format = "wv";
+  return rdm_send_request(dmx_num, &request, format, &mute, sizeof(*mute), ack);
 }
 
 bool rdm_send_disc_un_mute(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
@@ -54,8 +45,13 @@ bool rdm_send_disc_un_mute(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
   DMX_CHECK(dest_uid != NULL, 0, "dest_uid is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  rdm_pid_t pid = RDM_PID_DISC_UN_MUTE;
-  return rdm_send_mute_static(dmx_num, dest_uid, pid, mute, ack);
+  const rdm_request_t request = {.dest_uid = dest_uid,
+                                 .sub_device = RDM_SUB_DEVICE_ROOT,
+                                 .cc = RDM_CC_DISC_COMMAND,
+                                 .pid = RDM_PID_DISC_UN_MUTE};
+
+  const char *format = "wv";
+  return rdm_send_request(dmx_num, &request, format, &mute, sizeof(*mute), ack);
 }
 
 int rdm_discover_with_callback(dmx_port_t dmx_num, rdm_disc_cb_t cb,
@@ -104,7 +100,9 @@ int rdm_discover_with_callback(dmx_port_t dmx_num, rdm_disc_cb_t cb,
 
       // Call the callback function and report a device has been found
       if (ack.type == RDM_RESPONSE_TYPE_ACK) {
+        xSemaphoreGiveRecursive(driver->mux);
         cb(dmx_num, ack.src_uid, num_found, &mute, context);
+        xSemaphoreTakeRecursive(driver->mux, portMAX_DELAY);
         ++num_found;
       }
     } else {

src/rdm/controller/dmx_setup.c

@@ -18,15 +18,16 @@ size_t rdm_send_get_dmx_start_address(dmx_port_t dmx_num,
   DMX_CHECK(dmx_start_address != NULL, 0, "dmx_start_address is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const rdm_cc_t cc = RDM_CC_GET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_DMX_START_ADDRESS;
-  size_t pdl = rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, NULL,
-                                NULL, 0, ack);
-  if (pdl == sizeof(*dmx_start_address)) {
-    const char *format = "w$";
-    rdm_read_pd(dmx_num, format, dmx_start_address, sizeof(*dmx_start_address));
-  }
-  return pdl;
+  const rdm_request_t request = {
+    .dest_uid = dest_uid, 
+    .sub_device = sub_device,
+    .cc = RDM_CC_GET_COMMAND,
+    .pid = RDM_PID_DMX_START_ADDRESS,
+  };
+
+  const char *format = "w$";
+  return rdm_send_request(dmx_num, &request, format, dmx_start_address,
+                          sizeof(*dmx_start_address), ack);
 }
 
 bool rdm_send_set_dmx_start_address(dmx_port_t dmx_num,
@@ -41,16 +42,13 @@ bool rdm_send_set_dmx_start_address(dmx_port_t dmx_num,
   DMX_CHECK(dmx_start_address < 513, 0, "dmx_start_address is invalid");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const char *format = "w$";
-  const rdm_cc_t cc = RDM_CC_SET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_DMX_START_ADDRESS;
-  rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, format,
-                   &dmx_start_address, sizeof(dmx_start_address), ack);
-  if (ack != NULL) {
-    return ack->type == RDM_RESPONSE_TYPE_ACK;
-  } else {
-    rdm_header_t header;
-    return rdm_read_header(dmx_num, &header) &&
-           header.response_type == RDM_RESPONSE_TYPE_ACK;
-  }
+  const rdm_request_t request = {.dest_uid = dest_uid,
+                                 .sub_device = sub_device,
+                                 .cc = RDM_CC_SET_COMMAND,
+                                 .pid = RDM_PID_DMX_START_ADDRESS,
+                                 .format = "w$",
+                                 .pd = &dmx_start_address,
+                                 .pdl = sizeof(dmx_start_address)};
+
+  return rdm_send_request(dmx_num, &request, NULL, NULL, 0, ack);
 }

src/rdm/controller/include/utils.h

@@ -16,6 +16,20 @@
 extern "C" {
 #endif
 
+/**
+ * @brief Type for constructing an RDM request. Contains all the necessary
+ * information needed to address a request on the RDM bus.
+ */
+typedef struct rdm_request_t {
+  const rdm_uid_t *dest_uid;    // The destination UID of the request.
+  rdm_sub_device_t sub_device;  // The target sub-device of the request.
+  rdm_cc_t cc;                  // The command class of the request.
+  rdm_pid_t pid;                // The parameter ID.
+  const char *format;           // The format string for the parameter data.
+  const void *pd;  // A pointer to the parameter data of the request.
+  size_t pdl;      // The parameter data length of the request.
+} rdm_request_t;
+
 /**
  * @brief Sends an RDM controller request and processes the response. This
  * function writes, sends, receives, and reads a request and response RDM
@@ -51,24 +65,20 @@ extern "C" {
  * reason.
  *
  * @param dmx_num The DMX port number.
- * @param[in] dest_uid A pointer to the UID of the destination.
- * @param sub_device The sub-device number of the destination.
- * @param pid The parameter ID of the request.
- * @param cc The command class of the request.
- * @param[in] format The RDM parameter format string. More information about
- * RDM parameter format strings can be found in the documentation on the
- * rdm_read_pd() and rdm_write() functions.
- * @param[in] pd A pointer which stores parameter data to be included with the
- * RDM request.
- * @param pdl The size of the parameter data.
+ * @param[in] request A pointer to a request constructor.
+ * @param[in] format The RDM parameter format string for the response data. More
+ * information about RDM parameter format strings can be found in the
+ * documentation on the rdm_read_pd() and rdm_write() functions.
+ * @param[out] pd A pointer to an array which will store the parameter data
+ * received in the response. This value may be NULL if no data is expected.
+ * @param size The size of the pd array.
  * @param[out] ack A pointer to an rdm_ack_t which stores information about the
  * RDM response.
- * @return The number of bytes that were received in the response parameter
- * data.
+ * @return When an RDM_RESPONSE_TYPE_ACK response is received, the response PDL
+ * is returned or true if there is no parameter data received. 0 on failure.
  */
-size_t rdm_send_request(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
-                        rdm_sub_device_t sub_device, rdm_pid_t pid, rdm_cc_t cc,
-                        const char *format, const void *pd, size_t pdl,
+size_t rdm_send_request(dmx_port_t dmx_num, const rdm_request_t *request,
+                        const char *format, void *pd, size_t size,
                         rdm_ack_t *ack);
 
 /**

src/rdm/controller/product_info.c

@@ -17,15 +17,16 @@ size_t rdm_send_get_device_info(dmx_port_t dmx_num, const rdm_uid_t *dest_uid,
   DMX_CHECK(device_info != NULL, 0, "device_info is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const rdm_cc_t cc = RDM_CC_GET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_DEVICE_INFO;
-  size_t pdl = rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, NULL,
-                                NULL, 0, ack);
-  if (pdl == sizeof(*device_info)) {
-    const char *format = "x01x00wwdwbbwwb$";
-    rdm_read_pd(dmx_num, format, device_info, sizeof(*device_info));
-  }
-  return pdl;
+  const rdm_request_t request = {
+    .dest_uid = dest_uid,
+    .sub_device = sub_device,
+    .cc = RDM_CC_GET_COMMAND,
+    .pid = RDM_PID_DEVICE_INFO
+  };
+
+  const char *format = "x01x00wwdwbbwwb$";
+  return rdm_send_request(dmx_num, &request, format, &device_info,
+                          sizeof(*device_info), ack);
 }
 
 size_t rdm_send_get_software_version_label(dmx_port_t dmx_num,
@@ -41,13 +42,12 @@ size_t rdm_send_get_software_version_label(dmx_port_t dmx_num,
             "software_version_label is null");
   DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed");
 
-  const rdm_cc_t cc = RDM_CC_GET_COMMAND;
-  const rdm_pid_t pid = RDM_PID_SOFTWARE_VERSION_LABEL;
-  size_t pdl = rdm_send_request(dmx_num, dest_uid, sub_device, pid, cc, NULL,
-                                NULL, 0, ack);
-  if (pdl > 0) {
-    const char *format = "a$";
-    rdm_read_pd(dmx_num, format, software_version_label, size);
-  }
-  return pdl;
+  const rdm_request_t request = {.dest_uid = dest_uid,
+                                 .sub_device = sub_device,
+                                 .cc = RDM_CC_GET_COMMAND,
+                                 .pid = RDM_PID_SOFTWARE_VERSION_LABEL};
+
+  const char *format = "a$";
+  return rdm_send_request(dmx_num, &request, format, software_version_label,
+                          size, ack);
 }