ota: optionally send extended reply data for each uploaded fw chunk

docs/index.rst

@@ -468,14 +468,16 @@ Request to initiate a firmware update passing the full firmware image.
             "fwsize": 926448,
             "cmpsize": 579204,
             "cmphash": <32 bytes>,
-            "fwhash": <32 bytes>
+            "fwhash": <32 bytes>,
+            "extended_replies": false
         }
     }
 
 * 'fwsize' is the total length of the final firmware when uncompressed.
 * 'cmpsize' is the length of the compressed firmware image which will be uploaded.
 * 'cmphash' is the sha256 hash of the compressed firmware image.
 * 'fwhash' is the sha256 hash of the final firmware image to be booted.
+* 'extended_replies' is optional, and if set enables rich progress replies (see ota_data_reply_).
 * NOTE: 'fwhash' is a new addition and is optional at this time, although it will become mandatory in a future release.
 
 .. _ota_reply:
@@ -490,6 +492,7 @@ ota reply
         "result": true
     }
 
+
 After this reply is received, the compressed firmware is sent in chunks using 'ota_data' messages.  The chunks can be any size up to the `JADE_OTA_MAX_CHUNK` limit (see get_version_info_request_).
 
 .. _ota_delta_request:
@@ -508,14 +511,16 @@ Request to initiate a firmware update using a binary diff/patch to be applied on
             "fwsize": 926448,
             "patchsize": 987291,
             "cmpsize": 14006,
-            "cmphash": <32 bytes>
+            "cmphash": <32 bytes>,
+            "extended_replies": false
         }
     }
 
 * 'fwsize' is the total length of the final firmware when uncompressed.
 * 'patchsize' is the length of the patch when uncompressed.
 * 'cmpsize' is the length of the compressed firmware patch which will be uploaded.
 * 'cmphash' is the sha256 hash of the compressed firmware patch.
+* 'extended_replies' is optional, and if set enables rich progress replies (see ota_data_reply_).
 
 .. _ota_delta_reply:
 
@@ -549,6 +554,22 @@ ota_data reply
         "result": true
     }
 
+or, if 'extended_replies' was set in the original request, and the Jade fw supports this:
+
+.. code-block:: cbor
+
+    {
+        "id": "48",
+        "result": {
+            "confirmed": true
+            "progress": 58
+        }
+    }
+
+* 'confirmed' is initially false, until the user verifies the OTA upload, then all subsequent replies will be true
+* 'progress' is the percentage complete value as shown on Jade (which is based on uncompressed final firmware written, so can differ from the proportion of compressed data uploaded)
+NOTE: 'extended_replies' is supported as of Jade fw 1.0.34
+
 We then send the 'ota_complete' message to verify the OTA was successful (before the device reboots).
 
 .. _ota_complete_request:

jade_ota.py

@@ -248,7 +248,7 @@ def get_bleid(jade):
 # final (uncompressed) firmware, the length of the uncompressed diff/patch
 # (if this is a patch to apply to the current running firmware), and whether
 # to apply the test mnemonic rather than using normal pinserver authentication.
-def ota(jade, fwcompressed, fwlength, fwhash, patchlen=None, pushmnemonic=False):
+def ota(jade, fwcompressed, fwlength, fwhash, patchlen, extended_replies, pushmnemonic):
     info = jade.get_version_info()
     logger.info(f'Running OTA on: {info}')
     has_pin = info['JADE_HAS_PIN']
@@ -273,10 +273,15 @@ def ota(jade, fwcompressed, fwlength, fwhash, patchlen=None, pushmnemonic=False)
     last_written = 0
 
     # Callback to log progress
-    def _log_progress(written, compressed_size):
+    def _log_progress(written, compressed_size, extended_reply):
         nonlocal last_time
         nonlocal last_written
 
+        # For the purposes of this test we usually assume the hw supports
+        # extended replies.  Use --no-extended-reply if this is not the case
+        # (eg. updating from older firmware).
+        assert (extended_reply is not None) == (extended_replies is True)
+
         current_time = time.time()
         secs = current_time - last_time
         total_secs = current_time - start_time
@@ -287,13 +292,18 @@ def _log_progress(written, compressed_size):
         secs_remaining = (compressed_size - written) / avg_rate
         template = '{0:.2f} b/s - progress {1:.2f}% - {2:.2f} seconds left'
         logger.info(template.format(last_rate, progress, secs_remaining))
-        logger.info('Written {0}b in {1:.2f}s'.format(written, total_secs))
+        logger.info(f'Written {written}b in {total_secs}s')
+        if extended_reply:
+            logger.info(f'Confirmed: {extended_reply["confirmed"]}, ' +
+                        f'Write progress: {extended_reply["progress"]}%')
 
         last_time = current_time
         last_written = written
 
     result = jade.ota_update(fwcompressed, fwlength, chunksize, fwhash,
-                             patchlen=patchlen, cb=_log_progress, gcov_dump=info.get('GCOV', False))
+                             patchlen=patchlen, cb=_log_progress,
+                             extended_replies=extended_replies,
+                             gcov_dump=info.get('GCOV', False))
     assert result is True
 
     logger.info(f'Total ota time in secs: {time.time() - start_time}')
