[ch1741] OTA'd module info sent as a payload in UpdateDone message or as an ACK to UpdateDone

communication/src/chunked_transfer.h

@@ -149,6 +149,9 @@ class ChunkedTransfer
 
 	void cancel();
 
+	size_t notify_update_done(Message& msg, Message& response, MessageChannel& channel, token_t token,
+							  uint8_t code);
+
 };
 
 }

communication/src/messages.cpp

@@ -132,15 +132,26 @@ size_t Messages::hello(uint8_t* buf, message_id_t message_id, uint8_t flags,
 	return len;
 }
 
-size_t Messages::update_done(uint8_t* buf, message_id_t message_id, bool confirmable)
+size_t Messages::update_done(uint8_t* buf, message_id_t message_id, const uint8_t* result, size_t result_len, bool confirmable)
 {
+	size_t sz = 6;
 	buf[0] = confirmable ? 0x40 : 0x50; // confirmable/non-confirmable, no token
 	buf[1] = 0x03; // PUT
 	buf[2] = message_id >> 8;
 	buf[3] = message_id & 0xff;
 	buf[4] = 0xb1; // Uri-Path option of length 1
 	buf[5] = 'u';
-	return 6;
+	if (result && result_len) {
+		buf[sz++] = 0xff; // payload marker
+		memcpy(buf + sz, result, result_len);
+		sz += result_len;
+	}
+	return sz;
+}
+
+size_t Messages::update_done(uint8_t* buf, message_id_t message_id, bool confirmable)
+{
+	return update_done(buf, message_id, NULL, 0, confirmable);
 }
 
 size_t Messages::function_return(unsigned char *buf, message_id_t message_id, token_t token, int return_value, bool confirmable)
@@ -319,6 +330,18 @@ size_t Messages::event(uint8_t buf[], uint16_t message_id, const char *event_nam
   return p - buf;
 }
 
-
+size_t Messages::coded_ack(uint8_t* buf, uint8_t token, uint8_t code,
+                           uint8_t message_id_msb, uint8_t message_id_lsb,
+                           uint8_t* data, size_t data_len)
+{
+    size_t sz = Messages::coded_ack(buf, token, code, message_id_msb, message_id_lsb);
+    if (data && data_len) {
+        buf[sz++] = 0xff; // Payload marker
+        memcpy(buf + sz, data, data_len);
+        sz += data_len;
+    }
+
+    return sz;
+}
 
 }}

communication/src/messages.h

@@ -53,6 +53,7 @@ class Messages
 			uint16_t product_firmware_version, bool confirmable, const uint8_t* device_id, uint16_t device_id_len);
 
 	static size_t update_done(uint8_t* buf, message_id_t message_id, bool confirmable);
+  static size_t update_done(uint8_t* buf, message_id_t message_id, const uint8_t* result, size_t result_len, bool confirmable);
 
 	static const size_t function_return_size = 10;
 
@@ -122,6 +123,14 @@ class Messages
       return 5;
     }
 
+    static size_t coded_ack(uint8_t* buf,
+                            uint8_t token,
+                            uint8_t code,
+                            uint8_t message_id_msb,
+                            uint8_t message_id_lsb,
+                            uint8_t* data,
+                            size_t data_len);
+
     static inline size_t reset(unsigned char *buf,
                                          unsigned char message_id_msb,
                                          unsigned char message_id_lsb)

communication/src/protocol_defs.h

@@ -108,5 +108,12 @@ typedef std::function<int()> callback;
 const product_id_t UNDEFINED_PRODUCT_ID = product_id_t(-1);
 const product_firmware_version_t UNDEFINED_PRODUCT_VERSION = product_firmware_version_t(-1);
 
+namespace UpdateFlag {
+enum Enum {
+    ERROR         = 0x00,
+    SUCCESS       = 0x01,
+    VALIDATE_ONLY = 0x02
+};
+}
 
 }}

communication/src/spark_protocol.cpp

@@ -171,7 +171,7 @@ bool SparkProtocol::event_loop(CoAPMessageType::Enum& message_type)
       LOG(WARN,"received ERROR CoAPMessage");
       if (updating) {      // was updating but had an error, inform the client
         serial_dump("handle received message failed - aborting transfer");
-        callbacks.finish_firmware_update(file, 0, NULL);
+        callbacks.finish_firmware_update(file, UpdateFlag::ERROR, NULL);
         updating = false;
       }
 
