From 70d3818e7ae4c71fa0055b18ae1270bd9739ade7 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sat, 13 Mar 2021 14:38:40 +0000 Subject: [PATCH] Tidy up --- .../include/Data/Stream/RbootOutputStream.h | 12 ++-- .../rboot/include/Network/RbootHttpUpdater.h | 62 ++++++++++++++----- .../Components/rboot/src/RbootHttpUpdater.cpp | 18 +++--- .../OtaUpgrade/OtaUpgrade/BasicStream.h | 19 +++--- .../OtaUpgrade/OtaUpgrade/EncryptedStream.h | 8 +-- 5 files changed, 79 insertions(+), 40 deletions(-) diff --git a/Sming/Components/rboot/include/Data/Stream/RbootOutputStream.h b/Sming/Components/rboot/include/Data/Stream/RbootOutputStream.h index cff7d97f37..1873d01885 100644 --- a/Sming/Components/rboot/include/Data/Stream/RbootOutputStream.h +++ b/Sming/Components/rboot/include/Data/Stream/RbootOutputStream.h @@ -26,8 +26,10 @@ class RbootOutputStream : public ReadWriteStream { public: /** + * @brief Construct a stream using raw flash address/size * @param startAddress the start address on the storage media * @param maxLength the maximum allowed length of the rom. Use 0 if unlimited. + * @note This should be avoided, use partition where possible */ RbootOutputStream(uint32_t startAddress, size_t maxLength = 0) : startAddress(startAddress), maxLength(maxLength) { @@ -86,11 +88,11 @@ class RbootOutputStream : public ReadWriteStream } protected: - bool initialized = false; - rboot_write_status rBootWriteStatus = {}; - size_t written = 0; // << the number of written bytes - uint32_t startAddress = 0; // << the start address on the storage - size_t maxLength = 0; // << maximum allowed length + bool initialized{false}; + rboot_write_status rBootWriteStatus{}; + size_t written{0}; // << the number of written bytes + uint32_t startAddress{0}; // << the start address on the storage + size_t maxLength{0}; // << maximum allowed length protected: virtual bool init(); diff --git a/Sming/Components/rboot/include/Network/RbootHttpUpdater.h b/Sming/Components/rboot/include/Network/RbootHttpUpdater.h index c7ed16a43c..049056488d 100644 --- a/Sming/Components/rboot/include/Network/RbootHttpUpdater.h +++ b/Sming/Components/rboot/include/Network/RbootHttpUpdater.h @@ -18,11 +18,14 @@ #include #include "../Data/Stream/RbootOutputStream.h" -#define NO_ROM_SWITCH 0xff +/** + * @brief Magic value for ROM slot indicating slot won't change after successful OTA + */ +constexpr uint8_t NO_ROM_SWITCH{0xff}; class RbootHttpUpdater; -typedef Delegate OtaUpdateDelegate; +using OtaUpdateDelegate = Delegate; struct RbootHttpUpdaterItem { String url; @@ -39,11 +42,41 @@ class RbootHttpUpdater : protected HttpClient cleanup(); } + /** + * @brief Add an item to update + * @param offset + * @param firmwareFileUrl + * @param maxSize + * @retval bool + * @note Use the `Partition` overload where possible + */ bool addItem(uint32_t offset, const String& firmwareFileUrl, size_t maxSize = 0); - bool addItem(const String& firmwareFileUrl, RbootOutputStream* stream = nullptr); + + /** + * @brief Add an item to update + * @param firmwareFileUrl + * @param partition Target partition to write + * @retval bool + */ + bool addItem(const String& firmwareFileUrl, Partition partition) + { + return addItem(partition.address(), firmwareFileUrl, partition.size()); + } + + /** + * @brief Add an item to update use a pre-constructed stream + * @param firmwareFileUrl + * @param stream + * @retval bool + */ + bool addItem(const String& firmwareFileUrl, RbootOutputStream* stream); void start(); + /** + * @brief On completion, switch to the given ROM slot + * @param romSlot specify NO_ROM_SWITCH (the default) to cancel any previously set switch + */ void switchToRom(uint8_t romSlot) { this->romSlot = romSlot; @@ -59,11 +92,13 @@ class RbootHttpUpdater : protected HttpClient this->updateDelegate = reqUpdateDelegate; } - /* Sets the base request that can be used to pass - * - default request parameters, like request headers... - * - default SSL options - * - default SSL fingeprints - * - default SSL client certificates + /** + * @brief Sets the base request that can be used to pass + * + * - default request parameters, like request headers... + * - default SSL options + * - default SSL fingeprints + * - default SSL client certificates * * @param request */ @@ -87,12 +122,11 @@ class RbootHttpUpdater : protected HttpClient protected: Vector items; - int currentItem = 0; - rboot_write_status rbootWriteStatus; - uint8_t romSlot = NO_ROM_SWITCH; - OtaUpdateDelegate updateDelegate = nullptr; - - HttpRequest* baseRequest = nullptr; + OtaUpdateDelegate updateDelegate; + HttpRequest* baseRequest{nullptr}; + uint8_t romSlot{NO_ROM_SWITCH}; + uint8_t currentItem{0}; + rboot_write_status rbootWriteStatus{}; private: void cleanup() diff --git a/Sming/Components/rboot/src/RbootHttpUpdater.cpp b/Sming/Components/rboot/src/RbootHttpUpdater.cpp index 07d28bf31d..b6a6f4b101 100644 --- a/Sming/Components/rboot/src/RbootHttpUpdater.cpp +++ b/Sming/Components/rboot/src/RbootHttpUpdater.cpp @@ -43,8 +43,10 @@ bool RbootHttpUpdater::addItem(const String& firmwareFileUrl, RbootOutputStream* void RbootHttpUpdater::start() { for(unsigned i = 0; i < items.count(); i++) { - RbootHttpUpdaterItem& it = items[i]; - debug_d("Download file:\r\n (%d) %s -> %X", currentItem, it.url.c_str(), it.targetOffset); + auto& it = items[i]; + debug_d("Download file:\r\n" + " (%u) %s -> %X", + currentItem, it.url.c_str(), it.targetOffset); HttpRequest* request; if(baseRequest != nullptr) { @@ -82,8 +84,8 @@ int RbootHttpUpdater::itemComplete(HttpConnection& client, bool success) return -1; } - RbootHttpUpdaterItem& it = items[currentItem]; - debug_d("Finished: URL: %s, Offset: %d, Length: %d", it.url.c_str(), it.stream->getStartAddress(), + auto& it = items[currentItem]; + debug_d("Finished: URL: %s, Offset: 0x%X, Length: %u", it.url.c_str(), it.stream->getStartAddress(), it.stream->available()); it.stream = nullptr; // the actual deletion will happen outside of this class @@ -94,14 +96,14 @@ int RbootHttpUpdater::itemComplete(HttpConnection& client, bool success) int RbootHttpUpdater::updateComplete(HttpConnection& client, bool success) { - auto hasError = itemComplete(client, success); - if(hasError) { + int hasError = itemComplete(client, success); + if(hasError != 0) { return hasError; } debug_d("\r\nFirmware download finished!"); for(unsigned i = 0; i < items.count(); i++) { - debug_d(" - item: %d, addr: %X, url: %s", i, items[i].targetOffset, items[i].url.c_str()); + debug_d(" - item: %u, addr: 0x%X, url: %s", i, items[i].targetOffset, items[i].url.c_str()); } if(!success) { @@ -136,7 +138,7 @@ void RbootHttpUpdater::applyUpdate() } // set to boot new rom and then reboot - debug_d("Firmware updated, rebooting to rom %d...\r\n", romSlot); + debug_d("Firmware updated, rebooting to rom %u...\r\n", romSlot); rboot_set_current_rom(romSlot); System.restart(); } diff --git a/Sming/Libraries/OtaUpgrade/OtaUpgrade/BasicStream.h b/Sming/Libraries/OtaUpgrade/OtaUpgrade/BasicStream.h index 4e4c3bfce6..303431caae 100644 --- a/Sming/Libraries/OtaUpgrade/OtaUpgrade/BasicStream.h +++ b/Sming/Libraries/OtaUpgrade/OtaUpgrade/BasicStream.h @@ -114,11 +114,12 @@ class BasicStream : public ReadWriteStream uint32_t address; uint32_t size; uint8_t index; - bool updated = false; - } slot; + bool updated{false}; + }; + Slot slot; // Instead of RbootOutputStream, the rboot write API is used directly because in a future extension the OTA file may contain data for multiple FLASH regions. - rboot_write_status rbootWriteStatus = {}; + rboot_write_status rbootWriteStatus{}; enum class State { Error, @@ -129,14 +130,14 @@ class BasicStream : public ReadWriteStream VerifyRoms, RomsComplete, }; - State state = State::Header; + State state{State::Header}; #ifdef ENABLE_OTA_SIGNING using Verifier = SignatureVerifier; - static const uint32_t expectedHeaderMagic = OTA_HEADER_MAGIC_SIGNED; + static const uint32_t expectedHeaderMagic{OTA_HEADER_MAGIC_SIGNED}; #else using Verifier = ChecksumVerifier; - static const uint32_t expectedHeaderMagic = OTA_HEADER_MAGIC_NOT_SIGNED; + static const uint32_t expectedHeaderMagic{OTA_HEADER_MAGIC_NOT_SIGNED}; #endif Verifier verifier; @@ -145,9 +146,9 @@ class BasicStream : public ReadWriteStream Verifier::VerificationData verificationData; - size_t remainingBytes = 0; - uint8_t* destinationPtr = nullptr; - uint8_t romIndex = 0; + size_t remainingBytes{0}; + uint8_t* destinationPtr{nullptr}; + uint8_t romIndex{0}; void setupChunk(State nextState, size_t size, void* destination = nullptr) { diff --git a/Sming/Libraries/OtaUpgrade/OtaUpgrade/EncryptedStream.h b/Sming/Libraries/OtaUpgrade/OtaUpgrade/EncryptedStream.h index 42c9e2b2c3..7b34375a33 100644 --- a/Sming/Libraries/OtaUpgrade/OtaUpgrade/EncryptedStream.h +++ b/Sming/Libraries/OtaUpgrade/OtaUpgrade/EncryptedStream.h @@ -50,12 +50,12 @@ class EncryptedStream : public BasicStream Chunk, None, }; - Fragment fragment = Fragment::Header; - size_t remainingBytes = sizeof(header); - uint8_t* fragmentPtr = header; + Fragment fragment{Fragment::Header}; + size_t remainingBytes{sizeof header}; + uint8_t* fragmentPtr{header}; std::unique_ptr buffer; - size_t bufferSize = 0; + size_t bufferSize{0}; }; } // namespace OtaUpgrade