From e5f50767024546f9d44eae9d65b246d06ebe5b48 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Thu, 19 Aug 2021 11:35:22 +0200 Subject: [PATCH 1/9] Clear non-factory module slots in DCT when preparing for an update --- hal/src/nRF52840/ota_flash_hal.cpp | 5 ++- hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp | 5 ++- .../SPARK_Firmware_Driver/src/flash_mal.c | 33 +++++++++++++++++-- platform/MCU/nRF52840/inc/flash_access.h | 2 +- platform/MCU/nRF52840/src/flash_mal.c | 33 +++++++++++++++++-- platform/MCU/shared/STM32/inc/flash_access.h | 2 +- 6 files changed, 70 insertions(+), 10 deletions(-) diff --git a/hal/src/nRF52840/ota_flash_hal.cpp b/hal/src/nRF52840/ota_flash_hal.cpp index 0b2401b774..7f3f6941ab 100644 --- a/hal/src/nRF52840/ota_flash_hal.cpp +++ b/hal/src/nRF52840/ota_flash_hal.cpp @@ -354,7 +354,10 @@ uint16_t HAL_OTA_ChunkSize() bool HAL_FLASH_Begin(uint32_t address, uint32_t length, void* reserved) { - FLASH_Begin(address, length); + const int r = FLASH_Begin(address, length); + if (r != 0) { + return false; + } return true; } diff --git a/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp b/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp index b4c9ec9152..bffb4fe9ab 100644 --- a/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp +++ b/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp @@ -250,7 +250,10 @@ uint16_t HAL_OTA_ChunkSize() bool HAL_FLASH_Begin(uint32_t address, uint32_t length, void* reserved) { - FLASH_Begin(address, length); + const int r = FLASH_Begin(address, length); + if (r != 0) { + return false; + } return true; } diff --git a/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c b/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c index ce0253546c..dee2138ee0 100644 --- a/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c +++ b/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c @@ -873,16 +873,43 @@ uint32_t FLASH_PagesMask(uint32_t imageSize, uint32_t pageSize) return numPages; } -void FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) +int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) { system_flags.OTA_FLASHED_Status_SysFlag = 0x0000; Save_SystemFlags(); + // Clear all non-factory module slots in the DCT + const size_t slot_count = MAX_MODULES_SLOT - GEN_START_SLOT; + size_t slot_offs = DCT_FLASH_MODULES_OFFSET + sizeof(platform_flash_modules_t) * GEN_START_SLOT; + for (size_t i = 0; i < slot_count; ++i) { + uint16_t magic_num = 0; + int r = dct_read_app_data_copy(slot_offs + offsetof(platform_flash_modules_t, magicNumber), &magic_num, + sizeof(magic_num)); + if (r != 0) { + return FLASH_ACCESS_RESULT_ERROR; + } + if (magic_num == 0xabcd) { + // Mark the slot as unused + magic_num = 0xffff; + r = dct_write_app_data(&magic_num, slot_offs + offsetof(platform_flash_modules_t, magicNumber), + sizeof(magic_num)); + if (r != 0) { + return FLASH_ACCESS_RESULT_ERROR; + } + } + slot_offs += sizeof(platform_flash_modules_t); + } + #ifdef USE_SERIAL_FLASH - FLASH_EraseMemory(FLASH_SERIAL, FLASH_Address, imageSize); + const bool ok = FLASH_EraseMemory(FLASH_SERIAL, FLASH_Address, imageSize); #else - FLASH_EraseMemory(FLASH_INTERNAL, FLASH_Address, imageSize); + const bool ok = FLASH_EraseMemory(FLASH_INTERNAL, FLASH_Address, imageSize); #endif + if (!ok) { + return FLASH_ACCESS_RESULT_ERROR; + } + + return 0; } int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize) diff --git a/platform/MCU/nRF52840/inc/flash_access.h b/platform/MCU/nRF52840/inc/flash_access.h index 0ab5a55ab9..e9a2901982 100644 --- a/platform/MCU/nRF52840/inc/flash_access.h +++ b/platform/MCU/nRF52840/inc/flash_access.h @@ -69,7 +69,7 @@ void FLASH_ClearFlags(void); void FLASH_Erase(void); void FLASH_Backup(uint32_t FLASH_Address); void FLASH_Restore(uint32_t FLASH_Address); -void FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize); +int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize); int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize); void FLASH_End(void); diff --git a/platform/MCU/nRF52840/src/flash_mal.c b/platform/MCU/nRF52840/src/flash_mal.c index eb9446b075..cbda7ba085 100644 --- a/platform/MCU/nRF52840/src/flash_mal.c +++ b/platform/MCU/nRF52840/src/flash_mal.c @@ -952,16 +952,43 @@ void FLASH_Restore(uint32_t FLASH_Address) #endif } -void FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) +int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) { system_flags.OTA_FLASHED_Status_SysFlag = 0x0000; Save_SystemFlags(); + // Clear all non-factory module slots in the DCT + const size_t slot_count = MAX_MODULES_SLOT - GEN_START_SLOT; + size_t slot_offs = DCT_FLASH_MODULES_OFFSET + sizeof(platform_flash_modules_t) * GEN_START_SLOT; + for (size_t i = 0; i < slot_count; ++i) { + uint16_t magic_num = 0; + int r = dct_read_app_data_copy(slot_offs + offsetof(platform_flash_modules_t, magicNumber), &magic_num, + sizeof(magic_num)); + if (r != 0) { + return FLASH_ACCESS_RESULT_ERROR; + } + if (magic_num == 0xabcd) { + // Mark the slot as unused + magic_num = 0xffff; + r = dct_write_app_data(&magic_num, slot_offs + offsetof(platform_flash_modules_t, magicNumber), + sizeof(magic_num)); + if (r != 0) { + return FLASH_ACCESS_RESULT_ERROR; + } + } + slot_offs += sizeof(platform_flash_modules_t); + } + #ifdef USE_SERIAL_FLASH - FLASH_EraseMemory(FLASH_SERIAL, FLASH_Address, imageSize); + const bool ok = FLASH_EraseMemory(FLASH_SERIAL, FLASH_Address, imageSize); #else - FLASH_EraseMemory(FLASH_INTERNAL, FLASH_Address, imageSize); + const bool ok = FLASH_EraseMemory(FLASH_INTERNAL, FLASH_Address, imageSize); #endif + if (!ok) { + return FLASH_ACCESS_RESULT_ERROR; + } + + return 0; } int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize) diff --git a/platform/MCU/shared/STM32/inc/flash_access.h b/platform/MCU/shared/STM32/inc/flash_access.h index 5a196b8efd..d778271c8e 100644 --- a/platform/MCU/shared/STM32/inc/flash_access.h +++ b/platform/MCU/shared/STM32/inc/flash_access.h @@ -69,7 +69,7 @@ void FLASH_ClearFlags(void); void FLASH_Erase(void); void FLASH_Backup(uint32_t FLASH_Address); void FLASH_Restore(uint32_t FLASH_Address); -void FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize); +int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize); int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize); void FLASH_End(void); int FLASH_ReadOTP(uint32_t offset, uint8_t* pBuffer, uint32_t bufferSize); From 48e02beb8cf2c04476e54ea33477f57e39e752b9 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Thu, 19 Aug 2021 23:06:59 +0200 Subject: [PATCH 2/9] Minor fixes --- hal/src/nRF52840/ota_flash_hal.cpp | 2 +- hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp | 2 +- platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c | 2 +- platform/MCU/nRF52840/src/flash_mal.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hal/src/nRF52840/ota_flash_hal.cpp b/hal/src/nRF52840/ota_flash_hal.cpp index 7f3f6941ab..e37ce76d50 100644 --- a/hal/src/nRF52840/ota_flash_hal.cpp +++ b/hal/src/nRF52840/ota_flash_hal.cpp @@ -355,7 +355,7 @@ uint16_t HAL_OTA_ChunkSize() bool HAL_FLASH_Begin(uint32_t address, uint32_t length, void* reserved) { const int r = FLASH_Begin(address, length); - if (r != 0) { + if (r != FLASH_ACCESS_RESULT_OK) { return false; } return true; diff --git a/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp b/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp index bffb4fe9ab..9ef42f8e66 100644 --- a/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp +++ b/hal/src/stm32f2xx/ota_flash_hal_stm32f2xx.cpp @@ -251,7 +251,7 @@ uint16_t HAL_OTA_ChunkSize() bool HAL_FLASH_Begin(uint32_t address, uint32_t length, void* reserved) { const int r = FLASH_Begin(address, length); - if (r != 0) { + if (r != FLASH_ACCESS_RESULT_OK) { return false; } return true; diff --git a/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c b/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c index dee2138ee0..d5ac48b26e 100644 --- a/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c +++ b/platform/MCU/STM32F2xx/SPARK_Firmware_Driver/src/flash_mal.c @@ -909,7 +909,7 @@ int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) return FLASH_ACCESS_RESULT_ERROR; } - return 0; + return FLASH_ACCESS_RESULT_OK; } int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize) diff --git a/platform/MCU/nRF52840/src/flash_mal.c b/platform/MCU/nRF52840/src/flash_mal.c index cbda7ba085..bd574b7c3c 100644 --- a/platform/MCU/nRF52840/src/flash_mal.c +++ b/platform/MCU/nRF52840/src/flash_mal.c @@ -988,7 +988,7 @@ int FLASH_Begin(uint32_t FLASH_Address, uint32_t imageSize) return FLASH_ACCESS_RESULT_ERROR; } - return 0; + return FLASH_ACCESS_RESULT_OK; } int FLASH_Update(const uint8_t *pBuffer, uint32_t address, uint32_t bufferSize) From 855dfecb455923ff4a9d0fa0a04f30f9aae4e76f Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Thu, 19 Aug 2021 23:08:19 +0200 Subject: [PATCH 3/9] Add an integration test (WIP) --- user/tests/integration/.gitignore | 1 + .../multiple_ota_no_reset.cpp | 16 + .../multiple_ota_no_reset.spec.js | 69 ++++ user/tests/integration/package-lock.json | 384 ++++++++++++++++++ user/tests/integration/package.json | 8 + 5 files changed, 478 insertions(+) create mode 100644 user/tests/integration/.gitignore create mode 100644 user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp create mode 100644 user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js create mode 100644 user/tests/integration/package-lock.json create mode 100644 user/tests/integration/package.json diff --git a/user/tests/integration/.gitignore b/user/tests/integration/.gitignore new file mode 100644 index 0000000000..2ccbe4656c --- /dev/null +++ b/user/tests/integration/.gitignore @@ -0,0 +1 @@ +/node_modules/ diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp new file mode 100644 index 0000000000..df6fd62d22 --- /dev/null +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp @@ -0,0 +1,16 @@ +#include "application.h" +#include "test.h" + +namespace { + +} // namespace + +test(01_disable_resets_and_connect) { + System.disableReset(); + Particle.connect(); + waitUntil(Particle.connected); +} + +test(02_flash_binaries) { + // See the spec file +} diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js new file mode 100644 index 0000000000..f91a478b7f --- /dev/null +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js @@ -0,0 +1,69 @@ +suite('Multiple OTA updates without a system reset'); + +platform('gen2', 'gen3'); +systemThread('enabled'); + +const { HalModuleParser, ModuleInfo, updateModulePrefix, updateModuleSuffix } = require('binary-version-reader'); +const tempy = require('tempy'); + +const { readFile } = require('fs').promises; +const path = require('path'); + +let api = null; +let auth = null; +let device = null; +let deviceId = null; + +async function waitFlashSuccessEvent(ctx) { + let data = null; + do { + data = await ctx.particle.receiveEvent('spark/flash/status'); + ctx.particle.log.verbose('spark/flash/status:', data); + } while (!data.startsWith('success')); +} + +async function flash(ctx, binFile) { + const appName = path.basename(binFile, '.bin'); + await api.flashDevice({ deviceId, files: { [appName]: binFile }, auth }); + await waitFlashSuccessEvent(ctx); +} + +before(function() { + api = this.particle.apiClient.instance; + auth = this.particle.apiClient.token; + device = this.particle.devices[0]; + deviceId = device.id; +}); + +test('01_disable_resets_and_connect', () => { + // See the test app +}); + +test('02_flash_binaries', async function () { + // Get the module binary of the test application that is currently running on the device + const origAppData = await readFile(device.testAppBinFile); + const parser = new HalModuleParser(); + const { prefixInfo: origPrefix, suffixInfo: origSuffix } = await parser.parseBuffer({ fileBuffer: origAppData }); + // The first binary is exactly the same as the one already flashed on the device but with a + // different SHA checksum + let appData = Buffer.from(origAppData); + let suffix = { ...origSuffix }; + suffix.fwUniqueId = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; + expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); + updateModuleSuffix(appData, suffix); + const appFile1 = await tempy.write(appData, { name: 'app1.bin' }); + // The second binary has a missing dependency + appData = Buffer.from(origAppData); + const prefix = { ...origPrefix }; + expect(prefix.depModuleFunction).to.equal(ModuleInfo.FunctionType.SYSTEM_PART); + ++prefix.depModuleVersion; + updateModulePrefix(appData, prefix); + suffix = { ...origSuffix }; + suffix.fwUniqueId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); + updateModuleSuffix(appData, suffix); + const appFile2 = await tempy.write(appData, { name: 'app2.bin' }); + // Flash the binaries to the device + await flash(this, appFile1); + await flash(this, appFile2); +}); diff --git a/user/tests/integration/package-lock.json b/user/tests/integration/package-lock.json new file mode 100644 index 0000000000..4e00dcd1b6 --- /dev/null +++ b/user/tests/integration/package-lock.json @@ -0,0 +1,384 @@ +{ + "requires": true, + "lockfileVersion": 1, + "dependencies": { + "@nodelib/fs.scandir": { + "version": "2.1.5", + "resolved": "https://registry.npmjs.org/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz", + "integrity": "sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==", + "requires": { + "@nodelib/fs.stat": "2.0.5", + "run-parallel": "^1.1.9" + } + }, + "@nodelib/fs.stat": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/@nodelib/fs.stat/-/fs.stat-2.0.5.tgz", + "integrity": "sha512-RkhPPp2zrqDAQA/2jNhnztcPAlv64XdhIp7a7454A5ovI7Bukxgt7MX7udwAu3zg1DcpPU0rz3VV1SeaqvY4+A==" + }, + "@nodelib/fs.walk": { + "version": "1.2.8", + "resolved": "https://registry.npmjs.org/@nodelib/fs.walk/-/fs.walk-1.2.8.tgz", + "integrity": "sha512-oGB+UxlgWcgQkgwo8GcEGwemoTFt3FIO9ababBmaGwXIoBKZ+GTy0pP185beGg7Llih/NSHSV2XAs1lnznocSg==", + "requires": { + "@nodelib/fs.scandir": "2.1.5", + "fastq": "^1.6.0" + } + }, + "aggregate-error": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/aggregate-error/-/aggregate-error-3.1.0.tgz", + "integrity": "sha512-4I7Td01quW/RpocfNayFdFVk1qSuoh0E7JrbRJ16nH01HhKFQ88INq9Sd+nd72zqRySlr9BmDA8xlEJ6vJMrYA==", + "requires": { + "clean-stack": "^2.0.0", + "indent-string": "^4.0.0" + } + }, + "array-union": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/array-union/-/array-union-2.1.0.tgz", + "integrity": "sha512-HGyxoOTYUyCM6stUe6EJgnd4EoewAI7zMdfqO+kGjnlZmBDz/cR5pf8r/cR4Wq60sL/p0IkcjUEEPwS3GFrIyw==" + }, + "balanced-match": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", + "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==" + }, + "binary-version-reader": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/binary-version-reader/-/binary-version-reader-1.0.1.tgz", + "integrity": "sha512-Dhn3W9eRuWSiK+2VOxVu+8aMmTl8jzKD1MngEJpJ/cF0BcPXWFYJHMeCKf35wq2SJGO66duChxKYnYzI1HEgvg==", + "requires": { + "buffer-crc32": "^0.2.5", + "when": "^3.7.3", + "xtend": "^4.0.2" + } + }, + "brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "requires": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "braces": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz", + "integrity": "sha512-b8um+L1RzM3WDSzvhm6gIz1yfTbBt6YTlcEKAvsmqCZZFw46z626lVj9j1yEPW33H5H+lBQpZMP1k8l+78Ha0A==", + "requires": { + "fill-range": "^7.0.1" + } + }, + "buffer-crc32": { + "version": "0.2.13", + "resolved": "https://registry.npmjs.org/buffer-crc32/-/buffer-crc32-0.2.13.tgz", + "integrity": "sha1-DTM+PwDqxQqhRUq9MO+MKl2ackI=" + }, + "clean-stack": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/clean-stack/-/clean-stack-2.2.0.tgz", + "integrity": "sha512-4diC9HaTE+KRAMWhDhrGOECgWZxoevMc5TlkObMqNSsVU62PYzXZ/SMTjzyGAFF1YusgxGcSWTEXBhp0CPwQ1A==" + }, + "concat-map": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", + "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=" + }, + "crypto-random-string": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-2.0.0.tgz", + "integrity": "sha512-v1plID3y9r/lPhviJ1wrXpLeyUIGAZ2SHNYTEapm7/8A9nLPoyvVp3RK/EPFqn5kEznyWgYZNsRtYYIWbuG8KA==" + }, + "del": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/del/-/del-6.0.0.tgz", + "integrity": "sha512-1shh9DQ23L16oXSZKB2JxpL7iMy2E0S9d517ptA1P8iw0alkPtQcrKH7ru31rYtKwF499HkTu+DRzq3TCKDFRQ==", + "requires": { + "globby": "^11.0.1", + "graceful-fs": "^4.2.4", + "is-glob": "^4.0.1", + "is-path-cwd": "^2.2.0", + "is-path-inside": "^3.0.2", + "p-map": "^4.0.0", + "rimraf": "^3.0.2", + "slash": "^3.0.0" + } + }, + "dir-glob": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz", + "integrity": "sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==", + "requires": { + "path-type": "^4.0.0" + } + }, + "fast-glob": { + "version": "3.2.7", + "resolved": "https://registry.npmjs.org/fast-glob/-/fast-glob-3.2.7.tgz", + "integrity": "sha512-rYGMRwip6lUMvYD3BTScMwT1HtAs2d71SMv66Vrxs0IekGZEjhM0pcMfjQPnknBt2zeCwQMEupiN02ZP4DiT1Q==", + "requires": { + "@nodelib/fs.stat": "^2.0.2", + "@nodelib/fs.walk": "^1.2.3", + "glob-parent": "^5.1.2", + "merge2": "^1.3.0", + "micromatch": "^4.0.4" + } + }, + "fastq": { + "version": "1.12.0", + "resolved": "https://registry.npmjs.org/fastq/-/fastq-1.12.0.tgz", + "integrity": "sha512-VNX0QkHK3RsXVKr9KrlUv/FoTa0NdbYoHHl7uXHv2rzyHSlxjdNAKug2twd9luJxpcyNeAgf5iPPMutJO67Dfg==", + "requires": { + "reusify": "^1.0.4" + } + }, + "fill-range": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.0.1.tgz", + "integrity": "sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==", + "requires": { + "to-regex-range": "^5.0.1" + } + }, + "fs.realpath": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", + "integrity": "sha1-FQStJSMVjKpA20onh8sBQRmU6k8=" + }, + "glob": { + "version": "7.1.7", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.7.tgz", + "integrity": "sha512-OvD9ENzPLbegENnYP5UUfJIirTg4+XwMWGaQfQTY0JenxNvvIKP3U3/tAQSPIu/lHxXYSZmpXlUHeqAIdKzBLQ==", + "requires": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.0.4", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + } + }, + "glob-parent": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", + "requires": { + "is-glob": "^4.0.1" + } + }, + "globby": { + "version": "11.0.4", + "resolved": "https://registry.npmjs.org/globby/-/globby-11.0.4.tgz", + "integrity": "sha512-9O4MVG9ioZJ08ffbcyVYyLOJLk5JQ688pJ4eMGLpdWLHq/Wr1D9BlriLQyL0E+jbkuePVZXYFj47QM/v093wHg==", + "requires": { + "array-union": "^2.1.0", + "dir-glob": "^3.0.1", + "fast-glob": "^3.1.1", + "ignore": "^5.1.4", + "merge2": "^1.3.0", + "slash": "^3.0.0" + } + }, + "graceful-fs": { + "version": "4.2.8", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.8.tgz", + "integrity": "sha512-qkIilPUYcNhJpd33n0GBXTB1MMPp14TxEsEs0pTrsSVucApsYzW5V+Q8Qxhik6KU3evy+qkAAowTByymK0avdg==" + }, + "ignore": { + "version": "5.1.8", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.1.8.tgz", + "integrity": "sha512-BMpfD7PpiETpBl/A6S498BaIJ6Y/ABT93ETbby2fP00v4EbvPBXWEoaR1UBPKs3iR53pJY7EtZk5KACI57i1Uw==" + }, + "indent-string": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/indent-string/-/indent-string-4.0.0.tgz", + "integrity": "sha512-EdDDZu4A2OyIK7Lr/2zG+w5jmbuk1DVBnEwREQvBzspBJkCEbRa8GxU1lghYcaGJCnRWibjDXlq779X1/y5xwg==" + }, + "inflight": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", + "integrity": "sha1-Sb1jMdfQLQwJvJEKEHW6gWW1bfk=", + "requires": { + "once": "^1.3.0", + "wrappy": "1" + } + }, + "inherits": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", + "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" + }, + "is-extglob": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/is-extglob/-/is-extglob-2.1.1.tgz", + "integrity": "sha1-qIwCU1eR8C7TfHahueqXc8gz+MI=" + }, + "is-glob": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/is-glob/-/is-glob-4.0.1.tgz", + "integrity": "sha512-5G0tKtBTFImOqDnLB2hG6Bp2qcKEFduo4tZu9MT/H6NQv/ghhy30o55ufafxJ/LdH79LLs2Kfrn85TLKyA7BUg==", + "requires": { + "is-extglob": "^2.1.1" + } + }, + "is-number": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz", + "integrity": "sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==" + }, + "is-path-cwd": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/is-path-cwd/-/is-path-cwd-2.2.0.tgz", + "integrity": "sha512-w942bTcih8fdJPJmQHFzkS76NEP8Kzzvmw92cXsazb8intwLqPibPPdXf4ANdKV3rYMuuQYGIWtvz9JilB3NFQ==" + }, + "is-path-inside": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/is-path-inside/-/is-path-inside-3.0.3.tgz", + "integrity": "sha512-Fd4gABb+ycGAmKou8eMftCupSir5lRxqf4aD/vd0cD2qc4HL07OjCeuHMr8Ro4CoMaeCKDB0/ECBOVWjTwUvPQ==" + }, + "is-stream": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-2.0.1.tgz", + "integrity": "sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg==" + }, + "merge2": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/merge2/-/merge2-1.4.1.tgz", + "integrity": "sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==" + }, + "micromatch": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.4.tgz", + "integrity": "sha512-pRmzw/XUcwXGpD9aI9q/0XOwLNygjETJ8y0ao0wdqprrzDa4YnxLcz7fQRZr8voh8V10kGhABbNcHVk5wHgWwg==", + "requires": { + "braces": "^3.0.1", + "picomatch": "^2.2.3" + } + }, + "minimatch": { + "version": "3.0.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", + "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", + "requires": { + "brace-expansion": "^1.1.7" + } + }, + "once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "requires": { + "wrappy": "1" + } + }, + "p-map": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/p-map/-/p-map-4.0.0.tgz", + "integrity": "sha512-/bjOqmgETBYB5BoEeGVea8dmvHb2m9GLy1E9W43yeyfP6QQCZGFNa+XRceJEuDB6zqr+gKpIAmlLebMpykw/MQ==", + "requires": { + "aggregate-error": "^3.0.0" + } + }, + "path-is-absolute": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", + "integrity": "sha1-F0uSaHNVNP+8es5r9TpanhtcX18=" + }, + "path-type": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/path-type/-/path-type-4.0.0.tgz", + "integrity": "sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==" + }, + "picomatch": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.0.tgz", + "integrity": "sha512-lY1Q/PiJGC2zOv/z391WOTD+Z02bCgsFfvxoXXf6h7kv9o+WmsmzYqrAwY63sNgOxE4xEdq0WyUnXfKeBrSvYw==" + }, + "queue-microtask": { + "version": "1.2.3", + "resolved": "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz", + "integrity": "sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A==" + }, + "reusify": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/reusify/-/reusify-1.0.4.tgz", + "integrity": "sha512-U9nH88a3fc/ekCF1l0/UP1IosiuIjyTh7hBvXVMHYgVcfGvt897Xguj2UOLDeI5BG2m7/uwyaLVT6fbtCwTyzw==" + }, + "rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "requires": { + "glob": "^7.1.3" + } + }, + "run-parallel": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/run-parallel/-/run-parallel-1.2.0.tgz", + "integrity": "sha512-5l4VyZR86LZ/lDxZTR6jqL8AFE2S0IFLMP26AbjsLVADxHdhB/c0GUsH+y39UfCi3dzz8OlQuPmnaJOMoDHQBA==", + "requires": { + "queue-microtask": "^1.2.2" + } + }, + "slash": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz", + "integrity": "sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==" + }, + "temp-dir": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/temp-dir/-/temp-dir-2.0.0.tgz", + "integrity": "sha512-aoBAniQmmwtcKp/7BzsH8Cxzv8OL736p7v1ihGb5e9DJ9kTwGWHrQrVB5+lfVDzfGrdRzXch+ig7LHaY1JTOrg==" + }, + "tempy": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/tempy/-/tempy-1.0.1.tgz", + "integrity": "sha512-biM9brNqxSc04Ee71hzFbryD11nX7VPhQQY32AdDmjFvodsRFz/3ufeoTZ6uYkRFfGo188tENcASNs3vTdsM0w==", + "requires": { + "del": "^6.0.0", + "is-stream": "^2.0.0", + "temp-dir": "^2.0.0", + "type-fest": "^0.16.0", + "unique-string": "^2.0.0" + } + }, + "to-regex-range": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", + "integrity": "sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==", + "requires": { + "is-number": "^7.0.0" + } + }, + "type-fest": { + "version": "0.16.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.16.0.tgz", + "integrity": "sha512-eaBzG6MxNzEn9kiwvtre90cXaNLkmadMWa1zQMs3XORCXNbsH/OewwbxC5ia9dCxIxnTAsSxXJaa/p5y8DlvJg==" + }, + "unique-string": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/unique-string/-/unique-string-2.0.0.tgz", + "integrity": "sha512-uNaeirEPvpZWSgzwsPGtU2zVSTrn/8L5q/IexZmH0eH6SA73CmAA5U4GwORTxQAZs95TAXLNqeLoPPNO5gZfWg==", + "requires": { + "crypto-random-string": "^2.0.0" + } + }, + "when": { + "version": "3.7.8", + "resolved": "https://registry.npmjs.org/when/-/when-3.7.8.tgz", + "integrity": "sha1-xxMLan6gRpPoQs3J56Hyqjmjn4I=" + }, + "wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + }, + "xtend": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/xtend/-/xtend-4.0.2.tgz", + "integrity": "sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ==" + } + } +} diff --git a/user/tests/integration/package.json b/user/tests/integration/package.json new file mode 100644 index 0000000000..a24126c8bd --- /dev/null +++ b/user/tests/integration/package.json @@ -0,0 +1,8 @@ +{ + "description": "Device OS Integration Tests", + "private": true, + "dependencies": { + "binary-version-reader": "^1.0.1", + "tempy": "^1.0.1" + } +} From a46225029da47db1afe7ebc3bf627edd968ddd51 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Fri, 20 Aug 2021 17:55:50 +0200 Subject: [PATCH 4/9] Add particle::toHex() --- services/inc/str_util.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/services/inc/str_util.h b/services/inc/str_util.h index c44653884c..5a8653b696 100644 --- a/services/inc/str_util.h +++ b/services/inc/str_util.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include namespace particle { @@ -102,4 +103,32 @@ inline bool endsWith(const char* str, const char* suffix) { */ size_t escape(const char* src, const char* spec, char esc, char* dest, size_t destSize); +/** + * Converts binary data to a hex-encoded string. The output is always null-terminated, unless the + * size of the destination buffer is 0. + * + * @param src Source data. + * @param srcSize Size of the source data. + * @param dest Destination buffer. + * @param destSize Size of the destination buffer. + * + * @return Number of characters written to the destination buffer, not including the trailing `\0`. + */ +inline size_t toHex(const void* src, size_t srcSize, char* dest, size_t destSize) { + static const char alpha[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + size_t n = 0; + auto srcBytes = (const uint8_t*)src; + for (size_t i = 0; i < srcSize && n + 1 < destSize; ++i) { + const auto b = srcBytes[i]; + dest[n++] = alpha[b >> 4]; + dest[n++] = alpha[b & 0x0f]; + } + if (n < destSize) { + dest[n] = '\0'; + } else if (destSize > 0) { + dest[destSize - 1] = '\0'; + } + return n; +} + } // particle From efc2daa44082c6166d36225f7e570ab6e24b563c Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Fri, 20 Aug 2021 17:56:36 +0200 Subject: [PATCH 5/9] Refactor the integration test --- .../multiple_ota_no_reset.cpp | 34 +++++++++++++- .../multiple_ota_no_reset.spec.js | 47 ++++++++++++------- 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp index df6fd62d22..98d7ff236f 100644 --- a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp @@ -1,16 +1,48 @@ #include "application.h" #include "test.h" +#include "str_util.h" + namespace { +retained char origAppHash[65] = {}; + +bool getAppHash(char* buf, size_t size) { + hal_system_info_t info = {}; + info.size = sizeof(info); + const int r = system_info_get(&info, 0 /* flags */, nullptr /* reserved */); + if (r != 0) { + return false; + } + SCOPE_GUARD({ + system_info_free(&info, 0 /* flags */, nullptr /* reserved */); + }); + for (size_t i = 0; i < info.module_count; ++i) { + const auto& module = info.modules[i]; + if (module.info.module_function == MODULE_FUNCTION_USER_PART) { + toHex(module.suffix.sha, sizeof(module.suffix.sha), buf, size); + return true; + } + } + return false; +} + } // namespace test(01_disable_resets_and_connect) { + assertTrue(getAppHash(origAppHash, sizeof(origAppHash))); System.disableReset(); Particle.connect(); waitUntil(Particle.connected); } -test(02_flash_binaries) { +test(02_flash_binaries_and_reset) { // See the spec file } + +test(03_validate_module_info) { + char appHash[65] = {}; + assertTrue(getAppHash(appHash, sizeof(appHash))); + // The app hash should not have changed + assertEqual(strcmp(appHash, origAppHash), 0); +} diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js index f91a478b7f..8872d432ce 100644 --- a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js @@ -14,18 +14,23 @@ let auth = null; let device = null; let deviceId = null; -async function waitFlashSuccessEvent(ctx) { +async function flash(ctx, binFile, { expectedStatus = 'success' } = {}) { + const appName = path.basename(binFile, '.bin'); + await api.flashDevice({ deviceId, files: { [appName]: binFile }, auth }); + await waitFlashStatusEvent(ctx, expectedStatus); + await delay(1000); +} + +async function waitFlashStatusEvent(ctx, status) { let data = null; do { data = await ctx.particle.receiveEvent('spark/flash/status'); ctx.particle.log.verbose('spark/flash/status:', data); - } while (!data.startsWith('success')); + } while (!data.startsWith(status)); } -async function flash(ctx, binFile) { - const appName = path.basename(binFile, '.bin'); - await api.flashDevice({ deviceId, files: { [appName]: binFile }, auth }); - await waitFlashSuccessEvent(ctx); +async function delay(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); } before(function() { @@ -35,35 +40,41 @@ before(function() { deviceId = device.id; }); -test('01_disable_resets_and_connect', () => { +test('01_disable_resets_and_connect', async function () { // See the test app }); -test('02_flash_binaries', async function () { +test('02_flash_binaries_and_reset', async function () { // Get the module binary of the test application that is currently running on the device const origAppData = await readFile(device.testAppBinFile); const parser = new HalModuleParser(); const { prefixInfo: origPrefix, suffixInfo: origSuffix } = await parser.parseBuffer({ fileBuffer: origAppData }); - // The first binary is exactly the same as the one already flashed on the device but with a - // different SHA checksum + // The first binary is exactly the same as the one already flashed on the device but it has a + // different SHA checksum so that we can identify it later let appData = Buffer.from(origAppData); let suffix = { ...origSuffix }; suffix.fwUniqueId = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); updateModuleSuffix(appData, suffix); - const appFile1 = await tempy.write(appData, { name: 'app1.bin' }); - // The second binary has a missing dependency + let appFile = await tempy.write(appData, { name: 'app1.bin' }); + // The device should accept this update + await flash(this, appFile); + // The second binary has an incompatible platform ID appData = Buffer.from(origAppData); const prefix = { ...origPrefix }; - expect(prefix.depModuleFunction).to.equal(ModuleInfo.FunctionType.SYSTEM_PART); - ++prefix.depModuleVersion; + ++prefix.platformID; updateModulePrefix(appData, prefix); suffix = { ...origSuffix }; suffix.fwUniqueId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); updateModuleSuffix(appData, suffix); - const appFile2 = await tempy.write(appData, { name: 'app2.bin' }); - // Flash the binaries to the device - await flash(this, appFile1); - await flash(this, appFile2); + appFile = await tempy.write(appData, { name: 'app2.bin' }); + // The device should reject this update as well as clear the previously received one + await flash(this, appFile, { expectedStatus: 'failed' }); + // Reset the device so that it can apply pending updates (it shouldn't have any) + await device.reset(); +}); + +test('03_validate_module_info', async function () { + // See the test app }); From 8e26d3dbee07203ac9bf19d519c1b6c4bd7249b3 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Mon, 23 Aug 2021 20:26:41 +0200 Subject: [PATCH 6/9] Allow disabling compressed OTA updates; fix the integration test --- communication/inc/protocol.h | 8 ++- communication/inc/protocol_defs.h | 4 +- .../src/spark_protocol_functions.cpp | 4 +- services/inc/str_util.h | 6 +- system/src/system_cloud_internal.cpp | 4 +- .../multiple_ota_no_reset.cpp | 39 +++++++++++-- .../multiple_ota_no_reset.spec.js | 58 +++++++++++++------ 7 files changed, 89 insertions(+), 34 deletions(-) diff --git a/communication/inc/protocol.h b/communication/inc/protocol.h index a323d6bac3..06213d2401 100644 --- a/communication/inc/protocol.h +++ b/communication/inc/protocol.h @@ -385,9 +385,13 @@ class Protocol protocol_flags |= ProtocolFlag::DEVICE_INITIATED_DESCRIBE; } - void enable_compressed_ota() + void set_compressed_ota_enabled(bool enabled) { - protocol_flags |= ProtocolFlag::COMPRESSED_OTA; + if (enabled) { + protocol_flags |= ProtocolFlag::COMPRESSED_OTA; + } else { + protocol_flags &= ~ProtocolFlag::COMPRESSED_OTA; + } } void set_system_version(uint16_t version) diff --git a/communication/inc/protocol_defs.h b/communication/inc/protocol_defs.h index b690c5d38e..1a0b0f78de 100644 --- a/communication/inc/protocol_defs.h +++ b/communication/inc/protocol_defs.h @@ -149,8 +149,8 @@ enum Enum { PING = 0, ///< Keepalive interval (set). FAST_OTA = 1, ///< Enable/disable fast OTA (set). - DEVICE_INITIATED_DESCRIBE = 2, ///< Enable device-initiated describe messages (set). - COMPRESSED_OTA = 3, ///< Enable support for compressed/combined OTA updates (set). + ENABLE_DEVICE_INITIATED_DESCRIBE = 2, ///< Enable device-initiated describe messages (set). + COMPRESSED_OTA = 3, ///< Enable/disable support for compressed/combined OTA updates (set). SYSTEM_MODULE_VERSION = 4, ///< Module version of the system firmware (set). MAX_BINARY_SIZE = 5, ///< Maximum size of a firmware binary (set). OTA_CHUNK_SIZE = 6, ///< Size of an OTA update chunk (set). diff --git a/communication/src/spark_protocol_functions.cpp b/communication/src/spark_protocol_functions.cpp index ac3dddc733..bed2f7d097 100644 --- a/communication/src/spark_protocol_functions.cpp +++ b/communication/src/spark_protocol_functions.cpp @@ -205,12 +205,12 @@ int spark_protocol_set_connection_property(ProtocolFacade* protocol, unsigned pr protocol->set_fast_ota(value); return 0; } - case Connection::DEVICE_INITIATED_DESCRIBE: { + case Connection::ENABLE_DEVICE_INITIATED_DESCRIBE: { protocol->enable_device_initiated_describe(); return 0; } case Connection::COMPRESSED_OTA: { - protocol->enable_compressed_ota(); + protocol->set_compressed_ota_enabled(value); return 0; } case Connection::SYSTEM_MODULE_VERSION: { diff --git a/services/inc/str_util.h b/services/inc/str_util.h index 5a8653b696..0ca48e6373 100644 --- a/services/inc/str_util.h +++ b/services/inc/str_util.h @@ -118,10 +118,12 @@ inline size_t toHex(const void* src, size_t srcSize, char* dest, size_t destSize static const char alpha[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; size_t n = 0; auto srcBytes = (const uint8_t*)src; - for (size_t i = 0; i < srcSize && n + 1 < destSize; ++i) { + for (size_t i = 0; i < srcSize && n < destSize; ++i) { const auto b = srcBytes[i]; dest[n++] = alpha[b >> 4]; - dest[n++] = alpha[b & 0x0f]; + if (n < destSize) { + dest[n++] = alpha[b & 0x0f]; + } } if (n < destSize) { dest[n] = '\0'; diff --git a/system/src/system_cloud_internal.cpp b/system/src/system_cloud_internal.cpp index 30a3235871..30d0b6ebc8 100644 --- a/system/src/system_cloud_internal.cpp +++ b/system/src/system_cloud_internal.cpp @@ -1079,12 +1079,12 @@ void Spark_Protocol_Init(void) spark_protocol_init(sp, (const char*) id, keys, callbacks, descriptor); // Enable device-initiated describe messages - spark_protocol_set_connection_property(sp, protocol::Connection::DEVICE_INITIATED_DESCRIBE, 0, nullptr, nullptr); + spark_protocol_set_connection_property(sp, protocol::Connection::ENABLE_DEVICE_INITIATED_DESCRIBE, 0, nullptr, nullptr); #if HAL_PLATFORM_COMPRESSED_OTA // Enable compressed/combined OTA updates if (bootloader_get_version() >= COMPRESSED_OTA_MIN_BOOTLOADER_VERSION) { - spark_protocol_set_connection_property(sp, protocol::Connection::COMPRESSED_OTA, 0, nullptr, nullptr); + spark_protocol_set_connection_property(sp, protocol::Connection::COMPRESSED_OTA, 1, nullptr, nullptr); } #endif // HAL_PLATFORM_COMPRESSED_OTA diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp index 98d7ff236f..8b286fce98 100644 --- a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp @@ -1,11 +1,23 @@ #include "application.h" #include "test.h" +#include "storage_hal.h" +#include "scope_guard.h" #include "str_util.h" namespace { -retained char origAppHash[65] = {}; +#if PLATFORM_GEN == 3 +const auto OTA_SECTION_STORAGE = HAL_STORAGE_ID_EXTERNAL_FLASH; +const auto OTA_SECTION_ADDRESS = EXTERNAL_FLASH_OTA_ADDRESS; +#elif PLATFORM_ID == PLATFORM_ELECTRON || PLATFORM_ID == PLATFORM_PHOTON || PLATFORM_ID == PLATFORM_P1 +const auto OTA_SECTION_STORAGE = HAL_STORAGE_ID_INTERNAL_FLASH; +const auto OTA_SECTION_ADDRESS = 0x080c0000; // See the platform's ota_module_bounds.c +#else +#error "Unsupported platform" +#endif + +retained char origAppHash[65] = {}; // Hex-encoded bool getAppHash(char* buf, size_t size) { hal_system_info_t info = {}; @@ -15,7 +27,7 @@ bool getAppHash(char* buf, size_t size) { return false; } SCOPE_GUARD({ - system_info_free(&info, 0 /* flags */, nullptr /* reserved */); + system_info_free(&info, nullptr /* reserved */); }); for (size_t i = 0; i < info.module_count; ++i) { const auto& module = info.modules[i]; @@ -30,17 +42,32 @@ bool getAppHash(char* buf, size_t size) { } // namespace test(01_disable_resets_and_connect) { - assertTrue(getAppHash(origAppHash, sizeof(origAppHash))); +#if HAL_PLATFORM_COMPRESSED_OTA + // Disable compressed OTA updates so that it's easier to mess with module checksums + spark_protocol_set_connection_property(spark_protocol_instance(), protocol::Connection::COMPRESSED_OTA, + 0 /* value */, nullptr /* data */, nullptr /* reserved */); +#endif System.disableReset(); Particle.connect(); waitUntil(Particle.connected); } -test(02_flash_binaries_and_reset) { - // See the spec file +test(02_flash_binaries) { + // Get the original app hash prior to sending any updates to the device + assertTrue(getAppHash(origAppHash, sizeof(origAppHash))); +} + +test(03_fix_ota_binary_and_reset) { + // The original issue (https://github.com/particle-iot/device-os/pull/2346) can't be reproduced + // easily by flashing application binaries. As a workaround, we're restoring the correct platform + // ID in the binary that is currently stored in the OTA section so that the bootloader can apply + // it if there still happens to be a valid module slot in the DCT + const uint16_t id = PLATFORM_ID; + assertEqual(hal_storage_write(OTA_SECTION_STORAGE, OTA_SECTION_ADDRESS + offsetof(module_info_t, platform_id), + (const uint8_t*)&id, sizeof(id)), sizeof(id)); } -test(03_validate_module_info) { +test(04_validate_module_info) { char appHash[65] = {}; assertTrue(getAppHash(appHash, sizeof(appHash))); // The app hash should not have changed diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js index 8872d432ce..5b189a67d3 100644 --- a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.spec.js @@ -1,9 +1,9 @@ -suite('Multiple OTA updates without a system reset'); +suite('Multiple OTA updates with disabled resets'); platform('gen2', 'gen3'); systemThread('enabled'); -const { HalModuleParser, ModuleInfo, updateModulePrefix, updateModuleSuffix } = require('binary-version-reader'); +const { HalModuleParser, ModuleInfo, updateModulePrefix, updateModuleSuffix, updateModuleCrc32 } = require('binary-version-reader'); const tempy = require('tempy'); const { readFile } = require('fs').promises; @@ -14,19 +14,33 @@ let auth = null; let device = null; let deviceId = null; -async function flash(ctx, binFile, { expectedStatus = 'success' } = {}) { +async function flash(ctx, binFile, { timeout = 120000, mayFail = false } = {}) { const appName = path.basename(binFile, '.bin'); await api.flashDevice({ deviceId, files: { [appName]: binFile }, auth }); - await waitFlashStatusEvent(ctx, expectedStatus); - await delay(1000); + const ok = await waitFlashStatusEvent(ctx, timeout); + if (!ok && !mayFail) { + throw new Error('Update failed'); + } + return ok; } -async function waitFlashStatusEvent(ctx, status) { +async function waitFlashStatusEvent(ctx, timeout) { + let timeoutAt = Date.now() + timeout; let data = null; - do { - data = await ctx.particle.receiveEvent('spark/flash/status'); + for (;;) { + const t = timeoutAt - Date.now(); + if (t <= 0) { + throw new Error("Event timeout"); + } + data = await ctx.particle.receiveEvent('spark/flash/status', { timeout: t }); ctx.particle.log.verbose('spark/flash/status:', data); - } while (!data.startsWith(status)); + if (data.startsWith('success')) { + return true; + } + if (data.startsWith('failed')) { + return false; + } + } } async function delay(ms) { @@ -44,8 +58,8 @@ test('01_disable_resets_and_connect', async function () { // See the test app }); -test('02_flash_binaries_and_reset', async function () { - // Get the module binary of the test application that is currently running on the device +test('02_flash_binaries', async function () { + // Get the module binary of the test app that is currently running on the device const origAppData = await readFile(device.testAppBinFile); const parser = new HalModuleParser(); const { prefixInfo: origPrefix, suffixInfo: origSuffix } = await parser.parseBuffer({ fileBuffer: origAppData }); @@ -56,25 +70,33 @@ test('02_flash_binaries_and_reset', async function () { suffix.fwUniqueId = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); updateModuleSuffix(appData, suffix); + updateModuleCrc32(appData); let appFile = await tempy.write(appData, { name: 'app1.bin' }); // The device should accept this update await flash(this, appFile); - // The second binary has an incompatible platform ID + await delay(2000); + // The second binary has an incompatible platform ID and an invalid CRC checksum appData = Buffer.from(origAppData); - const prefix = { ...origPrefix }; - ++prefix.platformID; - updateModulePrefix(appData, prefix); suffix = { ...origSuffix }; suffix.fwUniqueId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; expect(suffix.fwUniqueId).to.not.equal(origSuffix.fwUniqueId); updateModuleSuffix(appData, suffix); + updateModuleCrc32(appData); + const prefix = { ...origPrefix }; + // The platform ID has all bits set so that the app can overwrite it without erasing the flash + prefix.platformID = 0xffff; + updateModulePrefix(appData, prefix); appFile = await tempy.write(appData, { name: 'app2.bin' }); - // The device should reject this update as well as clear the previously received one - await flash(this, appFile, { expectedStatus: 'failed' }); + // The device may accept or reject this update depending on the platform and Device OS version. + // In either case, the previously received update should be invalidated + await flash(this, appFile, { mayFail: true }); +}); + +test('03_fix_ota_binary_and_reset', async function () { // Reset the device so that it can apply pending updates (it shouldn't have any) await device.reset(); }); -test('03_validate_module_info', async function () { +test('04_validate_module_info', async function () { // See the test app }); From 3f20f709ea0749c9f32117c94753c7350c9439e6 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Tue, 24 Aug 2021 18:28:53 +0200 Subject: [PATCH 7/9] Add unit tests; minor fixes --- services/inc/str_util.h | 6 ++-- test/unit_tests/services/str_util.cpp | 45 +++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/services/inc/str_util.h b/services/inc/str_util.h index 0ca48e6373..318dd01e04 100644 --- a/services/inc/str_util.h +++ b/services/inc/str_util.h @@ -118,17 +118,15 @@ inline size_t toHex(const void* src, size_t srcSize, char* dest, size_t destSize static const char alpha[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; size_t n = 0; auto srcBytes = (const uint8_t*)src; - for (size_t i = 0; i < srcSize && n < destSize; ++i) { + for (size_t i = 0; i < srcSize && n + 1 < destSize; ++i) { const auto b = srcBytes[i]; dest[n++] = alpha[b >> 4]; - if (n < destSize) { + if (n + 1 < destSize) { dest[n++] = alpha[b & 0x0f]; } } if (n < destSize) { dest[n] = '\0'; - } else if (destSize > 0) { - dest[destSize - 1] = '\0'; } return n; } diff --git a/test/unit_tests/services/str_util.cpp b/test/unit_tests/services/str_util.cpp index 023b201d50..0c67372ba8 100644 --- a/test/unit_tests/services/str_util.cpp +++ b/test/unit_tests/services/str_util.cpp @@ -22,7 +22,7 @@ using namespace particle; TEST_CASE("escape()") { - char buf[10] = {}; + char buf[100] = {}; SECTION("source string can be empty") { auto n = escape("", "", '@', buf, sizeof(buf)); @@ -50,7 +50,7 @@ TEST_CASE("escape()") { CHECK(strcmp(buf, "abcd") == 0); } SECTION("destination buffer can be smaller than the source string") { - auto n = escape("abcdefghijklmnop", "ap", '@', buf, sizeof(buf)); + auto n = escape("abcdefghijklmnop", "ap", '@', buf, 10); CHECK(n == 18); // Size of the entire escaped string CHECK(strcmp(buf, "@abcdefgh") == 0); } @@ -59,3 +59,44 @@ TEST_CASE("escape()") { CHECK(n == 6); } } + +TEST_CASE("toHex()") { + char buf[100] = {}; + + SECTION("source data can be empty") { + auto n = toHex(nullptr, 0, buf, sizeof(buf)); + CHECK(n == 0); + } + SECTION("null-terminates the destination buffer") { + memset(buf, 0xff, sizeof(buf)); + auto n = toHex(nullptr, 0, buf, sizeof(buf)); + CHECK(n == 0); + CHECK(buf[0] == '\0'); + } + SECTION("converts data to hex correctly") { + auto n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, sizeof(buf)); + CHECK(n == 16); + CHECK(strcmp(buf, "0123456789abcdef") == 0); + } + SECTION("destination buffer can be smaller than necessary to store the entire string") { + memset(buf, 0xff, sizeof(buf)); + auto n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, 1); + CHECK(n == 0); + CHECK(strcmp(buf, "") == 0); + n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, 2); + CHECK(n == 1); + CHECK(strcmp(buf, "0") == 0); + n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, 3); + CHECK(n == 2); + CHECK(strcmp(buf, "01") == 0); + n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, 4); + CHECK(n == 3); + CHECK(strcmp(buf, "012") == 0); + } + SECTION("destination buffer can be empty") { + memset(buf, 0xff, sizeof(buf)); + auto n = toHex("\x01\x23\x45\x67\x89\xab\xcd\xef", 8, buf, 0); + CHECK(n == 0); + CHECK((uint8_t)buf[0] == 0xff); + } +} From 6ede759ff62da76d7e10d11e84f288964db41496 Mon Sep 17 00:00:00 2001 From: Sergey Polyakov Date: Tue, 24 Aug 2021 19:58:13 +0200 Subject: [PATCH 8/9] Minor fix --- .../ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp index 8b286fce98..6abb70aacb 100644 --- a/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp +++ b/user/tests/integration/ota/multiple_ota_no_reset/multiple_ota_no_reset.cpp @@ -22,12 +22,12 @@ retained char origAppHash[65] = {}; // Hex-encoded bool getAppHash(char* buf, size_t size) { hal_system_info_t info = {}; info.size = sizeof(info); - const int r = system_info_get(&info, 0 /* flags */, nullptr /* reserved */); + const int r = system_info_get_unstable(&info, 0 /* flags */, nullptr /* reserved */); if (r != 0) { return false; } SCOPE_GUARD({ - system_info_free(&info, nullptr /* reserved */); + system_info_free_unstable(&info, nullptr /* reserved */); }); for (size_t i = 0; i < info.module_count; ++i) { const auto& module = info.modules[i]; From 30864f2d438f2e66f714f8df8a953882da822d76 Mon Sep 17 00:00:00 2001 From: Andrey Tolstoy Date: Wed, 25 Aug 2021 03:02:13 +0700 Subject: [PATCH 9/9] [test] wiring/no_fixture: minor refactoring to cut down on size --- user/tests/wiring/no_fixture/system.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/user/tests/wiring/no_fixture/system.cpp b/user/tests/wiring/no_fixture/system.cpp index 0fff1a5842..802645f6f4 100644 --- a/user/tests/wiring/no_fixture/system.cpp +++ b/user/tests/wiring/no_fixture/system.cpp @@ -171,7 +171,8 @@ test(SYSTEM_06_system_describe_is_not_overflowed_when_factory_module_present) assertTrue(waitFor(Particle.disconnected, 1000)); // Copy current user-part into factory location auto storageId = factory->bounds.location == MODULE_BOUNDS_LOC_EXTERNAL_FLASH ? HAL_STORAGE_ID_EXTERNAL_FLASH : HAL_STORAGE_ID_INTERNAL_FLASH; - assertEqual(factory->bounds.maximum_size, hal_storage_erase(storageId, factory->bounds.start_address, factory->bounds.maximum_size)); + int r = hal_storage_erase(storageId, factory->bounds.start_address, factory->bounds.maximum_size); + assertEqual(factory->bounds.maximum_size, r); module_info_t patchedModuleInfo = user->info; module_info_suffix_t patchedModuleSuffix = user->suffix; patchedModuleInfo.module_version = 123; @@ -195,20 +196,25 @@ test(SYSTEM_06_system_describe_is_not_overflowed_when_factory_module_present) while (pos < user->module_info_offset) { size_t sz = std::min(sizeof(buf), user->module_info_offset - pos); memcpy(buf, (const char*)user->bounds.start_address + pos, sz); - assertEqual(sz, hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz)); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz); + assertEqual(sz, r); pos += sz; } - assertEqual(sizeof(patchedModuleInfo), hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&patchedModuleInfo, sizeof(patchedModuleInfo))); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&patchedModuleInfo, sizeof(patchedModuleInfo)); + assertEqual(sizeof(patchedModuleInfo), r); pos += sizeof(patchedModuleInfo); while (pos < userModuleSize - sizeof(patchedModuleSuffix)) { size_t sz = std::min(sizeof(buf), userModuleSize - sizeof(patchedModuleSuffix) - pos); memcpy(buf, (const char*)user->bounds.start_address + pos, sz); - assertEqual(sz, hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz)); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz); + assertEqual(sz, r); pos += sz; } - assertEqual(sizeof(patchedModuleSuffix), hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&patchedModuleSuffix, sizeof(patchedModuleSuffix))); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&patchedModuleSuffix, sizeof(patchedModuleSuffix)); + assertEqual(sizeof(patchedModuleSuffix), r); pos += sizeof(patchedModuleSuffix); - assertEqual(sizeof(crc), hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&crc, sizeof(crc))); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)&crc, sizeof(crc)); + assertEqual(sizeof(crc), r); // Re-request module info to check that the factory binary is in fact valid now system_info_free_unstable(&info, nullptr); @@ -257,12 +263,14 @@ test(SYSTEM_07_system_describe_is_not_overflowed_when_factory_module_present_but // Copy HALF of current user-part into factory location size_t toCopy = std::min(((uintptr_t)user->info.module_end_address - (uintptr_t)user->info.module_start_address) / 2, factory->bounds.maximum_size); auto storageId = factory->bounds.location == MODULE_BOUNDS_LOC_EXTERNAL_FLASH ? HAL_STORAGE_ID_EXTERNAL_FLASH : HAL_STORAGE_ID_INTERNAL_FLASH; - assertEqual(factory->bounds.maximum_size, hal_storage_erase(storageId, factory->bounds.start_address, factory->bounds.maximum_size)); + int r = hal_storage_erase(storageId, factory->bounds.start_address, factory->bounds.maximum_size); + assertEqual(factory->bounds.maximum_size, r); char buf[256]; // some platforms cannot write into e.g. an external flash directly from internal for (size_t pos = 0; pos < toCopy; pos += sizeof(buf)) { size_t sz = std::min(sizeof(buf), toCopy - pos); memcpy(buf, (const char*)user->bounds.start_address + pos, sz); - assertEqual(sz, hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz)); + r = hal_storage_write(storageId, factory->bounds.start_address + pos, (const uint8_t*)buf, sz); + assertEqual(sz, r); } // Re-request module info to check that the factory binary is in fact invalid now system_info_free_unstable(&info, nullptr);