@@ -337,14 +337,6 @@ void SparkProtocol::hello(unsigned char *buf, bool newly_upgraded)
   wrap(buf, len);
 }
 
-void SparkProtocol::notify_update_done(uint8_t* buf)
-{
-    DEBUG("Sending UpdateDone");
-    unsigned short message_id = next_message_id();
-    size_t size = Messages::update_done(buf+2, message_id, false);
-    wrap(buf, size);
-}
-
 void SparkProtocol::key_changed(unsigned char *buf, unsigned char token)
 {
   separate_response(buf, token, 0x44);
@@ -1068,15 +1060,7 @@ bool SparkProtocol::handle_chunk(msg& message)
     last_chunk_millis = callbacks.millis();
 
     uint8_t* msg_to_send = message.response;
-    // send ACK
-    *msg_to_send = 0;
-    *(msg_to_send + 1) = 16;
-    empty_ack(msg_to_send + 2, queue[2], queue[3]);
-    if (0 > blocking_send(msg_to_send, 18))
-    {
-      // error
-      return false;
-    }
+
     LOG(INFO,"chunk");
     if (!this->updating) {
         LOG(WARN,"got chunk when not updating");
@@ -1101,6 +1085,20 @@ bool SparkProtocol::handle_chunk(msg& message)
         option++;
         payload += (queue[payload]&0xF)+1;  // increase by the size. todo handle > 11
     }
+
+    if (!fast_ota) {
+        // Send ACK
+        // Issue #1240: The firmware should not ACK every Chunk message in Fast OTA mode
+        *msg_to_send = 0;
+        *(msg_to_send + 1) = 16;
+        empty_ack(msg_to_send + 2, queue[2], queue[3]);
+        if (0 > blocking_send(msg_to_send, 18))
+        {
+          // error
+          return false;
+        }
+    }
+
     if (0xFF==queue[payload])
     {
         payload++;
@@ -1112,32 +1110,35 @@ bool SparkProtocol::handle_chunk(msg& message)
             return false;
         }
         uint32_t crc = callbacks.calculate_crc(chunk, file.chunk_size);
-        bool has_response = false;
+        size_t response_size = 0;
         bool crc_valid = (crc == given_crc);
         LOG(INFO,"chunk idx=%d crc=%d fast=%d updating=%d", chunk_index, crc_valid, fast_ota, updating);
         if (crc_valid)
         {
             callbacks.save_firmware_chunk(file, chunk, NULL);
-            if (!fast_ota || (updating!=2 && (true || (chunk_index & 32)==0))) {
+            //if (!fast_ota || (updating!=2 && (true || (chunk_index & 32)==0))) {
+            // Issue #1240:
+            // In Fast OTA mode, the firmware should not respond to every Chunk message
+            // (especially because they are NON confirmable messages).
+            if (!fast_ota) {
                 chunk_received(msg_to_send + 2, message.token, ChunkReceivedCode::OK);
-                has_response = true;
+                response_size = 18;
             }
             flag_chunk_received(chunk_index);
             if (updating==2) {                      // clearing up missed chunks at the end of fast OTA
                 chunk_index_t next_missed = next_chunk_missing(0);
                 if (next_missed==NO_CHUNKS_MISSING) {
                     LOG(INFO,"received all chunks");
                     reset_updating();
-                    notify_update_done(msg_to_send);
-                    callbacks.finish_firmware_update(file, 1, NULL);
-                    has_response = true;
+                    response_size = notify_update_done(msg_to_send, 0, 0);
+                    callbacks.finish_firmware_update(file, UpdateFlag::SUCCESS, NULL);
                 }
                 else {
-                    if (has_response && 0 > blocking_send(msg_to_send, 18)) {
+                    if (response_size && 0 > blocking_send(msg_to_send, 18)) {
                         LOG(WARN,"send chunk response failed");
                         return false;
                     }
-                    has_response = false;
+                    response_size = 0;
 
                     if (next_missed>missed_chunk_index)
                         send_missing_chunks(MISSED_CHUNKS_TO_SEND);
@@ -1148,12 +1149,12 @@ bool SparkProtocol::handle_chunk(msg& message)
         else if (!fast_ota)
         {
             chunk_received(msg_to_send + 2, message.token, ChunkReceivedCode::BAD);
-            has_response = true;
+            response_size = 18;
             LOG(WARN,"chunk bad %d", chunk_index);
         }
         // fast OTA will request the chunk later
 
-        if (has_response && 0 > blocking_send(msg_to_send, 18))
+        if (response_size && 0 > blocking_send(msg_to_send, response_size))
         {
           // error
           return false;
@@ -1198,6 +1199,33 @@ void SparkProtocol::set_chunks_received(uint8_t value)
     	memset(queue+QUEUE_SIZE-bytes, value, bytes);
 }
 
+size_t SparkProtocol::notify_update_done(uint8_t* msg, token_t token, uint8_t code)
+{
+    size_t msgsz = 0;
+    char buf[255];
+    size_t data_len = 0;
+
+    memset(buf, 0, sizeof(buf));
+    if (code != ChunkReceivedCode::BAD) {
+        callbacks.finish_firmware_update(file, UpdateFlag::SUCCESS | UpdateFlag::VALIDATE_ONLY, buf);
+        data_len = strnlen(buf, sizeof(buf) - 1);
+    }
+
+    if (code) {
+        // Send as ACK
+        msgsz = Messages::coded_ack(msg + 2, token, code, queue[2], queue[3], (uint8_t*)buf, data_len);
+    } else {
+        // Send as UpdateDone
+        unsigned short message_id = next_message_id();
+        msgsz = Messages::update_done(msg + 2, message_id, (uint8_t*)buf, data_len, false);
+    }
+
+    LOG(INFO, "Update done %02x", code);
+    LOG_DEBUG(TRACE, "%s", buf);
+
+    return wrap(msg, msgsz);
+}
+
 bool SparkProtocol::handle_update_done(msg& message)
 {
     // send ACK 2.04
@@ -1207,9 +1235,11 @@ bool SparkProtocol::handle_update_done(msg& message)
     *(msg_to_send + 1) = 16;
     chunk_index_t index = next_chunk_missing(0);
     bool missing = index!=NO_CHUNKS_MISSING;
-    coded_ack(msg_to_send + 2, message.token, missing ? ChunkReceivedCode::BAD : ChunkReceivedCode::OK, queue[2], queue[3]);
     LOG(WARN,"update done: received, has missing chunks %d", missing);
-    if (0 > blocking_send(msg_to_send, 18))
+
+    size_t response_size = notify_update_done(msg_to_send, message.token,
+                                              missing ? ChunkReceivedCode::BAD : ChunkReceivedCode::OK);
+    if (0 > blocking_send(msg_to_send, response_size))
     {
         // error
         return false;
@@ -1218,7 +1248,7 @@ bool SparkProtocol::handle_update_done(msg& message)
     if (!missing) {
         LOG(INFO,"update done: all done!");
         reset_updating();
-        callbacks.finish_firmware_update(file, 1, NULL);
+        callbacks.finish_firmware_update(file, UpdateFlag::SUCCESS, NULL);
     }
     else {
         updating = 2;       // flag that we are sending missing chunks.

communication/src/spark_protocol.h

@@ -247,7 +247,7 @@ class SparkProtocol
     void flag_chunk_received(chunk_index_t index);
     chunk_index_t next_chunk_missing(chunk_index_t index);
     int send_missing_chunks(int count);
-    void notify_update_done(uint8_t* buf);
+    size_t notify_update_done(uint8_t* msg, token_t token, uint8_t code);
 
     /**
      * Send a particular type of describe message.

hal/inc/hal_dynalib_ota.h

@@ -49,6 +49,7 @@ DYNALIB_FN(4, hal_ota, HAL_OTA_Flashed_ResetStatus, void(void))
 DYNALIB_FN(5, hal_ota, HAL_FLASH_Begin, bool(uint32_t, uint32_t, void*))
 DYNALIB_FN(6, hal_ota, HAL_FLASH_Update, int(const uint8_t*, uint32_t, uint32_t, void*))
 DYNALIB_FN(7, hal_ota, HAL_FLASH_End, hal_update_complete_t(hal_module_t*))
+DYNALIB_FN(8, hal_ota, HAL_FLASH_OTA_Validate, int(hal_module_t*, bool, module_validation_flags_t, void*))
 
 DYNALIB_END(hal_ota)
 

hal/inc/ota_flash_hal.h

@@ -113,6 +113,8 @@ uint16_t HAL_OTA_ChunkSize();
 
 flash_device_t HAL_OTA_FlashDevice();
 
+int HAL_FLASH_OTA_Validate(hal_module_t* mod, bool userDepsOptional, module_validation_flags_t flags, void* reserved);
+
 /**
  * Erase a region of flash in preparation for flashing content.
  * @param address   The start address to erase. Must be on a flash boundary.

hal/src/core/ota_flash_hal.c

@@ -64,6 +64,11 @@ int HAL_FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSi
     return FLASH_Update(pBuffer, address, bufferSize);
 }
 
+int HAL_FLASH_OTA_Validate(hal_module_t* mod, bool userDepsOptional, module_validation_flags_t flags, void* reserved)
+{
+  return 0;
+}
+
 hal_update_complete_t HAL_FLASH_End(hal_module_t* reserved)
 {
     FLASH_End();

hal/src/gcc/ota_flash_hal.cpp

@@ -46,6 +46,11 @@ int HAL_FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t length,
     return 0;
 }
 
+int HAL_FLASH_OTA_Validate(hal_module_t* mod, bool userDepsOptional, module_validation_flags_t flags, void* reserved)
+{
+  return 0;
+}
+
  hal_update_complete_t HAL_FLASH_End(hal_module_t* mod)
 {
 	 fclose(output_file);

hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp

@@ -217,12 +217,24 @@ int HAL_FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t length,
     return FLASH_Update(pBuffer, address, length);
 }
 
+int HAL_FLASH_OTA_Validate(hal_module_t* mod, bool userDepsOptional, module_validation_flags_t flags, void* reserved) {
+    hal_module_t module;
+
+    bool module_fetched = fetch_module(&module, &module_ota, userDepsOptional, flags);
+
+    if (mod) {
+        memcpy(mod, &module, sizeof(hal_module_t));
+    }
+
+    return (int)!module_fetched;
+}
+
 hal_update_complete_t HAL_FLASH_End(hal_module_t* mod)
 {
     hal_module_t module;
     hal_update_complete_t result = HAL_UPDATE_ERROR;
 
-    bool module_fetched = fetch_module(&module, &module_ota, true, MODULE_VALIDATION_INTEGRITY | MODULE_VALIDATION_DEPENDENCIES_FULL);
+    bool module_fetched = !HAL_FLASH_OTA_Validate(&module, true, (module_validation_flags_t)(MODULE_VALIDATION_INTEGRITY | MODULE_VALIDATION_DEPENDENCIES_FULL), NULL);
 	DEBUG("module fetched %d, checks=%d, result=%d", module_fetched, module.validity_checked, module.validity_result);
     if (module_fetched && (module.validity_checked==module.validity_result))
     {

hal/src/template/ota_flash_hal.cpp

@@ -58,6 +58,11 @@ int HAL_FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t length,
     return 0;
 }
 
+int HAL_FLASH_OTA_Validate(hal_module_t* mod, bool userDepsOptional, module_validation_flags_t flags, void* reserved)
+{
+  return 0;
+}
+
 hal_update_complete_t HAL_FLASH_End(hal_module_t* mod)
 {
     return HAL_UPDATE_ERROR;

system/src/system_cloud_internal.cpp

@@ -1165,21 +1165,16 @@ int formatOtaUpdateStatusEventData(uint32_t flags, int result, hal_module_t* mod
     return res;
 }
 
-int finish_ota_firmware_update(FileTransfer::Descriptor& file, uint32_t flags, void*)
+int finish_ota_firmware_update(FileTransfer::Descriptor& file, uint32_t flags, void* buf)
 {
+    using namespace particle::protocol;
     hal_module_t module;
-    uint8_t buf[512];
 
     int result = Spark_Finish_Firmware_Update(file, flags, &module);
 
-    formatOtaUpdateStatusEventData(flags, result, &module, buf, sizeof(buf));
-
-    LOG(INFO, "Send spark/device/ota_result event");
-    LOG_PRINT(TRACE, (const char*)buf);
-    LOG_PRINTF(TRACE, "\r\n");
-    Particle.publish("spark/device/ota_result", (const char*)buf, 60, PRIVATE);
-    HAL_Delay_Milliseconds(1000);
-    Spark_Process_Events();
+    if (buf && (flags & (UpdateFlag::SUCCESS | UpdateFlag::VALIDATE_ONLY)) == (UpdateFlag::SUCCESS | UpdateFlag::VALIDATE_ONLY)) {
+        formatOtaUpdateStatusEventData(flags, result, &module, (uint8_t*)buf, 255);
+    }
 
     return result;
 }