Merge pull request 'STR update' (#594) from str_small_fixes into develop

Reviewed-on: #594
Reviewed-by: Marius Eggert <eggertm@irs.uni-stuttgart.de>

CHANGELOG.md

@@ -21,6 +21,10 @@ will consitute of a breaking change warranting a new major release:
 ## Fixed
 
 - Small fix for `install-obsw-yocto.sh` script
+- Bugfix for STR firmware update procedure where the last remaining
+  bytes were not written properly.
+- Bugfix for STR where an invalid reply was received for special requests
+  like firmware updates.
 
 ## Added
 

linux/acs/StrComHandler.cpp

@@ -89,7 +89,7 @@ ReturnValue_t StrComHandler::performOperation(uint8_t operationCode) {
         break;
       }
       case InternalState::FIRMWARE_UPDATE: {
-        replyTimeout.setTimeout(200);
+        replyTimeout.setTimeout(2000);
         resetReplyHandlingState();
         result = performFirmwareUpdate();
         if (result == returnvalue::OK) {
@@ -125,6 +125,7 @@ ReturnValue_t StrComHandler::startImageUpload(std::string fullname) {
   }
   {
     MutexGuard mg(lock);
+    replyWasReceived = false;
     state = InternalState::UPLOAD_IMAGE;
   }
   semaphore.release();
@@ -151,6 +152,7 @@ ReturnValue_t StrComHandler::startImageDownload(std::string path) {
   downloadImage.path = path;
   {
     MutexGuard mg(lock);
+    replyWasReceived = false;
     state = InternalState::DOWNLOAD_IMAGE;
   }
   terminate = false;
@@ -187,6 +189,7 @@ ReturnValue_t StrComHandler::startFirmwareUpdate(std::string fullname) {
   flashWrite.lastRegion = static_cast<uint8_t>(startracker::FirmwareRegions::LAST);
   {
     MutexGuard mg(lock);
+    replyWasReceived = false;
     state = InternalState::FIRMWARE_UPDATE;
   }
   semaphore.release();
@@ -216,6 +219,7 @@ ReturnValue_t StrComHandler::startFlashRead(std::string path, uint8_t startRegio
   flashRead.size = length;
   {
     MutexGuard mg(lock);
+    replyWasReceived = false;
     state = InternalState::FLASH_READ;
   }
   semaphore.release();
@@ -390,7 +394,8 @@ ReturnValue_t StrComHandler::performFlashWrite() {
 #endif
   ReturnValue_t result = returnvalue::OK;
   uint32_t size = 0;
-  uint32_t bytesWritten = 0;
+  uint32_t bytesWrittenInRegion = 0;
+  size_t totalBytesWritten = 0;
   uint32_t fileSize = 0;
 
   struct WriteActionRequest req;
@@ -412,20 +417,18 @@ ReturnValue_t StrComHandler::performFlashWrite() {
   ProgressPrinter progressPrinter("Flash write", fileSize);
 #endif /* OBSW_DEBUG_STARTRACKER == 1 */
   uint32_t fileChunks = fileSize / CHUNK_SIZE;
-  bytesWritten = 0;
+  bytesWrittenInRegion = 0;
   req.region = flashWrite.firstRegion;
   req.length = CHUNK_SIZE;
-  for (uint32_t idx = 0; idx < fileChunks; idx++) {
+
-    if (terminate) {
+  auto writeNextSegment = [&](uint32_t chunkIdx) {
-      return returnvalue::OK;
+    file.seekg(chunkIdx * CHUNK_SIZE, file.beg);
-    }
-    file.seekg(idx * CHUNK_SIZE, file.beg);
     file.read(reinterpret_cast<char*>(req.data), CHUNK_SIZE);
-    if (bytesWritten + CHUNK_SIZE > FLASH_REGION_SIZE) {
+    if (bytesWrittenInRegion + CHUNK_SIZE > FLASH_REGION_SIZE) {
       req.region++;
-      bytesWritten = 0;
+      bytesWrittenInRegion = 0;
     }
-    req.address = bytesWritten;
+    req.address = bytesWrittenInRegion;
     arc_pack_write_action_req(&req, cmdBuf.data(), &size);
     result = sendAndRead(size, req.address);
     if (result != returnvalue::OK) {
@@ -435,34 +438,49 @@ ReturnValue_t StrComHandler::performFlashWrite() {
     if (result != returnvalue::OK) {
       return result;
     }
-    bytesWritten += CHUNK_SIZE;
+    totalBytesWritten += CHUNK_SIZE;
+    bytesWrittenInRegion += CHUNK_SIZE;
 #if OBSW_DEBUG_STARTRACKER == 1
-    progressPrinter.print(idx * CHUNK_SIZE);
+    progressPrinter.print(chunkIdx * CHUNK_SIZE);
 #endif /* OBSW_DEBUG_STARTRACKER == 1 */
+    return result;
+  };
+
+  for (uint32_t idx = 0; idx < fileChunks; idx++) {
+    if (terminate) {
+      return returnvalue::OK;
+    }
+    result = writeNextSegment(idx);
+    if(result != returnvalue::OK) {
+      return result;
+    }
     if (idx % 50 == 0) {
       // Some grace time for other tasks
       TaskFactory::delayTask(2);
     }
   }
   uint32_t remainingBytes = fileSize - fileChunks * CHUNK_SIZE;
-  file.seekg((fileChunks - 1) * CHUNK_SIZE, file.beg);
+  if (remainingBytes > 0) {
-  file.read(reinterpret_cast<char*>(req.data), remainingBytes);
+    file.seekg(fileChunks * CHUNK_SIZE, file.beg);
-  file.close();
+    file.read(reinterpret_cast<char*>(req.data), remainingBytes);
-  if (bytesWritten + CHUNK_SIZE > FLASH_REGION_SIZE) {
+    file.close();
-    req.region++;
+    if (bytesWrittenInRegion + CHUNK_SIZE > FLASH_REGION_SIZE) {
-    bytesWritten = 0;
+      req.region++;
-  }
+      bytesWrittenInRegion = 0;
-  req.address = bytesWritten;
+    }
-  req.length = remainingBytes;
+    req.address = bytesWrittenInRegion;
-  bytesWritten += remainingBytes;
+    req.length = remainingBytes;
-  arc_pack_write_action_req(&req, cmdBuf.data(), &size);
+    totalBytesWritten += CHUNK_SIZE;
-  result = sendAndRead(size, req.address);
+    bytesWrittenInRegion += remainingBytes;
-  if (result != returnvalue::OK) {
+    arc_pack_write_action_req(&req, cmdBuf.data(), &size);
-    return result;
+    result = sendAndRead(size, req.address);
-  }
+    if (result != returnvalue::OK) {
-  result = checkActionReply(replyLen);
+      return result;
-  if (result != returnvalue::OK) {
+    }
-    return result;
+    result = checkActionReply(replyLen);
+    if (result != returnvalue::OK) {
+      return result;
+    }
   }
 #if OBSW_DEBUG_STARTRACKER == 1
   progressPrinter.print(fileSize);

tmtc

@@ -1 +1 @@
-Subproject commit 01b3a894e66e7f38dd2e7a35e329e99a6199b7f1
+Subproject commit 33fd280e517a55175072ea336c7f8e85bce6e55a