From b723cf9ceeef9711664c9148e8718eed1b1c4cdd Mon Sep 17 00:00:00 2001 From: Andre Renaud Date: Mon, 5 Jun 2023 20:22:30 +1200 Subject: [PATCH 1/5] Added in support for original 8-bit checksum as well as 16-bit crc --- xmodem_server.c | 24 +++++++++++++++++------- xmodem_server.h | 6 ++++-- xmodem_server_test.c | 10 ++++------ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/xmodem_server.c b/xmodem_server.c index e5e2569..343d65c 100644 --- a/xmodem_server.c +++ b/xmodem_server.c @@ -9,6 +9,7 @@ #define XMODEM_ACK 0x06 #define XMODEM_NACK 0x15 #define XMODEM_CAN 0x18 +#define XMODEM_WANTCRC 'C' // How many milliseconds do we have to wait for a packet to arrive before // we send a NAK & restart the transfer @@ -95,8 +96,10 @@ bool xmodem_server_rx_byte(struct xmodem_server *xdm, uint8_t byte) { } case XMODEM_STATE_DATA: xdm->packet_data[xdm->packet_pos++] = byte; - if (xdm->packet_pos >= xdm->packet_size) - xdm->state = XMODEM_STATE_CRC0; + if (xdm->packet_pos >= xdm->packet_size) { + xdm->crc = 0; + xdm->state = xdm->crc_8_bit ? XMODEM_STATE_CRC1 : XMODEM_STATE_CRC0; + } break; case XMODEM_STATE_CRC0: @@ -107,8 +110,14 @@ bool xmodem_server_rx_byte(struct xmodem_server *xdm, uint8_t byte) { case XMODEM_STATE_CRC1: { uint16_t crc = 0; xdm->crc |= byte; - for (int i = 0; i < xdm->packet_size; i++) - crc = xmodem_server_crc(crc, xdm->packet_data[i]); + if (xdm->crc_8_bit) { + for (int i = 0; i < xdm->packet_size; i++) + crc += xdm->packet_data[i]; + crc &= 0xff; + } else { + for (int i = 0; i < xdm->packet_size; i++) + crc = xmodem_server_crc(crc, xdm->packet_data[i]); + } if (crc != xdm->crc) { xdm->error_count++; xdm->state = XMODEM_STATE_SOH; @@ -134,14 +143,15 @@ const char *xmodem_server_state_name(const struct xmodem_server *xdm) return state_name(xdm->state); } -int xmodem_server_init(struct xmodem_server *xdm, xmodem_tx_byte tx_byte, void *cb_data) { +int xmodem_server_init(struct xmodem_server *xdm, xmodem_tx_byte tx_byte, bool force_8_bit_checksum, void *cb_data) { if (!tx_byte) return -1; memset(xdm, 0, sizeof(*xdm)); xdm->tx_byte = tx_byte; xdm->cb_data = cb_data; + xdm->crc_8_bit = force_8_bit_checksum; - xdm->tx_byte(xdm, 'C', xdm->cb_data); + xdm->tx_byte(xdm, xdm->crc_8_bit ? XMODEM_NACK : XMODEM_WANTCRC, xdm->cb_data); return 0; } @@ -164,7 +174,7 @@ int xmodem_server_process(struct xmodem_server *xdm, uint8_t *packet, uint32_t * if (xdm->last_event_time == 0) xdm->last_event_time = ms_time; if (xdm->state == XMODEM_STATE_START && ms_time - xdm->last_event_time > 500) { - xdm->tx_byte(xdm, 'C', xdm->cb_data); + xdm->tx_byte(xdm, xdm->crc_8_bit ? XMODEM_NACK : XMODEM_WANTCRC, xdm->cb_data); xdm->last_event_time = ms_time; } if (ms_time - xdm->last_event_time > XMODEM_PACKET_TIMEOUT) { diff --git a/xmodem_server.h b/xmodem_server.h index 5f96792..0917a4d 100644 --- a/xmodem_server.h +++ b/xmodem_server.h @@ -60,7 +60,8 @@ struct xmodem_server { xmodem_server_state state; // What state are we in? uint8_t packet_data[XMODEM_MAX_PACKET_SIZE]; // Incoming packet data int packet_pos; // Where are we up to in this packet - uint16_t crc; // Whatis the expected CRC of the incoming packet + uint16_t crc; // What is the expected CRC of the incoming packet + bool crc_8_bit; // If using 128B packets, force the original 8-bit checksum, not 16-bit CRC uint16_t packet_size; // Are we receiving 128B or 1K packets? bool repeating; // Are we receiving a packet that we've already processed? int64_t last_event_time; // When did we last do something interesting? @@ -74,10 +75,11 @@ struct xmodem_server { * Initialise the internal xmodem server state * @param xdm Xmodem server state area to initialise * @param tx_byte callback to be called for ACK/NACK bytes + * @param force_8_bit_checksum if set, use original 8-bit checksum responses. If not set, use newer 16-bit CRC (XMODEM-CRC) * @param cb_data user-supplied pointer to be supplied to the tx_byte function * @return < 0 on failure, >= 0 on success */ -int xmodem_server_init(struct xmodem_server *xdm, xmodem_tx_byte tx_byte, void *cb_data); +int xmodem_server_init(struct xmodem_server *xdm, xmodem_tx_byte tx_byte, bool force_8_bit_checksum, void *cb_data); /** * Send a single byte to the xmodem state machine diff --git a/xmodem_server_test.c b/xmodem_server_test.c index ad1556e..bf8a317 100644 --- a/xmodem_server_test.c +++ b/xmodem_server_test.c @@ -52,7 +52,7 @@ static void test_simple(void) { struct xmodem_server xdm; uint8_t tx_char = 0; - TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, &tx_char) >= 0); + TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, false, &tx_char) >= 0); for (uint32_t i = 0; i < 5; i++) { uint8_t data[128]; uint8_t resp[128]; @@ -74,9 +74,8 @@ static void test_simple(void) { static void test_errors(void) { struct xmodem_server xdm; uint8_t tx_char = 0; - int attempts = 0; - TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, &tx_char) >= 0); + TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, false, &tx_char) >= 0); for (uint32_t i = 0; i < 5 && !xmodem_server_is_done(&xdm); ) { uint8_t data[1024]; uint8_t resp[1024]; @@ -92,7 +91,6 @@ static void test_errors(void) { TEST_ASSERT(block_nr == i); i++; } - attempts++; } xmodem_server_rx_byte(&xdm, 0x04); TEST_ASSERT(xmodem_server_get_state(&xdm) == XMODEM_STATE_SUCCESSFUL); @@ -102,7 +100,7 @@ static void test_errors(void) { static void test_timeout(void) { struct xmodem_server xdm; uint8_t tx_char = 0; - TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, &tx_char) >= 0); + TEST_ASSERT(xmodem_server_init(&xdm, tx_byte, false, &tx_char) >= 0); // Transmit 1 packet so we're not in the initial phase uint8_t data[XMODEM_MAX_PACKET_SIZE] = {0}; @@ -202,7 +200,7 @@ static void test_sz(bool use_1k, size_t data_size) { pid_t pid = spawn_process(args, &rd_fd, &wr_fd); uint32_t block_nr; TEST_ASSERT(pid >= 0); - TEST_ASSERT(xmodem_server_init(&xdm, tx_byte_fd, &wr_fd) >= 0); + TEST_ASSERT(xmodem_server_init(&xdm, tx_byte_fd, !use_1k, &wr_fd) >= 0); while (!xmodem_server_is_done(&xdm)) { fd_set rd_fds, wr_fds; From c899776944d176ca1d42845ca4bc4ca97e2498da Mon Sep 17 00:00:00 2001 From: Andre Renaud Date: Mon, 5 Jun 2023 20:37:30 +1200 Subject: [PATCH 2/5] Fix up README usage --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2d7fa39..209e9b2 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ if available ```c struct xmodem_server xdm; -xmodem_server_init(&xdm, uart_tx_char, NULL); +xmodem_server_init(&xdm, uart_tx_char, false, NULL); while (!xmodem_server_is_done(&xdm)) { uint8_t resp[XMODEM_MAX_PACKET_SIZE]; uint32_t block_nr; From 9474235d8895e03c98ca3fd0c87769eab2edf3e6 Mon Sep 17 00:00:00 2001 From: Andre Renaud Date: Mon, 5 Jun 2023 20:46:14 +1200 Subject: [PATCH 3/5] Stick more info in xml --- acutest.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/acutest.h b/acutest.h index 3644d40..3f6b2c5 100644 --- a/acutest.h +++ b/acutest.h @@ -1728,6 +1728,7 @@ main(int argc, char** argv) } if (test_xml_output_) { + double duration_total = 0; #if defined ACUTEST_UNIX_ char *suite_name = basename(argv[0]); #elif defined ACUTEST_WIN_ @@ -1737,8 +1738,11 @@ main(int argc, char** argv) const char *suite_name = argv[0]; #endif fprintf(test_xml_output_, "\n"); - fprintf(test_xml_output_, "\n", - suite_name, (int)test_list_size_, test_stat_failed_units_, test_stat_failed_units_, + for(i = 0; test_list_[i].func != NULL; i++) { + duration_total += test_details_[i].duration; + } + fprintf(test_xml_output_, "\n", + suite_name, (int)test_list_size_, duration_total, test_stat_failed_units_, test_stat_failed_units_, (int)test_list_size_ - test_stat_run_units_); for(i = 0; test_list_[i].func != NULL; i++) { struct test_detail_ *details = &test_details_[i]; From 3df2291cdc141b89f8e1b5001495f937f74ee630 Mon Sep 17 00:00:00 2001 From: Andre Renaud Date: Mon, 5 Jun 2023 20:48:44 +1200 Subject: [PATCH 4/5] bump action version --- .github/workflows/build_and_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_and_test.yaml b/.github/workflows/build_and_test.yaml index 3755dda..d25367d 100644 --- a/.github/workflows/build_and_test.yaml +++ b/.github/workflows/build_and_test.yaml @@ -20,7 +20,7 @@ jobs: make valgrind --leak-check=full --error-exitcode=1 ./xmodem_server_test --xml-output=test-results.xml - name: Publish Unit Test Results - uses: EnricoMi/publish-unit-test-result-action@v1.6 + uses: EnricoMi/publish-unit-test-result-action@v2 if: always() with: github_token: ${{ secrets.GITHUB_TOKEN }} From 7c00bd232f3f67df8a6f792bd703788b7ecb7640 Mon Sep 17 00:00:00 2001 From: Andre Renaud Date: Mon, 5 Jun 2023 21:09:50 +1200 Subject: [PATCH 5/5] Flip ordering for marginally better packing behaviour on some platforms --- xmodem_server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodem_server.h b/xmodem_server.h index 0917a4d..7fc53e6 100644 --- a/xmodem_server.h +++ b/xmodem_server.h @@ -61,8 +61,8 @@ struct xmodem_server { uint8_t packet_data[XMODEM_MAX_PACKET_SIZE]; // Incoming packet data int packet_pos; // Where are we up to in this packet uint16_t crc; // What is the expected CRC of the incoming packet - bool crc_8_bit; // If using 128B packets, force the original 8-bit checksum, not 16-bit CRC uint16_t packet_size; // Are we receiving 128B or 1K packets? + bool crc_8_bit; // If using 128B packets, force the original 8-bit checksum, not 16-bit CRC bool repeating; // Are we receiving a packet that we've already processed? int64_t last_event_time; // When did we last do something interesting? uint32_t block_num; // What block are we up to?