Update vfs_manager & file_stream for out-of-order Universal Hex blocks.

When Universalh Hex was created two formats where specified, the
segment format, which is currently used due to faster parsing, and
the block format.

The block format was design so that each 512-bytes of hex
characters contain enough info to be self contained, however it
was never tested in a systen that sent blocks out-of-order.

The original vfs_manager expects all file blocks to arrive
sequentially in order. This update adds a check of "self contained
blocks" and a sticky flag that process these blocks whatever order
they arrive and ignores all other non-self-contained vfs sectors
received.

Author: microbit-carlos

source/daplink/drag-n-drop/file_stream.c

@@ -30,6 +30,16 @@
 #include "compiler.h"
 #include "validation.h"
 
+// Set to 1 to enable debugging
+#define DEBUG_FILE_STREAM     0
+
+#if DEBUG_FILE_STREAM
+#include "daplink_debug.h"
+#define stream_printf    debug_msg
+#else
+#define stream_printf(...)
+#endif
+
 typedef enum {
     STREAM_STATE_CLOSED,
     STREAM_STATE_OPEN,
@@ -75,9 +85,15 @@ static error_t open_hex(void *state);
 static error_t write_hex(void *state, const uint8_t *data, uint32_t size);
 static error_t close_hex(void *state);
 
-stream_t stream[] = {
+static bool detect_uhex_blocks(const uint8_t *data, uint32_t size);
+static error_t open_uhex_blocks(void *state);
+static error_t write_uhex_blocks(void *state, const uint8_t *data, uint32_t size);
+static error_t close_uhex_blocks(void *state);
+
+static stream_t stream[] = {
     {detect_bin, open_bin, write_bin, close_bin},   // STREAM_TYPE_BIN
     {detect_hex, open_hex, write_hex, close_hex},   // STREAM_TYPE_HEX
+    {detect_uhex_blocks, open_uhex_blocks, write_uhex_blocks, close_uhex_blocks},   // STREAM_TYPE_UHEX_BLOCKS
 };
 COMPILER_ASSERT(ARRAY_SIZE(stream) == STREAM_TYPE_COUNT);
 // STREAM_TYPE_NONE must not be included in count
@@ -104,6 +120,7 @@ stream_type_t stream_start_identify(const uint8_t *data, uint32_t size)
 
     for (i = STREAM_TYPE_START; i < STREAM_TYPE_COUNT; i++) {
         if (stream[i].detect(data, size)) {
+            stream_printf("file_stream start_identify stream=%i\r\n", i);
             return i;
         }
     }
@@ -118,12 +135,48 @@ stream_type_t stream_type_from_name(const vfs_filename_t filename)
     if (0 == strncmp("BIN", &filename[8], 3)) {
         return STREAM_TYPE_BIN;
     } else if (0 == strncmp("HEX", &filename[8], 3)) {
-        return STREAM_TYPE_HEX;
+        return STREAM_TYPE_HEX_OR_UHEX;
     } else {
         return STREAM_TYPE_NONE;
     }
 }
 
+bool stream_compatible(stream_type_t type_left, stream_type_t type_right)
+{
+    if (type_left == type_right) {
+        return true;
+    }
+
+    if ((type_left == STREAM_TYPE_HEX_OR_UHEX &&
+            (type_right == STREAM_TYPE_HEX || type_right == STREAM_TYPE_UHEX_BLOCKS)) ||
+        (type_right == STREAM_TYPE_HEX_OR_UHEX &&
+            (type_left == STREAM_TYPE_HEX || type_left == STREAM_TYPE_UHEX_BLOCKS))) {
+        return true;
+    }
+
+    return false;
+}
+
+bool stream_self_contained_block(stream_type_t type, const uint8_t *data, uint32_t size)
+{
+    switch (type) {
+        case STREAM_TYPE_BIN:
+            return false;
+
+        case STREAM_TYPE_HEX:
+            // A hex stream can also be a Universal Hex stream
+            return validate_uhex_block(data, size) ? true : false;
+
+        case STREAM_TYPE_UHEX_BLOCKS:
+            // The Universal Hex stream can be ordered (sectors) or unordered (blocks)
+            return validate_uhex_block(data, size) ? true : false;
+
+        default:
+            util_assert(0);
+            return false;
+    }
+}
+
 error_t stream_open(stream_type_t stream_type)
 {
     error_t status;
@@ -147,6 +200,7 @@ error_t stream_open(stream_type_t stream_type)
     current_stream = &stream[stream_type];
     // Initialize the specified stream
     status = current_stream->open(&shared_state);
+    stream_printf("file_stream stream_open(type=%d); open ret=%d\r\n", stream_type, status);
 
     if (ERROR_SUCCESS != status) {
         state = STREAM_STATE_ERROR;
@@ -170,6 +224,7 @@ error_t stream_write(const uint8_t *data, uint32_t size)
     stream_thread_assert();
     // Write to stream
     status = current_stream->write(&shared_state, data, size);
+    stream_printf("file_stream stream_write(size=%d); write ret=%d\r\n", size, status);
 
     if (ERROR_SUCCESS_DONE == status) {
         state = STREAM_STATE_END;
@@ -198,6 +253,7 @@ error_t stream_close(void)
     stream_thread_assert();
     // Close stream
     status = current_stream->close(&shared_state);
+    stream_printf("file_stream stream_close; close ret=%d\r\n", status);
     state = STREAM_STATE_CLOSED;
     return status;
 }
@@ -289,6 +345,13 @@ static error_t close_bin(void *state)
 
 static bool detect_hex(const uint8_t *data, uint32_t size)
 {
+    // Both Universal Hex formats will pass the normal hex file validation,
+    // but a Universal Hex in block format needs to be processed with the
+    // STREAM_TYPE_UHEX_BLOCKS stream.
+    // A Universal Hex in segment format can be be processed as a normal hex.
+    if (1 == validate_uhex_block(data, size)) {
+        return false;
+    }
     return 1 == validate_hexfile(data);
 }
 
@@ -315,6 +378,7 @@ static error_t write_hex(void *state, const uint8_t *data, uint32_t size)
     while (1) {
         // try to decode a block of hex data into bin data
         parse_status = parse_hex_blob(data, size, &block_amt_parsed, hex_state->bin_buffer, sizeof(hex_state->bin_buffer), &bin_start_address, &bin_buf_written);
+        stream_printf("file_stream write_hex; parse_hex_blob ret=%d, bin_buf_written=%d\r\n", parse_status, bin_buf_written);
 
         // the entire block of hex was decoded. This is a simple state
         if (HEX_PARSE_OK == parse_status) {
@@ -364,3 +428,35 @@ static error_t close_hex(void *state)
     status = flash_decoder_close();
     return status;
 }
+
+/* Universal Hex, block format, file processing */
+/* https://tech.microbit.org/software/spec-universal-hex/ */
+/* The Universal Hex segment format is processed by the Intel Hex parser. */
+/* This stream is for the Universal Hex block format only. */
+
+static bool detect_uhex_blocks(const uint8_t *data, uint32_t size)
+{
+    return 1 == validate_uhex_block(data, size);
+}
+
+static inline error_t open_uhex_blocks(void *state)
+{
+    return open_hex(state);
+}
+
+static inline error_t write_uhex_blocks(void *state, const uint8_t *data, uint32_t size)
+{
+    error_t status = write_hex(state, data, size);
+
+    // The block containing the EoF record could arrive at any point
+    if (ERROR_SUCCESS_DONE == status || ERROR_SUCCESS == status) {
+        status = ERROR_SUCCESS_DONE_OR_CONTINUE;
+    }
+
+    return status;
+}
+
+static inline error_t close_uhex_blocks(void *state)
+{
+    return close_hex(state);
+}

source/daplink/drag-n-drop/file_stream.h

@@ -36,11 +36,15 @@ typedef enum {
 
     STREAM_TYPE_BIN = STREAM_TYPE_START,
     STREAM_TYPE_HEX,
+    STREAM_TYPE_UHEX_BLOCKS,
 
     // Add new stream types here
 
     STREAM_TYPE_COUNT,
 
+    // Needed as both formats share the same .hex extension
+    STREAM_TYPE_HEX_OR_UHEX,
+
     STREAM_TYPE_NONE
 } stream_type_t;
 
@@ -50,6 +54,12 @@ stream_type_t stream_start_identify(const uint8_t *data, uint32_t size);
 // Stateless function to identify a filestream by its name
 stream_type_t stream_type_from_name(const vfs_filename_t filename);
 
+// Stateless function to identify if two streams are compatible
+bool stream_compatible(stream_type_t type_left, stream_type_t type_right);
+
+// Stateless function to identify if a stream can take blocks out of order
+bool stream_self_contained_block(stream_type_t type, const uint8_t *data, uint32_t size);
+
 error_t stream_open(stream_type_t stream_type);
 
 error_t stream_write(const uint8_t *data, uint32_t size);

source/daplink/drag-n-drop/flash_decoder.c

@@ -374,6 +374,9 @@ error_t flash_decoder_close(void)
     if ((DECODER_STATE_DONE != prev_state) &&
             (flash_type != FLASH_DECODER_TYPE_TARGET) &&
             (status == ERROR_SUCCESS)) {
+        flash_decoder_printf("    error: update incomplete\r\n");
+        flash_decoder_printf("    prev_state=%d; flash_type=%d; status=%d\r\n",
+                             prev_state, flash_type, status);
         status = ERROR_IAP_UPDT_INCOMPLETE;
     }
 

source/daplink/drag-n-drop/vfs_manager.c

@@ -85,6 +85,7 @@ typedef struct {
     bool stream_started;            // Stream processing started. This only gets reset remount
     bool stream_finished;           // Stream processing is done. This only gets reset remount
     bool stream_optional_finish;    // True if the stream processing can be considered done
+    bool stream_ooo_blocks;         // True if the stream data blocks can be processed out of order
     bool file_info_optional_finish; // True if the file transfer can be considered done
     bool transfer_timeout;          // Set if the transfer was finished because of a timeout. This only gets reset remount
     stream_type_t stream;           // Current stream or STREAM_TYPE_NONE is stream is closed.  This only gets reset remount
@@ -112,6 +113,7 @@ static const file_transfer_state_t default_transfer_state = {
     false,
     false,
     false,
+    false,
     STREAM_TYPE_NONE,
 };
 
@@ -439,27 +441,39 @@ static void file_change_handler(const vfs_filename_t filename, vfs_file_change_t
 static void file_data_handler(uint32_t sector, const uint8_t *buf, uint32_t num_of_sectors)
 {
     stream_type_t stream;
-    uint32_t size;
+    uint32_t size = VFS_SECTOR_SIZE * num_of_sectors;
 
     // this is the key for starting a file write - we dont care what file types are sent
     //  just look for something unique (NVIC table, hex, srec, etc) until root dir is updated
     if (!file_transfer_state.stream_started) {
         // look for file types we can program
-        stream = stream_start_identify((uint8_t *)buf, VFS_SECTOR_SIZE * num_of_sectors);
+        stream = stream_start_identify((uint8_t *)buf, size);
 
         if (STREAM_TYPE_NONE != stream) {
             transfer_stream_open(stream, sector);
         }
     }
 
     if (file_transfer_state.stream_started) {
-        // Ignore sectors coming before this file
-        if (sector < file_transfer_state.start_sector) {
-            return;
-        }
+        bool self_contained_block = stream_self_contained_block(file_transfer_state.stream, buf, size);
 
-        // sectors must be in order
-        if (sector != file_transfer_state.file_next_sector) {
+        if (self_contained_block) {
+            vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);
+            vfs_mngr_printf("    sector self-contained\r\n");
+            file_transfer_state.stream_ooo_blocks = true;
+        } else if (file_transfer_state.stream_ooo_blocks) {
+            // Invalid block for a self-contained stream, can be ignored
+            vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);
+            vfs_mngr_printf("    invalid data for self-contained stream\r\n");
+            vfs_mngr_printf("    size=%d data=%x,%x,%x,%x,...,%x,%x,%x,%x\r\n",
+                            size, buf[0], buf[1], buf[2], buf[3], buf[size - 4],
+                            buf[size - 3], buf[size - 2], buf[size - 1]);
+            return;
+        } else if (sector < file_transfer_state.start_sector) {
+            // For ordered streams, ignore sectors coming before this file
+            return;
+        } else if (sector != file_transfer_state.file_next_sector) {
+            // For ordered streams, sector must be the next in the sequence
             vfs_mngr_printf("vfs_manager file_data_handler sector=%i\r\n", sector);
 
             if (sector < file_transfer_state.file_next_sector) {
@@ -482,7 +496,6 @@ static void file_data_handler(uint32_t sector, const uint8_t *buf, uint32_t num_
         }
 
         // This sector could be part of the file so record it
-        size = VFS_SECTOR_SIZE * num_of_sectors;
         file_transfer_state.size_transferred += size;
         file_transfer_state.file_next_sector = sector + num_of_sectors;
 
@@ -584,15 +597,6 @@ static void transfer_update_file_info(vfs_file_t file, uint32_t start_sector, ui
         }
     }
 
-    // Initialize the stream if it has not been set
-    if (STREAM_TYPE_NONE == file_transfer_state.stream) {
-        file_transfer_state.stream = stream;
-
-        if (stream != STREAM_TYPE_NONE) {
-            vfs_mngr_printf("    stream=%i\r\n", stream);
-        }
-    }
-
     // Check - File size must either grow or be smaller than the size already transferred
     if ((size < file_transfer_state.file_size) && (size < file_transfer_state.size_transferred) && (size > 0)) {
         vfs_mngr_printf("    error: file size changed from %i to %i\r\n", file_transfer_state.file_size, size);
@@ -608,7 +612,7 @@ static void transfer_update_file_info(vfs_file_t file, uint32_t start_sector, ui
     }
 
     // Check - stream must be the same
-    if ((stream != STREAM_TYPE_NONE) && (stream != file_transfer_state.stream)) {
+    if (stream != STREAM_TYPE_NONE && file_transfer_state.stream != STREAM_TYPE_NONE && !stream_compatible(file_transfer_state.stream, stream)) {
         vfs_mngr_printf("    error: changed types during transfer from %i to %i\r\n", file_transfer_state.stream, stream);
         transfer_update_state(ERROR_ERROR_DURING_TRANSFER);
         return;
@@ -761,6 +765,12 @@ static void transfer_update_state(error_t status)
         (file_transfer_state.size_transferred >= file_transfer_state.file_size) &&
         (file_transfer_state.file_size > 0) &&
         (file_transfer_state.start_sector == file_transfer_state.file_start_sector);
+    if (file_transfer_state.stream_ooo_blocks &&
+            file_transfer_state.file_size > 0 &&
+            file_transfer_state.size_transferred >= file_transfer_state.file_size) {
+        vfs_mngr_printf("    OoO file_info_optional_finish=%d\r\n", file_transfer_state.file_info_optional_finish);
+        file_transfer_state.file_info_optional_finish = true;
+    }
     transfer_timeout = file_transfer_state.transfer_timeout;
     transfer_started = (VFS_FILE_INVALID != file_transfer_state.file_to_program) ||
                        (STREAM_TYPE_NONE != file_transfer_state.stream);

source/daplink/validation.c

@@ -94,3 +94,12 @@ uint8_t validate_hexfile(const uint8_t *buf)
         return ((buf[0] == ':') && ((buf[8] == '0') || (buf[8] == '2') || (buf[8] == '3') || (buf[8] == '4') || (buf[8] == '5'))) ? 1 : 0;
     }
 }
+
+uint8_t validate_uhex_block(const uint8_t *buf, uint32_t size) {
+    if (size != 512) {
+        return 0;
+    }
+    return (memcmp(buf, (const void *)":02000004", 9) == 0) &&
+            (memcmp(buf + 16, (const void *)":0400000A", 9) == 0) &&
+            (buf[511] == '\n');
+}

source/daplink/validation.h

@@ -30,6 +30,7 @@ extern "C" {
 
 uint8_t validate_bin_nvic(const uint8_t *buf);
 uint8_t validate_hexfile(const uint8_t *buf);
+uint8_t validate_uhex_block(const uint8_t *buf, uint32_t size);
 
 /*!
  * @brief Baseline implementation of NVIC validator.