Encapsulate DFU Image buffering and writing into spi flash in DfuImage.

Add some const in SPI driver.

src/CMakeLists.txt

@@ -355,7 +355,6 @@ list(APPEND SOURCE_FILES
   Components/Ble/DfuService.cpp
   Components/Ble/CurrentTimeService.cpp
   Components/Ble/AlertNotificationService.cpp
-  Components/Ble/DfuImage.cpp
   drivers/Cst816s.cpp
   FreeRTOS/port.c
   FreeRTOS/port_cmsis_systick.c
@@ -409,7 +408,6 @@ set(INCLUDE_FILES
   Components/Ble/CurrentTimeClient.h
   Components/Ble/AlertNotificationClient.h
   Components/Ble/DfuService.h
-  Components/Ble/DfuImage.h
     drivers/Cst816s.h
   FreeRTOS/portmacro.h
   FreeRTOS/portmacro_cmsis.h

src/Components/Ble/DfuService.cpp

@@ -30,7 +30,7 @@ DfuService::DfuService(Pinetime::System::SystemTask &systemTask, Pinetime::Contr
                        Pinetime::Drivers::SpiNorFlash &spiNorFlash) :
         systemTask{systemTask},
         bleController{bleController},
-        spiNorFlash{spiNorFlash},
+        dfuImage{spiNorFlash},
         characteristicDefinition{
                 {
                         .uuid = (ble_uuid_t *) &packetCharacteristicUuid,
@@ -124,11 +124,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) {
       NRF_LOG_INFO("[DFU] -> Start data received : SD size : %d, BT size : %d, app size : %d", softdeviceSize,
                    bootloaderSize, applicationSize);
 
-      for (int erased = 0; erased < maxImageSize; erased += 0x1000) {
+      dfuImage.Erase();
-#if 1
-        spiNorFlash.SectorErase(writeOffset + erased);
-#endif
-      }
 
       uint8_t data[]{16, 1, 1};
       notificationManager.Send(connectionHandle, controlPointCharacteristicHandle, data, 3);
@@ -157,13 +153,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) {
 
     case States::Data: {
       nbPacketReceived++;
-      auto offset = ((nbPacketReceived - 1) % nbPacketsToNotify) * 20;
+      dfuImage.Append(om->om_data, om->om_len);
-      std::memcpy(tempBuffer + offset, om->om_data, om->om_len);
-
-      if (nbPacketReceived > 0 && (nbPacketReceived % nbPacketsToNotify) == 0) {
-        spiNorFlash.Write(writeOffset + ((nbPacketReceived - nbPacketsToNotify) * 20), tempBuffer, 200);
-      }
-
       bytesReceived += om->om_len;
       bleController.FirmwareUpdateCurrentBytes(bytesReceived);
 
@@ -174,15 +164,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) {
         NRF_LOG_INFO("[DFU] -> Send packet notification: %d bytes received", bytesReceived);
         notificationManager.Send(connectionHandle, controlPointCharacteristicHandle, data, 5);
       }
-      if (bytesReceived == applicationSize) {
+      if (dfuImage.IsComplete()) {
-        if ((nbPacketReceived % nbPacketsToNotify) != 0) {
-          auto remaningPacket = nbPacketReceived % nbPacketsToNotify;
-          spiNorFlash.Write(writeOffset + ((nbPacketReceived - remaningPacket) * 20), tempBuffer, remaningPacket * 20);
-        }
-
-        if (applicationSize < maxImageSize)
-          WriteMagicNumber();
-
         uint8_t data[3]{static_cast<uint8_t>(Opcodes::Response),
                         static_cast<uint8_t>(Opcodes::ReceiveFirmwareImage),
                         static_cast<uint8_t>(ErrorCodes::NoError)};
@@ -257,6 +239,8 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) {
         NRF_LOG_INFO("[DFU] -> Receive firmware image requested, but we are not in Start Init");
         return 0;
       }
+      // TODO the chunk size is dependant of the implementation of the host application...
+      dfuImage.Init(20, applicationSize, expectedCrc);
       NRF_LOG_INFO("[DFU] -> Starting receive firmware");
       state = States::Data;
       return 0;
@@ -268,8 +252,11 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) {
 
       NRF_LOG_INFO("[DFU] -> Validate firmware image requested -- %d", connectionHandle);
 
-      if(Validate()){
+      if(dfuImage.Validate()){
         state = States::Validated;
+        bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Validated);
+        NRF_LOG_INFO("Image OK");
+
         uint8_t data[3] {
                 static_cast<uint8_t>(Opcodes::Response),
                 static_cast<uint8_t>(Opcodes::ValidateFirmware),
@@ -277,6 +264,9 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) {
         };
         notificationManager.AsyncSend(connectionHandle, controlPointCharacteristicHandle, data, 3);
       } else {
+        bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Error);
+        NRF_LOG_INFO("Image Error : bad CRC");
+
         uint8_t data[3] {
                 static_cast<uint8_t>(Opcodes::Response),
                 static_cast<uint8_t>(Opcodes::ValidateFirmware),
@@ -303,64 +293,6 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) {
   }
 }
 
-
-
-uint16_t DfuService::ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc) {
-  uint16_t crc = (p_crc == NULL) ? 0xFFFF : *p_crc;
-
-  for (uint32_t i = 0; i < size; i++) {
-    crc = (uint8_t) (crc >> 8) | (crc << 8);
-    crc ^= p_data[i];
-    crc ^= (uint8_t) (crc & 0xFF) >> 4;
-    crc ^= (crc << 8) << 4;
-    crc ^= ((crc & 0xFF) << 4) << 1;
-  }
-
-  return crc;
-}
-
-bool DfuService::Validate() {
-  uint32_t chunkSize = 200;
-  int currentOffset = 0;
-  uint16_t crc = 0;
-
-  bool first = true;
-  while (currentOffset < applicationSize) {
-    uint32_t readSize = (applicationSize - currentOffset) > chunkSize ? chunkSize : (applicationSize - currentOffset);
-
-    spiNorFlash.Read(writeOffset + currentOffset, tempBuffer, readSize);
-    if (first) {
-      crc = ComputeCrc(tempBuffer, readSize, NULL);
-      first = false;
-    } else
-      crc = ComputeCrc(tempBuffer, readSize, &crc);
-    currentOffset += readSize;
-  }
-
-  NRF_LOG_INFO("Expected CRC : %u - Processed CRC : %u", expectedCrc, crc);
-  bool crcOk = (crc == expectedCrc);
-  if (crcOk) {
-    bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Validated);
-    NRF_LOG_INFO("Image OK");
-  } else {
-    bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Error);
-    NRF_LOG_INFO("Image Error : bad CRC");
-  }
-  return crcOk;
-}
-
-void DfuService::WriteMagicNumber() {
-  uint32_t magic[4] = {
-          0xf395c277,
-          0x7fefd260,
-          0x0f505235,
-          0x8079b62c,
-  };
-
-  uint32_t offset = writeOffset + (maxImageSize - (4 * sizeof(uint32_t)));
-  spiNorFlash.Write(offset, reinterpret_cast<uint8_t *>(magic), 4 * sizeof(uint32_t));
-}
-
 void DfuService::OnTimeout() {
   Reset();
 }
@@ -415,3 +347,90 @@ void DfuService::NotificationManager::Reset() {
   size = 0;
   xTimerStop(timer, 0);
 }
+
+void DfuService::DfuImage::Init(size_t chunkSize, size_t totalSize, uint16_t expectedCrc) {
+  if(chunkSize != 20) return;
+  this->chunkSize = chunkSize;
+  this->totalSize = totalSize;
+  this->expectedCrc = expectedCrc;
+  this->ready = true;
+}
+
+void DfuService::DfuImage::Append(uint8_t *data, size_t size) {
+  if(!ready) return;
+  ASSERT(size <= 20);
+
+  std::memcpy(tempBuffer + bufferWriteIndex, data, size);
+  bufferWriteIndex += size;
+
+  if(bufferWriteIndex == bufferSize) {
+    spiNorFlash.Write(writeOffset + totalWriteIndex, tempBuffer, bufferWriteIndex);
+    totalWriteIndex += bufferWriteIndex;
+    bufferWriteIndex = 0;
+  }
+
+  if(bufferWriteIndex > 0 && totalWriteIndex + bufferWriteIndex == totalSize) {
+    spiNorFlash.Write(writeOffset + totalWriteIndex, tempBuffer, bufferWriteIndex);
+    totalWriteIndex += bufferWriteIndex;
+    if (totalSize < maxSize);
+      WriteMagicNumber();
+  }
+}
+
+void DfuService::DfuImage::WriteMagicNumber() {
+  static constexpr uint32_t magic[4] = {
+          0xf395c277,
+          0x7fefd260,
+          0x0f505235,
+          0x8079b62c,
+  };
+
+  uint32_t offset = writeOffset + (maxSize - (4 * sizeof(uint32_t)));
+  spiNorFlash.Write(offset, reinterpret_cast<const uint8_t *>(magic), 4 * sizeof(uint32_t));
+}
+
+void DfuService::DfuImage::Erase() {
+  for (int erased = 0; erased < maxSize; erased += 0x1000) {
+    spiNorFlash.SectorErase(writeOffset + erased);
+  }
+}
+
+bool DfuService::DfuImage::Validate() {
+  uint32_t chunkSize = 200;
+  int currentOffset = 0;
+  uint16_t crc = 0;
+
+  bool first = true;
+  while (currentOffset < totalSize) {
+    uint32_t readSize = (totalSize - currentOffset) > chunkSize ? chunkSize : (totalSize - currentOffset);
+
+    spiNorFlash.Read(writeOffset + currentOffset, tempBuffer, readSize);
+    if (first) {
+      crc = ComputeCrc(tempBuffer, readSize, NULL);
+      first = false;
+    } else
+      crc = ComputeCrc(tempBuffer, readSize, &crc);
+    currentOffset += readSize;
+  }
+
+  return (crc == expectedCrc);
+}
+
+uint16_t DfuService::DfuImage::ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc) {
+  uint16_t crc = (p_crc == NULL) ? 0xFFFF : *p_crc;
+
+  for (uint32_t i = 0; i < size; i++) {
+    crc = (uint8_t) (crc >> 8) | (crc << 8);
+    crc ^= p_data[i];
+    crc ^= (uint8_t) (crc & 0xFF) >> 4;
+    crc ^= (crc << 8) << 4;
+    crc ^= ((crc & 0xFF) << 4) << 1;
+  }
+
+  return crc;
+}
+
+bool DfuService::DfuImage::IsComplete() {
+  if(!ready) return false;
+  return totalWriteIndex == totalSize;
+}

src/Components/Ble/DfuService.h

@@ -20,7 +20,6 @@ namespace Pinetime {
         DfuService(Pinetime::System::SystemTask &systemTask, Pinetime::Controllers::Ble &bleController,
                    Pinetime::Drivers::SpiNorFlash &spiNorFlash);
         void Init();
-        bool Validate();
         int OnServiceData(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt *context);
         void OnTimeout();
         void Reset();
@@ -40,11 +39,38 @@ namespace Pinetime {
             void OnNotificationTimer();
             void Reset();
         };
+        class DfuImage {
+          public:
+            DfuImage(Pinetime::Drivers::SpiNorFlash& spiNorFlash) : spiNorFlash{spiNorFlash} {}
+            void Init(size_t chunkSize, size_t totalSize, uint16_t expectedCrc);
+            void Erase();
+            void Append(uint8_t* data, size_t size);
+            bool Validate();
+            bool IsComplete();
+
+          private:
+            Pinetime::Drivers::SpiNorFlash& spiNorFlash;
+            static constexpr size_t bufferSize = 200;
+            bool ready = false;
+            size_t chunkSize = 0;
+            size_t totalSize = 0;
+            size_t maxSize = 475136;
+            size_t bufferWriteIndex = 0;
+            size_t totalWriteIndex = 0;
+            static constexpr size_t writeOffset = 0x40000;
+            uint8_t tempBuffer[bufferSize];
+            uint16_t expectedCrc = 0;
+
+            void WriteMagicNumber();
+            uint16_t ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc);
+
+        };
 
       private:
         Pinetime::System::SystemTask &systemTask;
         Pinetime::Controllers::Ble &bleController;
-        Pinetime::Drivers::SpiNorFlash &spiNorFlash;
+        DfuImage dfuImage;
+        NotificationManager notificationManager;
 
         static constexpr uint16_t dfuServiceId{0x1530};
         static constexpr uint16_t packetCharacteristicId{0x1532};
@@ -119,22 +145,17 @@ namespace Pinetime {
         uint8_t nbPacketsToNotify = 0;
         uint32_t nbPacketReceived = 0;
         uint32_t bytesReceived = 0;
-        uint32_t writeOffset = 0x40000;
 
         uint32_t softdeviceSize = 0;
         uint32_t bootloaderSize = 0;
         uint32_t applicationSize = 0;
-        static constexpr uint32_t maxImageSize = 475136;
         uint16_t expectedCrc = 0;
 
         int SendDfuRevision(os_mbuf *om) const;
         int WritePacketHandler(uint16_t connectionHandle, os_mbuf *om);
         int ControlPointHandler(uint16_t connectionHandle, os_mbuf *om);
-        uint8_t tempBuffer[200];
+
-        uint16_t ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc);
-        void WriteMagicNumber();
         TimerHandle_t timeoutTimer;
-        NotificationManager notificationManager;
     };
   }
 }

src/drivers/Spi.cpp

@@ -27,7 +27,7 @@ bool Spi::Init() {
   return true;
 }
 
-bool Spi::WriteCmdAndBuffer(uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) {
+bool Spi::WriteCmdAndBuffer(const uint8_t *cmd, size_t cmdSize, const uint8_t *data, size_t dataSize) {
   return spiMaster.WriteCmdAndBuffer(pinCsn, cmd, cmdSize, data, dataSize);
 }
 

src/drivers/Spi.h

@@ -22,7 +22,7 @@ namespace Pinetime {
         bool Init();
         bool Write(const uint8_t* data, size_t size);
         bool Read(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize);
-        bool WriteCmdAndBuffer(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize);
+        bool WriteCmdAndBuffer(const uint8_t* cmd, size_t cmdSize, const uint8_t *data, size_t dataSize);
         void Sleep();
         void Wakeup();
 

src/drivers/SpiMaster.cpp

@@ -239,7 +239,7 @@ void SpiMaster::Wakeup() {
   Init();
 }
 
-bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) {
+bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t *cmd, size_t cmdSize, const uint8_t *data, size_t dataSize) {
   xSemaphoreTake(mutex, portMAX_DELAY);
 
   taskToNotify = nullptr;

src/drivers/SpiMaster.h

@@ -37,7 +37,7 @@ namespace Pinetime {
         bool Write(uint8_t pinCsn, const uint8_t* data, size_t size);
         bool Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize);
 
-        bool WriteCmdAndBuffer(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize);
+        bool WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t* cmd, size_t cmdSize, const uint8_t *data, size_t dataSize);
 
         void OnStartedEvent();
         void OnEndEvent();

src/drivers/SpiNorFlash.cpp

@@ -96,12 +96,12 @@ bool SpiNorFlash::EraseFailed() {
   return (ReadSecurityRegister() & 0x40u) == 0x40u;
 }
 
-void SpiNorFlash::Write(uint32_t address, uint8_t *buffer, size_t size) {
+void SpiNorFlash::Write(uint32_t address, const uint8_t *buffer, size_t size) {
   static constexpr uint8_t cmdSize = 4;
 
   size_t len = size;
   uint32_t addr = address;
-  uint8_t* b = buffer;
+  const uint8_t* b = buffer;
   while(len > 0) {
     uint32_t pageLimit = (addr & ~(pageSize - 1u)) + pageSize;
     uint32_t toWrite = pageLimit - addr > len ? len :  pageLimit - addr;

src/drivers/SpiNorFlash.h

@@ -24,7 +24,7 @@ namespace Pinetime {
         bool WriteEnabled();
         uint8_t ReadConfigurationRegister();
         void Read(uint32_t address, uint8_t* buffer, size_t size);
-        void Write(uint32_t address, uint8_t *buffer, size_t size);
+        void Write(uint32_t address, const uint8_t *buffer, size_t size);
         void WriteEnable();
         void SectorErase(uint32_t sectorAddress);
         uint8_t ReadSecurityRegister();