From ed34948f26c83da3907f229d73b4c2974303f640 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Mon, 6 Feb 2023 20:22:00 -0800 Subject: [PATCH 01/15] rename var for clarity --- src/esp_dmx.c | 18 +++++++++--------- src/private/driver.h | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index f391dde9b..eccae0c4b 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -95,7 +95,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // DMX Receive #################################################### if (intr_flags & DMX_INTR_RX_ERR) { - if (!driver->received_a_packet) { + if (!driver->received_eop) { // Read data from the FIFO into the driver buffer if possible if (driver->data.head >= 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { // Data can be read into driver buffer @@ -116,7 +116,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // Set driver flags driver->is_in_break = false; driver->data.timestamp = now; - driver->received_a_packet = true; + driver->received_eop = true; driver->data.type = RDM_PACKET_TYPE_NON_RDM; driver->data.err = intr_flags & DMX_INTR_RX_FRAMING_ERR ? ESP_FAIL @@ -148,7 +148,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { driver->timer_idx, 0); #endif - if (!driver->received_a_packet && driver->data.head > 0 && + if (!driver->received_eop && driver->data.head > 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { // When a DMX break is received before the driver thinks a packet is // finished, the data.rx_size must be updated. @@ -158,7 +158,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { taskENTER_CRITICAL_ISR(spinlock); // Set driver flags driver->is_in_break = true; - driver->received_a_packet = false; + driver->received_eop = false; driver->data.head = 0; // Driver buffer is ready for data taskEXIT_CRITICAL_ISR(spinlock); } @@ -171,7 +171,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { uint8_t *data_ptr = &driver->data.buffer[driver->data.head]; dmx_uart_read_rxfifo(uart, data_ptr, &read_len); driver->data.head += read_len; - if (driver->received_a_packet) { + if (driver->received_eop) { // Update expected size if already sent a packet notification driver->data.rx_size = driver->data.head; } @@ -201,7 +201,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // Determine if a complete packet has been received bool packet_is_complete = false; - if (!driver->received_a_packet) { + if (!driver->received_eop) { const rdm_data_t *const rdm = (rdm_data_t *)driver->data.buffer; if (rdm->sc == RDM_SC && rdm->sub_sc == RDM_SUB_SC) { // The packet is a standard RDM packet @@ -255,7 +255,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { if (packet_is_complete) { taskENTER_CRITICAL_ISR(spinlock); driver->data.err = ESP_OK; - driver->received_a_packet = true; + driver->received_eop = true; driver->data.sent_last = false; if (driver->task_waiting) { xTaskNotifyFromISR(driver->task_waiting, driver->data.head, @@ -305,7 +305,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { } taskENTER_CRITICAL_ISR(spinlock); if (expecting_response) { - driver->received_a_packet = false; + driver->received_eop = false; dmx_uart_rxfifo_reset(uart); dmx_uart_set_rts(uart, 1); dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); @@ -463,7 +463,7 @@ esp_err_t dmx_driver_install(dmx_port_t dmx_num, int intr_flags) { // Initialize driver flags driver->is_in_break = false; - driver->received_a_packet = false; + driver->received_eop = false; driver->is_sending = false; driver->rdm.uid = 0; diff --git a/src/private/driver.h b/src/private/driver.h index d897bba33..850e498ed 100644 --- a/src/private/driver.h +++ b/src/private/driver.h @@ -54,9 +54,9 @@ typedef __attribute__((aligned(4))) struct dmx_driver_t { esp_err_t err; // The error state of the received DMX data. } data; - int is_in_break; // True if the driver is sending or receiving a DMX break. - int received_a_packet; // True if the driver is receiving data. - int is_sending; // True if the driver is sending data. + int is_in_break; // True if the driver is sending or receiving a DMX break. + int received_eop; // True if the driver received an end-of-packet condition. When this is true, the driver doesn't check for an end-of-packet condition when it receives DMX data. + int is_sending; // True if the driver is sending data. TaskHandle_t task_waiting; // The handle to a task that is waiting for data to be sent or received. SemaphoreHandle_t mux; // The handle to the driver mutex which allows multi-threaded driver function calls. From 54eebcd91ba3e0078b2f5a9dfc9ff3d038a5dbf9 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Mon, 6 Feb 2023 21:36:57 -0800 Subject: [PATCH 02/15] clean uart rx isr codesmell --- src/esp_dmx.c | 125 ++++++++++++++++++-------------------------------- 1 file changed, 45 insertions(+), 80 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index eccae0c4b..991aea1ee 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -94,33 +94,50 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { if (intr_flags == 0) break; // DMX Receive #################################################### - if (intr_flags & DMX_INTR_RX_ERR) { - if (!driver->received_eop) { - // Read data from the FIFO into the driver buffer if possible - if (driver->data.head >= 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { - // Data can be read into driver buffer - int read_len = DMX_MAX_PACKET_SIZE - driver->data.head; - uint8_t *data_ptr = &driver->data.buffer[driver->data.head]; - dmx_uart_read_rxfifo(uart, data_ptr, &read_len); - driver->data.head += read_len; - } else { - // Data cannot be read into driver buffer - if (driver->data.head > 0) { - // Only increment head if data has already been read into the buffer - driver->data.head += dmx_uart_get_rxfifo_len(uart); - } - dmx_uart_rxfifo_reset(uart); + if (intr_flags & DMX_INTR_RX_ALL) { + // Stop the RDM receive alarm (which may or may not be running) +#if ESP_IDF_VERSION_MAJOR >= 5 + gptimer_stop(driver->gptimer_handle); +#else + timer_group_set_counter_enable_in_isr(driver->timer_group, + driver->timer_idx, 0); +#endif + + // Read data into the DMX buffer if there is enough space + if (driver->data.head >= 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { + int read_len = DMX_MAX_PACKET_SIZE - driver->data.head; + if (intr_flags & DMX_INTR_RX_BREAK) --read_len; // Ignore DMX breaks + + uint8_t *current_slot = &driver->data.buffer[driver->data.head]; + dmx_uart_read_rxfifo(uart, current_slot, &read_len); + driver->data.head += read_len; + } else { + if (driver->data.head > 0) { + // Record the number of slots received for error reporting + driver->data.head += dmx_uart_get_rxfifo_len(uart); } + dmx_uart_rxfifo_reset(uart); + } + + // Set data timestamp and DMX break flag + taskENTER_CRITICAL_ISR(spinlock); + driver->data.timestamp = now; + driver->is_in_break = (intr_flags & DMX_INTR_RX_BREAK); + taskEXIT_CRITICAL_ISR(spinlock); + dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); + } + + if (intr_flags & DMX_INTR_RX_ERR) { + if (!driver->received_eop) { + // Receiving an error is an end-of-packet condition taskENTER_CRITICAL_ISR(spinlock); // Set driver flags - driver->is_in_break = false; - driver->data.timestamp = now; driver->received_eop = true; driver->data.type = RDM_PACKET_TYPE_NON_RDM; driver->data.err = intr_flags & DMX_INTR_RX_FRAMING_ERR - ? ESP_FAIL - : ESP_ERR_NOT_FINISHED; + ? ESP_FAIL // Slot is missing stop bits + : ESP_ERR_NOT_FINISHED; // UART overflow // Notify the task if there is one waiting if (driver->task_waiting) { @@ -128,77 +145,25 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { eSetValueWithOverwrite, &task_awoken); } taskEXIT_CRITICAL_ISR(spinlock); - } else { - // Packet has already been received - don't process errors - dmx_uart_rxfifo_reset(uart); } - dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ERR); } - + else if (intr_flags & DMX_INTR_RX_BREAK) { - // Reset the FIFO and clear the interrupt - dmx_uart_rxfifo_reset(uart); - dmx_uart_clear_interrupt(uart, DMX_INTR_RX_BREAK | DMX_INTR_RX_DATA); - - // Pause the receive timer alarm -#if ESP_IDF_VERSION_MAJOR >= 5 - gptimer_stop(driver->gptimer_handle); -#else - timer_group_set_counter_enable_in_isr(driver->timer_group, - driver->timer_idx, 0); -#endif + taskENTER_CRITICAL_ISR(spinlock); + // Set driver flags + driver->received_eop = false; + driver->data.head = 0; // Driver is ready for data + taskEXIT_CRITICAL_ISR(spinlock); + // Update rx_size when a valid packet with an atypical size is received if (!driver->received_eop && driver->data.head > 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { - // When a DMX break is received before the driver thinks a packet is - // finished, the data.rx_size must be updated. driver->data.rx_size = driver->data.head; } - taskENTER_CRITICAL_ISR(spinlock); - // Set driver flags - driver->is_in_break = true; - driver->received_eop = false; - driver->data.head = 0; // Driver buffer is ready for data - taskEXIT_CRITICAL_ISR(spinlock); } - + else if (intr_flags & DMX_INTR_RX_DATA) { - // Read data from the FIFO into the driver buffer if possible - if (driver->data.head >= 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { - // Data can be read into driver buffer - int read_len = DMX_MAX_PACKET_SIZE - driver->data.head; - uint8_t *data_ptr = &driver->data.buffer[driver->data.head]; - dmx_uart_read_rxfifo(uart, data_ptr, &read_len); - driver->data.head += read_len; - if (driver->received_eop) { - // Update expected size if already sent a packet notification - driver->data.rx_size = driver->data.head; - } - } else { - // Data cannot be read into driver buffer - if (driver->data.head > 0) { - // Only increment head if data has already been read into the buffer - driver->data.head += dmx_uart_get_rxfifo_len(uart); - } - dmx_uart_rxfifo_reset(uart); - } - dmx_uart_clear_interrupt(uart, DMX_INTR_RX_DATA); - - // Pause the receive timer alarm -#if ESP_IDF_VERSION_MAJOR >= 5 - gptimer_stop(driver->gptimer_handle); -#else - timer_group_set_counter_enable_in_isr(driver->timer_group, - driver->timer_idx, 0); -#endif - - // Set driver flags - taskENTER_CRITICAL_ISR(spinlock); - driver->is_in_break = false; - driver->data.timestamp = now; - taskEXIT_CRITICAL_ISR(spinlock); - // Determine if a complete packet has been received bool packet_is_complete = false; if (!driver->received_eop) { From 691a33f2ede0f34722455ba8888c1e0ebeb210f7 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 19:48:53 -0800 Subject: [PATCH 03/15] refactor rx logic --- src/esp_dmx.c | 92 +++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 991aea1ee..f9bb837c6 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -104,39 +104,62 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { #endif // Read data into the DMX buffer if there is enough space + const bool is_in_break = intr_flags & DMX_INTR_RX_BREAK; if (driver->data.head >= 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { int read_len = DMX_MAX_PACKET_SIZE - driver->data.head; - if (intr_flags & DMX_INTR_RX_BREAK) --read_len; // Ignore DMX breaks - + if (is_in_break) --read_len; // Ignore DMX breaks + + // Read the UART data into the DMX buffer and increment the head uint8_t *current_slot = &driver->data.buffer[driver->data.head]; dmx_uart_read_rxfifo(uart, current_slot, &read_len); driver->data.head += read_len; + + // Handle receiving a valid packet with larger than expected size + if (driver->data.head > driver->data.rx_size) { + driver->data.rx_size = driver->data.head; + } + } else { + // Record the number of slots received for error reporting if (driver->data.head > 0) { - // Record the number of slots received for error reporting driver->data.head += dmx_uart_get_rxfifo_len(uart); } dmx_uart_rxfifo_reset(uart); } + // Handle DMX break condition + if (is_in_break) { + // Handle receiveing a valid packet with smaller than expected size + if (!driver->received_eop && driver->data.head > 0 && + driver->data.head < DMX_MAX_PACKET_SIZE) { + driver->data.rx_size = driver->data.head; + } + + // Set driver flags + taskENTER_CRITICAL_ISR(spinlock); + driver->received_eop = false; // A new packet is being received + driver->data.head = 0; // Driver is ready for data + taskEXIT_CRITICAL_ISR(spinlock); + } + // Set data timestamp and DMX break flag taskENTER_CRITICAL_ISR(spinlock); driver->data.timestamp = now; - driver->is_in_break = (intr_flags & DMX_INTR_RX_BREAK); + driver->is_in_break = is_in_break; taskEXIT_CRITICAL_ISR(spinlock); dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); - } - if (intr_flags & DMX_INTR_RX_ERR) { - if (!driver->received_eop) { - // Receiving an error is an end-of-packet condition + if (driver->received_eop) { + continue; + } + + if (intr_flags & DMX_INTR_RX_ERR) { taskENTER_CRITICAL_ISR(spinlock); - // Set driver flags driver->received_eop = true; driver->data.type = RDM_PACKET_TYPE_NON_RDM; driver->data.err = intr_flags & DMX_INTR_RX_FRAMING_ERR - ? ESP_FAIL // Slot is missing stop bits + ? ESP_FAIL // Missing stop bits : ESP_ERR_NOT_FINISHED; // UART overflow // Notify the task if there is one waiting @@ -145,28 +168,9 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { eSetValueWithOverwrite, &task_awoken); } taskEXIT_CRITICAL_ISR(spinlock); - } - } - - else if (intr_flags & DMX_INTR_RX_BREAK) { - taskENTER_CRITICAL_ISR(spinlock); - // Set driver flags - driver->received_eop = false; - driver->data.head = 0; // Driver is ready for data - taskEXIT_CRITICAL_ISR(spinlock); - - // Update rx_size when a valid packet with an atypical size is received - if (!driver->received_eop && driver->data.head > 0 && - driver->data.head < DMX_MAX_PACKET_SIZE) { - driver->data.rx_size = driver->data.head; - } - - } - - else if (intr_flags & DMX_INTR_RX_DATA) { - // Determine if a complete packet has been received - bool packet_is_complete = false; - if (!driver->received_eop) { + } else if (intr_flags & DMX_INTR_RX_DATA) { + // Determine if a complete packet has been received + bool packet_is_complete = false; const rdm_data_t *const rdm = (rdm_data_t *)driver->data.buffer; if (rdm->sc == RDM_SC && rdm->sub_sc == RDM_SUB_SC) { // The packet is a standard RDM packet @@ -214,19 +218,19 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { packet_is_complete = true; } } - } - // Notify tasks that the packet is complete - if (packet_is_complete) { - taskENTER_CRITICAL_ISR(spinlock); - driver->data.err = ESP_OK; - driver->received_eop = true; - driver->data.sent_last = false; - if (driver->task_waiting) { - xTaskNotifyFromISR(driver->task_waiting, driver->data.head, - eSetValueWithOverwrite, &task_awoken); + // Notify tasks that the packet is complete + if (packet_is_complete) { + taskENTER_CRITICAL_ISR(spinlock); + driver->data.err = ESP_OK; + driver->received_eop = true; + driver->data.sent_last = false; + if (driver->task_waiting) { + xTaskNotifyFromISR(driver->task_waiting, driver->data.head, + eSetValueWithOverwrite, &task_awoken); + } + taskEXIT_CRITICAL_ISR(spinlock); } - taskEXIT_CRITICAL_ISR(spinlock); } } @@ -428,7 +432,7 @@ esp_err_t dmx_driver_install(dmx_port_t dmx_num, int intr_flags) { // Initialize driver flags driver->is_in_break = false; - driver->received_eop = false; + driver->received_eop = true; driver->is_sending = false; driver->rdm.uid = 0; From dd9bd0506e677f8e145856a9a8278f1636f580a4 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 20:47:21 -0800 Subject: [PATCH 04/15] rx refactors --- src/esp_dmx.c | 117 +++++++++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 64 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index f9bb837c6..64a77c70d 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -142,96 +142,85 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { taskEXIT_CRITICAL_ISR(spinlock); } - // Set data timestamp and DMX break flag + // Set data timestamp, DMX break flag, and clear interrupts taskENTER_CRITICAL_ISR(spinlock); driver->data.timestamp = now; driver->is_in_break = is_in_break; taskEXIT_CRITICAL_ISR(spinlock); - dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); + // Don't process data if end-of-packet condition already reached if (driver->received_eop) { continue; } + // Handle DMX errors or process DMX data + esp_err_t packet_err = ESP_OK; + rdm_response_type_t packet_type = RDM_PACKET_TYPE_NON_RDM; if (intr_flags & DMX_INTR_RX_ERR) { - taskENTER_CRITICAL_ISR(spinlock); - driver->received_eop = true; - driver->data.type = RDM_PACKET_TYPE_NON_RDM; - driver->data.err = intr_flags & DMX_INTR_RX_FRAMING_ERR - ? ESP_FAIL // Missing stop bits - : ESP_ERR_NOT_FINISHED; // UART overflow - - // Notify the task if there is one waiting - if (driver->task_waiting) { - xTaskNotifyFromISR(driver->task_waiting, driver->data.head, - eSetValueWithOverwrite, &task_awoken); - } - taskEXIT_CRITICAL_ISR(spinlock); + packet_err = intr_flags & DMX_INTR_RX_FRAMING_ERR + ? ESP_FAIL // Missing stop bits + : ESP_ERR_NOT_FINISHED; // UART overflow } else if (intr_flags & DMX_INTR_RX_DATA) { // Determine if a complete packet has been received - bool packet_is_complete = false; const rdm_data_t *const rdm = (rdm_data_t *)driver->data.buffer; + if (rdm->sc == RDM_SC && rdm->sub_sc == RDM_SUB_SC) { // The packet is a standard RDM packet - if (driver->data.head >= RDM_BASE_PACKET_SIZE && - driver->data.head >= rdm->message_len + 2) { - if (rdm->cc == RDM_CC_DISC_COMMAND && - rdm->pid == bswap16(RDM_PID_DISC_UNIQUE_BRANCH)) { - driver->data.type = RDM_PACKET_TYPE_DISCOVERY; - } else if (rdm_uid_is_broadcast(buf_to_uid(rdm->destination_uid))) { - driver->data.type = RDM_PACKET_TYPE_BROADCAST; - } else if (rdm->cc == RDM_CC_GET_COMMAND || - rdm->cc == RDM_CC_SET_COMMAND || - rdm->cc == RDM_CC_DISC_COMMAND) { - driver->data.type = RDM_PACKET_TYPE_REQUEST; - } else { - driver->data.type = RDM_PACKET_TYPE_RESPONSE; - } - packet_is_complete = true; + if (driver->data.head < RDM_BASE_PACKET_SIZE || + driver->data.head < rdm->message_len + 2) { + continue; // Guard against base packet size too small + } + + // Determine the packet type + if (rdm->cc == RDM_CC_DISC_COMMAND && + rdm->pid == bswap16(RDM_PID_DISC_UNIQUE_BRANCH)) { + packet_type = RDM_PACKET_TYPE_DISCOVERY; + } else if (rdm_uid_is_broadcast(buf_to_uid(rdm->destination_uid))) { + packet_type = RDM_PACKET_TYPE_BROADCAST; + } else if (rdm->cc == RDM_CC_GET_COMMAND || + rdm->cc == RDM_CC_SET_COMMAND || + rdm->cc == RDM_CC_DISC_COMMAND) { + packet_type = RDM_PACKET_TYPE_REQUEST; + } else { + packet_type = RDM_PACKET_TYPE_RESPONSE; } + } else if (rdm->sc == RDM_PREAMBLE || rdm->sc == RDM_DELIMITER) { // The packet is a DISC_UNIQUE_BRANCH response - if (driver->data.head >= 17) { - // Find the length of the preamble (0 to 7 bytes) - size_t preamble_len = 0; - for (; preamble_len < 7; ++preamble_len) { - if (driver->data.buffer[preamble_len] == RDM_DELIMITER) { - break; - } - } + if (driver->data.head < 17) { + continue; // Guard against base packet size too small + } - if (driver->data.head >= preamble_len + 17) { - // DISC_UNIQUE_BRANCH responses should be 17 bytes after preamble - taskENTER_CRITICAL_ISR(spinlock); - driver->data.type = RDM_PACKET_TYPE_DISCOVERY_RESPONSE; - taskEXIT_CRITICAL_ISR(spinlock); - packet_is_complete = true; + // Find the length of the preamble (0 to 7 bytes) + size_t preamble_len = 0; + for (; preamble_len < 7; ++preamble_len) { + if (driver->data.buffer[preamble_len] == RDM_DELIMITER) { + break; } } - } else { - // The packet is a DMX packet - if (driver->data.head >= driver->data.rx_size) { - taskENTER_CRITICAL_ISR(spinlock); - driver->data.type = RDM_PACKET_TYPE_NON_RDM; - taskEXIT_CRITICAL_ISR(spinlock); - packet_is_complete = true; + if (driver->data.buffer[preamble_len] != RDM_DELIMITER || + driver->data.head < preamble_len + 17) { + continue; // Delimiter not found or packet not complete yet } - } + packet_type = RDM_PACKET_TYPE_DISCOVERY_RESPONSE; - // Notify tasks that the packet is complete - if (packet_is_complete) { - taskENTER_CRITICAL_ISR(spinlock); - driver->data.err = ESP_OK; - driver->received_eop = true; - driver->data.sent_last = false; - if (driver->task_waiting) { - xTaskNotifyFromISR(driver->task_waiting, driver->data.head, - eSetValueWithOverwrite, &task_awoken); - } - taskEXIT_CRITICAL_ISR(spinlock); + } else if (driver->data.head < driver->data.rx_size) { + continue; // Guard against smaller DMX packets than expected } } + + // Set driver flags and notify task + taskENTER_CRITICAL_ISR(spinlock); + driver->received_eop = true; + driver->data.sent_last = false; + driver->data.type = packet_type; + driver->data.err = packet_err; + if (driver->task_waiting) { + xTaskNotifyFromISR(driver->task_waiting, driver->data.head, + eSetValueWithOverwrite, &task_awoken); + } + taskEXIT_CRITICAL_ISR(spinlock); } // DMX Transmit ##################################################### From a4587f77480b33871c817462c95bcf23e5daea20 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 20:47:46 -0800 Subject: [PATCH 05/15] formatting --- src/esp_dmx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 64a77c70d..f457b5795 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -113,7 +113,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { uint8_t *current_slot = &driver->data.buffer[driver->data.head]; dmx_uart_read_rxfifo(uart, current_slot, &read_len); driver->data.head += read_len; - + // Handle receiving a valid packet with larger than expected size if (driver->data.head > driver->data.rx_size) { driver->data.rx_size = driver->data.head; From 8ba78f40692d6ce3af310f0f3204988d21c483bc Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 20:52:31 -0800 Subject: [PATCH 06/15] rename var --- src/esp_dmx.c | 14 +++++++------- src/private/driver.h | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index f457b5795..33ab33fb7 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -130,15 +130,15 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // Handle DMX break condition if (is_in_break) { // Handle receiveing a valid packet with smaller than expected size - if (!driver->received_eop && driver->data.head > 0 && + if (!driver->end_of_packet && driver->data.head > 0 && driver->data.head < DMX_MAX_PACKET_SIZE) { driver->data.rx_size = driver->data.head; } // Set driver flags taskENTER_CRITICAL_ISR(spinlock); - driver->received_eop = false; // A new packet is being received - driver->data.head = 0; // Driver is ready for data + driver->end_of_packet = false; // A new packet is being received + driver->data.head = 0; // Driver is ready for data taskEXIT_CRITICAL_ISR(spinlock); } @@ -150,7 +150,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); // Don't process data if end-of-packet condition already reached - if (driver->received_eop) { + if (driver->end_of_packet) { continue; } @@ -212,7 +212,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // Set driver flags and notify task taskENTER_CRITICAL_ISR(spinlock); - driver->received_eop = true; + driver->end_of_packet = true; driver->data.sent_last = false; driver->data.type = packet_type; driver->data.err = packet_err; @@ -263,7 +263,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { } taskENTER_CRITICAL_ISR(spinlock); if (expecting_response) { - driver->received_eop = false; + driver->end_of_packet = false; dmx_uart_rxfifo_reset(uart); dmx_uart_set_rts(uart, 1); dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); @@ -421,7 +421,7 @@ esp_err_t dmx_driver_install(dmx_port_t dmx_num, int intr_flags) { // Initialize driver flags driver->is_in_break = false; - driver->received_eop = true; + driver->end_of_packet = true; driver->is_sending = false; driver->rdm.uid = 0; diff --git a/src/private/driver.h b/src/private/driver.h index 850e498ed..fa252a59e 100644 --- a/src/private/driver.h +++ b/src/private/driver.h @@ -54,9 +54,9 @@ typedef __attribute__((aligned(4))) struct dmx_driver_t { esp_err_t err; // The error state of the received DMX data. } data; - int is_in_break; // True if the driver is sending or receiving a DMX break. - int received_eop; // True if the driver received an end-of-packet condition. When this is true, the driver doesn't check for an end-of-packet condition when it receives DMX data. - int is_sending; // True if the driver is sending data. + int is_in_break; // True if the driver is sending or receiving a DMX break. + int end_of_packet; // True if the driver received an end-of-packet condition. + int is_sending; // True if the driver is sending data. TaskHandle_t task_waiting; // The handle to a task that is waiting for data to be sent or received. SemaphoreHandle_t mux; // The handle to the driver mutex which allows multi-threaded driver function calls. From 17a90c34dda696518514f5501f083026ee8e506d Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 20:56:54 -0800 Subject: [PATCH 07/15] minor critical section optimization --- src/esp_dmx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 33ab33fb7..7c156babf 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -261,15 +261,15 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { expecting_response = true; driver->data.head = -1; // Expecting a DMX break } - taskENTER_CRITICAL_ISR(spinlock); if (expecting_response) { + taskENTER_CRITICAL_ISR(spinlock); driver->end_of_packet = false; dmx_uart_rxfifo_reset(uart); dmx_uart_set_rts(uart, 1); dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); dmx_uart_enable_interrupt(uart, DMX_INTR_RX_ALL); + taskEXIT_CRITICAL_ISR(spinlock); } - taskEXIT_CRITICAL_ISR(spinlock); } } From 7b4a2e1c579c3a4a4808453cd7bd48979e3b2f8a Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 20:57:09 -0800 Subject: [PATCH 08/15] comments --- src/esp_dmx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 7c156babf..f4fb45156 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -172,7 +172,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { continue; // Guard against base packet size too small } - // Determine the packet type + // Determine the RDM packet type if (rdm->cc == RDM_CC_DISC_COMMAND && rdm->pid == bswap16(RDM_PID_DISC_UNIQUE_BRANCH)) { packet_type = RDM_PACKET_TYPE_DISCOVERY; From 2d667f6c0bff0f4810de984ea9414ddac2071add Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Tue, 7 Feb 2023 21:02:46 -0800 Subject: [PATCH 09/15] add code lifeline --- src/esp_dmx.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index f4fb45156..033a84dbd 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -208,6 +208,11 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { } else if (driver->data.head < driver->data.rx_size) { continue; // Guard against smaller DMX packets than expected } + } else { + // This code should never run, but can prevent crashes just in case! + dmx_uart_disable_interrupt(uart, ~(DMX_INTR_RX_ALL | DMX_INTR_TX_ALL)); + dmx_uart_clear_interrupt(uart, ~(DMX_INTR_RX_ALL | DMX_INTR_TX_ALL)); + continue; } // Set driver flags and notify task From 7dafa7ccacb238a36ef74a58d3ff7652e63047c7 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 10:04:35 -0800 Subject: [PATCH 10/15] remove extra calls to enable RX interrupts --- src/esp_dmx.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index f4135314e..60f1b1fc7 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -270,8 +270,6 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { driver->end_of_packet = false; dmx_uart_rxfifo_reset(uart); dmx_uart_set_rts(uart, 1); - dmx_uart_clear_interrupt(uart, DMX_INTR_RX_ALL); - dmx_uart_enable_interrupt(uart, DMX_INTR_RX_ALL); taskEXIT_CRITICAL_ISR(spinlock); } } @@ -950,7 +948,6 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, dmx_uart_disable_interrupt(uart, DMX_INTR_TX_ALL); xTaskNotifyStateClear(xTaskGetCurrentTaskHandle()); dmx_uart_set_rts(uart, 1); - dmx_uart_enable_interrupt(uart, DMX_INTR_RX_ALL); driver->data.head = -1; // Wait for DMX break before reading data } taskEXIT_CRITICAL(spinlock); From ee687b4b0dc30bcd45f57972fa0fbe54da7ecda2 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 11:15:20 -0800 Subject: [PATCH 11/15] receive optimizations --- src/esp_dmx.c | 39 ++++++++++++++++----------------------- src/private/driver.h | 1 + 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 60f1b1fc7..6874977e4 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -216,6 +216,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { // Set driver flags and notify task taskENTER_CRITICAL_ISR(spinlock); + driver->new_packet = true; driver->end_of_packet = true; driver->data.sent_last = false; driver->data.type = packet_type; @@ -268,6 +269,7 @@ static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { if (expecting_response) { taskENTER_CRITICAL_ISR(spinlock); driver->end_of_packet = false; + driver->new_packet = false; dmx_uart_rxfifo_reset(uart); dmx_uart_set_rts(uart, 1); taskEXIT_CRITICAL_ISR(spinlock); @@ -425,6 +427,7 @@ esp_err_t dmx_driver_install(dmx_port_t dmx_num, int intr_flags) { driver->is_in_break = false; driver->end_of_packet = true; driver->is_sending = false; + driver->new_packet = false; driver->rdm.uid = 0; driver->rdm.discovery_is_muted = false; @@ -911,13 +914,15 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, DMX_CHECK(dmx_num < DMX_NUM_MAX, 0, "dmx_num error"); DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed"); - spinlock_t *const restrict spinlock = &dmx_spinlock[dmx_num]; dmx_driver_t *const driver = dmx_driver[dmx_num]; + size_t packet_size = 0; - // Block until the mutex can be taken + // Block until mutex is taken and driver is idle, or until a timeout TimeOut_t timeout; vTaskSetTimeOutState(&timeout); if (!xSemaphoreTakeRecursive(driver->mux, wait_ticks) || + xTaskCheckForTimeOut(&timeout, &wait_ticks) || + !dmx_wait_sent(dmx_num, wait_ticks) || xTaskCheckForTimeOut(&timeout, &wait_ticks)) { if (packet != NULL) { packet->err = ESP_ERR_TIMEOUT; @@ -925,36 +930,23 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, packet->size = 0; packet->is_rdm = false; } - return 0; - } - - // Block until the driver is done sending - if (!dmx_wait_sent(dmx_num, wait_ticks) || - xTaskCheckForTimeOut(&timeout, &wait_ticks)) { - xSemaphoreGiveRecursive(driver->mux); - if (packet != NULL) { - packet->err = ESP_ERR_TIMEOUT; - packet->sc = -1; - packet->size = 0; - packet->is_rdm = false; - } - return 0; + return packet_size; } - // Set the RTS pin to read from the DMX bus + spinlock_t *const restrict spinlock = &dmx_spinlock[dmx_num]; uart_dev_t *const restrict uart = driver->uart; - taskENTER_CRITICAL(spinlock); + + // Set the RTS pin to enable reading from the DMX bus if (dmx_uart_get_rts(uart) == 0) { - dmx_uart_disable_interrupt(uart, DMX_INTR_TX_ALL); + taskENTER_CRITICAL(spinlock); xTaskNotifyStateClear(xTaskGetCurrentTaskHandle()); - dmx_uart_set_rts(uart, 1); driver->data.head = -1; // Wait for DMX break before reading data + driver->new_packet = false; + dmx_uart_set_rts(uart, 1); + taskEXIT_CRITICAL(spinlock); } - taskEXIT_CRITICAL(spinlock); - // Receive the latest data packet or check if the driver must wait esp_err_t err = ESP_OK; - uint32_t packet_size = 0; if (ulTaskNotifyTake(true, 0)) { taskENTER_CRITICAL(spinlock); packet_size = driver->data.head; @@ -967,6 +959,7 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, driver->task_waiting = xTaskGetCurrentTaskHandle(); } + // Get additional driver flags taskENTER_CRITICAL(spinlock); const bool driver_sent_last = driver->data.sent_last; diff --git a/src/private/driver.h b/src/private/driver.h index fa252a59e..347358b06 100644 --- a/src/private/driver.h +++ b/src/private/driver.h @@ -57,6 +57,7 @@ typedef __attribute__((aligned(4))) struct dmx_driver_t { int is_in_break; // True if the driver is sending or receiving a DMX break. int end_of_packet; // True if the driver received an end-of-packet condition. int is_sending; // True if the driver is sending data. + int new_packet; // True if the driver has a new, unhandled packet. TaskHandle_t task_waiting; // The handle to a task that is waiting for data to be sent or received. SemaphoreHandle_t mux; // The handle to the driver mutex which allows multi-threaded driver function calls. From dae216837b8b447ac501da4ac431fae356f09225 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 14:09:56 -0800 Subject: [PATCH 12/15] refactor dmx_receive() --- src/esp_dmx.c | 143 +++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 77 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index 6874977e4..e88f8ea74 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -914,22 +914,16 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, DMX_CHECK(dmx_num < DMX_NUM_MAX, 0, "dmx_num error"); DMX_CHECK(dmx_driver_is_installed(dmx_num), 0, "driver is not installed"); - dmx_driver_t *const driver = dmx_driver[dmx_num]; - size_t packet_size = 0; + dmx_driver_t *const restrict driver = dmx_driver[dmx_num]; - // Block until mutex is taken and driver is idle, or until a timeout - TimeOut_t timeout; - vTaskSetTimeOutState(&timeout); - if (!xSemaphoreTakeRecursive(driver->mux, wait_ticks) || - xTaskCheckForTimeOut(&timeout, &wait_ticks) || - !dmx_wait_sent(dmx_num, wait_ticks) || - xTaskCheckForTimeOut(&timeout, &wait_ticks)) { - if (packet != NULL) { - packet->err = ESP_ERR_TIMEOUT; - packet->sc = -1; - packet->size = 0; - packet->is_rdm = false; - } + // Set default return value and default values for output argument + uint32_t packet_size = 0; + if (packet != NULL) { + packet->err = ESP_ERR_TIMEOUT; + packet->sc = -1; + packet->size = 0; + packet->is_rdm = false; + } return packet_size; } @@ -946,40 +940,42 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, taskEXIT_CRITICAL(spinlock); } - esp_err_t err = ESP_OK; - if (ulTaskNotifyTake(true, 0)) { + // Wait for new DMX packet to be received + taskENTER_CRITICAL(spinlock); + const bool new_packet_available = driver->new_packet; + taskEXIT_CRITICAL(spinlock); + if (!new_packet_available && wait_ticks > 0) { + // Set task waiting and get additional DMX driver flags taskENTER_CRITICAL(spinlock); - packet_size = driver->data.head; - err = driver->data.err; - taskEXIT_CRITICAL(spinlock); - if (packet_size == -1) { - packet_size = 0; - } - } else { driver->task_waiting = xTaskGetCurrentTaskHandle(); - } - + const bool sent_last = driver->data.sent_last; + const enum rdm_packet_type_t data_type = driver->data.type; + taskEXIT_CRITICAL(spinlock); - // Get additional driver flags - taskENTER_CRITICAL(spinlock); - const bool driver_sent_last = driver->data.sent_last; - const enum rdm_packet_type_t data_type = driver->data.type; - taskEXIT_CRITICAL(spinlock); + // Check for early timeout according to RDM specification + if (sent_last && // FIXME: can this be one line? + data_type & (RDM_PACKET_TYPE_REQUEST | RDM_PACKET_TYPE_DISCOVERY)) { + taskENTER_CRITICAL(spinlock); + const int64_t last_timestamp = driver->data.timestamp; + taskEXIT_CRITICAL(spinlock); + + // Guard against setting hardware alarm durations with negative values + int64_t elapsed = esp_timer_get_time() - last_timestamp; + if (elapsed >= RDM_CONTROLLER_RESPONSE_LOST_TIMEOUT) { + xSemaphoreGiveRecursive(driver->mux); + return packet_size; + } - if (driver_sent_last && (data_type == RDM_PACKET_TYPE_REQUEST || - data_type == RDM_PACKET_TYPE_DISCOVERY)) { - // An RDM response is expected in <10ms so a hardware timer may be needed - taskENTER_CRITICAL(spinlock); - const int64_t elapsed = esp_timer_get_time() - driver->data.timestamp; - if (elapsed < RDM_CONTROLLER_RESPONSE_LOST_TIMEOUT) { - // Start a timer alarm that triggers when the RDM timeout occurs + // Set an early timeout with the hardware timer + taskENTER_CRITICAL(spinlock); #if ESP_IDF_VERSION_MAJOR >= 5 - gptimer_set_raw_count(driver->gptimer_handle, elapsed); + const gptimer_handle_t gptimer_handle = driver->gptimer_handle; const gptimer_alarm_config_t alarm_config = { .alarm_count = RDM_CONTROLLER_RESPONSE_LOST_TIMEOUT, .flags.auto_reload_on_alarm = false}; - gptimer_set_alarm_action(driver->gptimer_handle, &alarm_config); - gptimer_start(driver->gptimer_handle); + gptimer_set_raw_count(gptimer_handle, elapsed); + gptimer_set_alarm_action(gptimer_handle, &alarm_config); + gptimer_start(gptimer_handle); #else const timer_group_t timer_group = driver->timer_group; const timer_idx_t timer_idx = driver->timer_idx; @@ -988,55 +984,48 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, RDM_CONTROLLER_RESPONSE_LOST_TIMEOUT); timer_start(timer_group, timer_idx); #endif + taskEXIT_CRITICAL(spinlock); } - taskEXIT_CRITICAL(spinlock); - // Check if the response has already timed out - if (elapsed >= RDM_CONTROLLER_RESPONSE_LOST_TIMEOUT) { - driver->task_waiting = NULL; + // Wait for a task notification + const bool notified = xTaskNotifyWait(0, -1, &packet_size, wait_ticks); + taskENTER_CRITICAL(spinlock); + driver->task_waiting = NULL; + taskEXIT_CRITICAL(spinlock); + if (!notified) { +#if ESP_IDF_VERSION_MAJOR >= 5 + gptimer_stop(driver->gptimer_handle); +#else + timer_pause(driver->timer_group, driver->timer_idx); +#endif xTaskNotifyStateClear(xTaskGetCurrentTaskHandle()); xSemaphoreGiveRecursive(driver->mux); - if (packet != NULL) { - packet->err = ESP_ERR_TIMEOUT; - packet->sc = -1; - packet->size = 0; - packet->is_rdm = false; - } return packet_size; } - } - // Wait for a task notification - if (xTaskNotifyWait(0, ULONG_MAX, &packet_size, wait_ticks)) { - err = driver->data.err; - if (packet_size == -1) { - packet_size = 0; - } - } else { - err = ESP_ERR_TIMEOUT; + } else if (!new_packet_available) { + // Fail early if there is no data available and this function cannot block + xSemaphoreGiveRecursive(driver->mux); + return packet_size; } - driver->task_waiting = NULL; - // Report data in the DMX packet + // Parse DMX data packet if (packet != NULL) { - packet->err = err; - packet->size = packet_size; + taskENTER_CRITICAL(spinlock); + packet->err = driver->data.err; + packet_size = driver->data.head; if (packet_size > 0) { - bool is_rdm = false; - if (!err) { - // Quickly check if the packet is an RDM packet - const rdm_data_t *const rdm = (rdm_data_t *)driver->data.buffer; - taskENTER_CRITICAL(spinlock); - is_rdm = (rdm->sc == RDM_SC && rdm->sub_sc == RDM_SUB_SC) || - rdm->sc == RDM_PREAMBLE || rdm->sc == RDM_DELIMITER; - taskEXIT_CRITICAL(spinlock); - } + const rdm_data_t *const restrict rdm = (rdm_data_t *)driver->data.buffer; + packet->is_rdm = (rdm->sc == RDM_SC && rdm->sub_sc == RDM_SUB_SC) || + rdm->sc == RDM_PREAMBLE || rdm->sc == RDM_DELIMITER; packet->sc = driver->data.buffer[0]; - packet->is_rdm = is_rdm; - } else { - packet->sc = -1; - packet->is_rdm = false; } + driver->new_packet = false; + taskEXIT_CRITICAL(spinlock); + if (packet_size == -1) { + packet_size = 0; + } + packet->size = packet_size; } // Give the mutex back and return From df3413871f1657dae89e8e688cb6a67fef3a25fe Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 14:10:15 -0800 Subject: [PATCH 13/15] update TimeOut_t behavior on non-blocking --- src/esp_dmx.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index e88f8ea74..ef6706c6a 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -9,6 +9,7 @@ #include "esp_check.h" #include "esp_log.h" #include "esp_rdm.h" +#include "esp_task_wdt.h" #include "private/dmx_hal.h" #include "private/driver.h" #include "private/rdm_encode/types.h" @@ -924,6 +925,16 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, packet->size = 0; packet->is_rdm = false; } + + // Block until mutex is taken and driver is idle, or until a timeout + TimeOut_t timeout; + vTaskSetTimeOutState(&timeout); + if (!xSemaphoreTakeRecursive(driver->mux, wait_ticks) || + (wait_ticks && xTaskCheckForTimeOut(&timeout, &wait_ticks))) { + return packet_size; + } else if (!dmx_wait_sent(dmx_num, wait_ticks) || + (wait_ticks && xTaskCheckForTimeOut(&timeout, &wait_ticks))) { + xSemaphoreGiveRecursive(driver->mux); return packet_size; } @@ -1217,7 +1228,7 @@ bool dmx_wait_sent(dmx_port_t dmx_num, TickType_t wait_ticks) { TimeOut_t timeout; vTaskSetTimeOutState(&timeout); if (!xSemaphoreTakeRecursive(driver->mux, wait_ticks) || - xTaskCheckForTimeOut(&timeout, &wait_ticks)) { + (wait_ticks && xTaskCheckForTimeOut(&timeout, &wait_ticks))) { return false; } From ce9efc2d1772ce2f6880f16f5a3bf5dcd2754081 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 14:40:55 -0800 Subject: [PATCH 14/15] add constant --- src/esp_dmx.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/esp_dmx.c b/src/esp_dmx.c index ef6706c6a..259d0347a 100644 --- a/src/esp_dmx.c +++ b/src/esp_dmx.c @@ -74,12 +74,15 @@ enum rdm_packet_timing_t { }; enum rdm_packet_type_t { - RDM_PACKET_TYPE_NON_RDM, - RDM_PACKET_TYPE_DISCOVERY, - RDM_PACKET_TYPE_DISCOVERY_RESPONSE, - RDM_PACKET_TYPE_REQUEST, - RDM_PACKET_TYPE_RESPONSE, - RDM_PACKET_TYPE_BROADCAST + RDM_PACKET_TYPE_NON_RDM = 0, + RDM_PACKET_TYPE_DISCOVERY = BIT(0), + RDM_PACKET_TYPE_DISCOVERY_RESPONSE = BIT(1), + RDM_PACKET_TYPE_REQUEST = BIT(2), + RDM_PACKET_TYPE_RESPONSE = BIT(3), + RDM_PACKET_TYPE_BROADCAST = BIT(4), + + RDM_PACKET_TYPE_EARLY_TIMEOUT = + (RDM_PACKET_TYPE_REQUEST | RDM_PACKET_TYPE_DISCOVERY) }; static void DMX_ISR_ATTR dmx_uart_isr(void *arg) { @@ -964,8 +967,7 @@ size_t dmx_receive(dmx_port_t dmx_num, dmx_packet_t *packet, taskEXIT_CRITICAL(spinlock); // Check for early timeout according to RDM specification - if (sent_last && // FIXME: can this be one line? - data_type & (RDM_PACKET_TYPE_REQUEST | RDM_PACKET_TYPE_DISCOVERY)) { + if (sent_last && (data_type & RDM_PACKET_TYPE_EARLY_TIMEOUT)) { taskENTER_CRITICAL(spinlock); const int64_t last_timestamp = driver->data.timestamp; taskEXIT_CRITICAL(spinlock); From 1fa2d57411f9691907f12b5fb4d892681d4ac3b9 Mon Sep 17 00:00:00 2001 From: Mitch Weisbrod Date: Sun, 12 Feb 2023 14:41:00 -0800 Subject: [PATCH 15/15] update documentation --- src/esp_dmx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esp_dmx.h b/src/esp_dmx.h index 9bf3a57af..144872a5b 100644 --- a/src/esp_dmx.h +++ b/src/esp_dmx.h @@ -284,8 +284,8 @@ int dmx_write_slot(dmx_port_t dmx_num, size_t slot_num, uint8_t value); /** * @brief Receives a DMX packet from the DMX bus. This is a blocking function. * This function first blocks until the DMX driver is idle and then it blocks - * using a timeout until a packet is received. This function will timeout early - * according to RDM specification if an RDM packet is expected. + * using a timeout until a new packet is received. This function will timeout + * early according to RDM specification if an RDM packet is expected. * * @note This function uses FreeRTOS direct-to-task notifications to block and * unblock. Using task notifications on the same task that calls this function