@@ -396,6 +406,11 @@ def _log_progress(written, compressed_size):
                         dest='pushmnemonic',
                         help='Sets a test mnemonic - only works with debug build of Jade',
                         default=False)
+    parser.add_argument('--no-extended-replies',
+                        action='store_true',
+                        dest='noextendedreplies',
+                        help='Do not request extended-replies (for backward-compatability)',
+                        default=False)
     parser.add_argument('--log',
                         action='store',
                         dest='loglevel',
@@ -479,7 +494,10 @@ def _log_progress(written, compressed_size):
         if not args.skipserial:
             logger.info(f'Jade OTA over serial')
             with JadeAPI.create_serial(device=args.serialport) as jade:
-                has_radio, bleid = ota(jade, fwcmp, fwlen, fwhash, patchlen, args.pushmnemonic)
+                # By default serial uses extended-replies
+                extended_replies = not args.noextendedreplies
+                has_radio, bleid = ota(jade, fwcmp, fwlen, fwhash, patchlen,
+                                       extended_replies, args.pushmnemonic)
 
         if not args.skipble:
             if has_radio and bleid is None and args.bleidfromserial:
@@ -490,7 +508,10 @@ def _log_progress(written, compressed_size):
             if has_radio:
                 logger.info(f'Jade OTA over BLE {bleid}')
                 with JadeAPI.create_ble(serial_number=bleid) as jade:
-                    ota(jade, fwcmp, fwlen, fwhash, patchlen, args.pushmnemonic)
+                    # Do not use extended-replies for ble
+                    extended_replies = False
+                    ota(jade, fwcmp, fwlen, fwhash, patchlen, extended_replies,
+                        args.pushmnemonic)
             else:
                 msg = 'Skipping BLE tests - not enabled on the hardware'
                 logger.warning(msg)

jadepy/jade.py

@@ -458,7 +458,7 @@ def logout(self):
         return self._jadeRpc('logout')
 
     def ota_update(self, fwcmp, fwlen, chunksize, fwhash=None, patchlen=None, cb=None,
-                   gcov_dump=False):
+                   extended_replies=False, gcov_dump=False):
         """
         RPC call to attempt to update the unit's firmware.
 
@@ -488,9 +488,15 @@ def ota_update(self, fwcmp, fwlen, chunksize, fwhash=None, patchlen=None, cb=Non
         cb : function, optional
             Callback function accepting two integers - the amount of compressed firmware sent thus
             far, and the total length of the compressed firmware to send.
+            If 'extended_replies' was set, this callback is also passed the extended data included
+            in the replies, if not this is None.
             If passed, this function is invoked each time a fw chunk is successfully uploaded and
             ack'd by the hw, to notify of upload progress.
             Defaults to None, and nothing is called to report upload progress.
+        extended_replies: bool, optional
+            If set Jade may return addtional progress data with each data chunk uploaded, which is
+            then passed to any progress callback as above.  If not no additional data is returned
+            or passed.
 
         Returns
         -------
@@ -509,7 +515,8 @@ def ota_update(self, fwcmp, fwlen, chunksize, fwhash=None, patchlen=None, cb=Non
         ota_method = 'ota'
         params = {'fwsize': fwlen,
                   'cmpsize': cmplen,
-                  'cmphash': cmphash}
+                  'cmphash': cmphash,
+                  'extended_replies': extended_replies}
 
         if fwhash is not None:
             params['fwhash'] = fwhash
@@ -528,11 +535,13 @@ def ota_update(self, fwcmp, fwlen, chunksize, fwhash=None, patchlen=None, cb=Non
             length = min(remaining, chunksize)
             chunk = bytes(fwcmp[written:written + length])
             result = self._jadeRpc('ota_data', chunk)
-            assert result is True
             written += length
 
+            have_extended_reply = isinstance(result, dict)
+            assert result is True or (extended_replies and have_extended_reply)
+
             if (cb):
-                cb(written, cmplen)
+                cb(written, cmplen, result if have_extended_reply else None)
 
         if gcov_dump:
             self.run_remote_gcov_dump()

main/process/ota.c

@@ -108,6 +108,10 @@ void ota_process(void* process_ptr)
         goto cleanup;
     }
 
+    // Optional field indicating preference for rich reply data
+    bool extended_replies = false;
+    rpc_get_boolean("extended_replies", &params, &extended_replies);
+
     // Can accept either uploaded file data hash (legacy) or hash of the full/final firmware image (preferred)
     uint8_t expected_hash[SHA256_LEN];
     char* expected_hash_hexstr = NULL;
@@ -151,6 +155,7 @@ void ota_process(void* process_ptr)
         .firmwaresize = firmwaresize,
         .expected_hash_hexstr = expected_hash_hexstr,
         .expected_hash = expected_hash,
+        .extended_replies = extended_replies,
     };
 
     if (!ota_init(&joctx)) {

main/process/ota_delta.c

@@ -179,6 +179,10 @@ void ota_delta_process(void* process_ptr)
         goto cleanup;
     }
 
+    // Optional field indicating preference for rich reply data
+    bool extended_replies = false;
+    rpc_get_boolean("extended_replies", &params, &extended_replies);
+
     // Can accept either uploaded file data hash (legacy) or hash of the full/final firmware image (preferred)
     uint8_t expected_hash[SHA256_LEN];
     char* expected_hash_hexstr = NULL;
@@ -223,6 +227,7 @@ void ota_delta_process(void* process_ptr)
         .compressedsize = compressedsize,
         .expected_hash_hexstr = expected_hash_hexstr,
         .expected_hash = expected_hash,
+        .extended_replies = extended_replies,
     };
 
     int ret = deflate_init_read_uncompressed(dctx, compressedsize, compressed_stream_reader, &joctx);

main/process/ota_util.c

@@ -20,13 +20,30 @@ extern esp_app_desc_t running_app_info;
 const __attribute__((section(".rodata_custom_desc"))) esp_custom_app_desc_t custom_app_desc
     = { .version = 1, .board_type = JADE_OTA_BOARD_TYPE, .features = JADE_OTA_FEATURES, .config = JADE_OTA_CONFIG };
 
-static void send_ok(const char* id, const jade_msg_source_t source)
+static void reply_ok(const void* ctx, CborEncoder* container)
 {
-    JADE_ASSERT(id);
+    JADE_ASSERT(ctx);
 
-    uint8_t ok_msg[MAXLEN_ID + 10];
-    bool ok = true;
-    jade_process_reply_to_message_result_with_id(id, ok_msg, sizeof(ok_msg), source, &ok, cbor_result_boolean_cb);
+    const jade_ota_ctx_t* joctx = (const jade_ota_ctx_t*)ctx;
+
+    if (joctx->extended_replies) {
+        // Extended/structured response
+        JADE_LOGI("Sending extended reply ok for %s", joctx->id);
+        CborEncoder map_encoder; // result data
+        CborError cberr = cbor_encoder_create_map(container, &map_encoder, 2);
+        JADE_ASSERT(cberr == CborNoError);
+
+        add_boolean_to_map(&map_encoder, "confirmed", *joctx->validated_confirmed);
+        add_uint_to_map(&map_encoder, "progress", joctx->progress_bar.percent_last_value);
+
+        cberr = cbor_encoder_close_container(container, &map_encoder);
+        JADE_ASSERT(cberr == CborNoError);
+    } else {
+        // Simple 'true' response
+        JADE_LOGI("Sending simple ok for %s", joctx->id);
+        const CborError cberr = cbor_encode_boolean(container, true);
+        JADE_ASSERT(cberr == CborNoError);
+    }
 }
 
 void handle_in_bin_data(void* ctx, uint8_t* data, const size_t rawsize)
@@ -101,8 +118,9 @@ void handle_in_bin_data(void* ctx, uint8_t* data, const size_t rawsize)
         joctx->uncompressedsize - *joctx->remaining_uncompressed);
 
     // Send ack after all processing - see comment above.
-    JADE_LOGI("Sending ok for %s", joctx->id);
-    send_ok(joctx->id, *joctx->expected_source);
+    uint8_t reply_msg[64];
+    jade_process_reply_to_message_result_with_id(
+        joctx->id, reply_msg, sizeof(reply_msg), *joctx->expected_source, joctx, reply_ok);
 
     // Blank out the current msg id once 'ok' is sent for it
     joctx->id[0] = '\0';

main/process/ota_util.h

@@ -39,6 +39,7 @@ typedef struct {
     mbedtls_sha256_context* sha_ctx;
     hash_type_t hash_type;
     char* id;
+    bool extended_replies;
     const uint8_t* expected_hash;
     const char* expected_hash_hexstr;
     const esp_partition_t* running_partition;

update_jade_fw.py

@@ -176,10 +176,12 @@ def ota(jade, verinfo, fwcompressed, fwlength, fwhash, patchlen=None):
     last_written = 0
 
     # Callback to log progress
-    def _log_progress(written, compressed_size):
+    def _log_progress(written, compressed_size, extended_reply):
         nonlocal last_time
         nonlocal last_written
 
+        assert extended_reply is None
+
         current_time = time.time()
         secs = current_time - last_time
         total_secs = current_time - start_time
@@ -198,7 +200,7 @@ def _log_progress(written, compressed_size):
     print('Please approve the firmware update on the Jade device')
     try:
         result = jade.ota_update(fwcompressed, fwlength, chunksize, fwhash,
-                                 patchlen=patchlen, cb=_log_progress)
+                                 patchlen=patchlen, cb=_log_progress, extended_replies=False)
         assert result is True
         print(f'Total OTA time: {time.time() - start_time}s')
     except JadeError as err: