From de822cc3a2f07033e881331ac8914b26023bb003 Mon Sep 17 00:00:00 2001
From: JF <jf@codingfield.com>
Date: Mon, 1 Jun 2020 18:32:46 +0200
Subject: [PATCH] Encapsulate DFU Image buffering and writing into spi flash in
DfuImage. Add some const in SPI driver.
---
src/CMakeLists.txt | 4 +-
src/Components/Ble/DfuService.cpp | 181 +++++++++++++++++-------------
src/Components/Ble/DfuService.h | 37 ++++--
src/drivers/Spi.cpp | 2 +-
src/drivers/Spi.h | 2 +-
src/drivers/SpiMaster.cpp | 2 +-
src/drivers/SpiMaster.h | 2 +-
src/drivers/SpiNorFlash.cpp | 4 +-
src/drivers/SpiNorFlash.h | 2 +-
9 files changed, 137 insertions(+), 99 deletions(-)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index c37a05a6..4799ea1c 100644
--- a/src/CMakeLists.txt
+++ b/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,8 +408,7 @@ set(INCLUDE_FILES
Components/Ble/CurrentTimeClient.h
Components/Ble/AlertNotificationClient.h
Components/Ble/DfuService.h
- Components/Ble/DfuImage.h
- drivers/Cst816s.h
+ drivers/Cst816s.h
FreeRTOS/portmacro.h
FreeRTOS/portmacro_cmsis.h
libs/date/includes/date/tz.h
diff --git a/src/Components/Ble/DfuService.cpp b/src/Components/Ble/DfuService.cpp
index a170fdfa..e4dcdf38 100644
--- a/src/Components/Ble/DfuService.cpp
+++ b/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) {
-#if 1
- spiNorFlash.SectorErase(writeOffset + erased);
-#endif
- }
+ dfuImage.Erase();
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;
- std::memcpy(tempBuffer + offset, om->om_data, om->om_len);
-
- if (nbPacketReceived > 0 && (nbPacketReceived % nbPacketsToNotify) == 0) {
- spiNorFlash.Write(writeOffset + ((nbPacketReceived - nbPacketsToNotify) * 20), tempBuffer, 200);
- }
-
+ dfuImage.Append(om->om_data, om->om_len);
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 ((nbPacketReceived % nbPacketsToNotify) != 0) {
- auto remaningPacket = nbPacketReceived % nbPacketsToNotify;
- spiNorFlash.Write(writeOffset + ((nbPacketReceived - remaningPacket) * 20), tempBuffer, remaningPacket * 20);
- }
-
- if (applicationSize < maxImageSize)
- WriteMagicNumber();
-
+ if (dfuImage.IsComplete()) {
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;
+}
diff --git a/src/Components/Ble/DfuService.h b/src/Components/Ble/DfuService.h
index b0eccdf5..d7ba460c 100644
--- a/src/Components/Ble/DfuService.h
+++ b/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;
};
}
}
\ No newline at end of file
diff --git a/src/drivers/Spi.cpp b/src/drivers/Spi.cpp
index ec3a5e94..bf08178d 100644
--- a/src/drivers/Spi.cpp
+++ b/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);
}
diff --git a/src/drivers/Spi.h b/src/drivers/Spi.h
index bee39af2..82ba8a65 100644
--- a/src/drivers/Spi.h
+++ b/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();
diff --git a/src/drivers/SpiMaster.cpp b/src/drivers/SpiMaster.cpp
index ff241184..8087d386 100644
--- a/src/drivers/SpiMaster.cpp
+++ b/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;
diff --git a/src/drivers/SpiMaster.h b/src/drivers/SpiMaster.h
index 88b37a35..cd3193e4 100644
--- a/src/drivers/SpiMaster.h
+++ b/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();
diff --git a/src/drivers/SpiNorFlash.cpp b/src/drivers/SpiNorFlash.cpp
index 8fbb53a1..7e4da1ca 100644
--- a/src/drivers/SpiNorFlash.cpp
+++ b/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;
diff --git a/src/drivers/SpiNorFlash.h b/src/drivers/SpiNorFlash.h
index b5f19202..98267c09 100644
--- a/src/drivers/SpiNorFlash.h
+++ b/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();