Fix CI failures and address code review comments

- Add unit tests for new storage APIs added

config/standalone/CHIPProjectConfig.h

@@ -74,7 +74,7 @@
 #endif
 
 #ifndef CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING
-#define CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING "v1.0"
+#define CHIP_DEVICE_CONFIG_DEVICE_SOFTWARE_VERSION_STRING "1.0"
 #endif
 
 //

examples/lighting-app/nxp/k32w/k32w0/main/AppTask.cpp

@@ -127,7 +127,7 @@ CHIP_ERROR AppTask::Init()
     gRequestorCore.Init(Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);
     gRequestorUser.Init(&gRequestorCore, &gImageProcessor);
 
-    gImageProcessor.SetOTAImageFile(CharSpan("test.txt"));
+    gImageProcessor.SetOTAImageFile("test.txt");
     gImageProcessor.SetOTADownloader(&gDownloader);
 
     // Connect the gDownloader and Image Processor objects

examples/ota-requestor-app/efr32/src/AppTask.cpp

@@ -542,7 +542,7 @@ void AppTask::InitOTARequestor()
 
     gRequestorUser.Init(&gRequestorCore, &gImageProcessor);
 
-    gImageProcessor.SetOTAImageFile(CharSpan("test.txt"));
+    gImageProcessor.SetOTAImageFile("test.txt");
     gImageProcessor.SetOTADownloader(&gDownloader);
 
     // Connect the Downloader and Image Processor objects

examples/ota-requestor-app/linux/main.cpp

@@ -118,7 +118,7 @@ static void InitOTARequestor(void)
     gRequestorCore.Init(chip::Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);
     gRequestorUser.Init(&gRequestorCore, &gImageProcessor);
 
-    gImageProcessor.SetOTAImageFile(CharSpan::fromCharString(gOtaDownloadPath));
+    gImageProcessor.SetOTAImageFile(gOtaDownloadPath);
     gImageProcessor.SetOTADownloader(&gDownloader);
 
     // Set the image processor instance used for handling image being downloaded
@@ -199,9 +199,7 @@ int main(int argc, char * argv[])
             return -1;
         }
 
-        char execFilePathBuf[kMaxFilePathSize];
-        strncpy(execFilePathBuf, kImageExecPath, strlen(kImageExecPath));
-        argv[0] = execFilePathBuf;
+        argv[0] = kImageExecPath;
         execv(argv[0], argv);
 
         // If successfully executing the new iamge, execv should not return

examples/platform/efr32/OTAConfig.cpp

@@ -79,7 +79,7 @@ void OTAConfig::Init()
     gRequestorUser.SetPeriodicQueryTimeout(OTA_PERIODIC_TIMEOUT);
     gRequestorUser.Init(&gRequestorCore, &gImageProcessor);
 
-    gImageProcessor.SetOTAImageFile(chip::CharSpan("test.txt"));
+    gImageProcessor.SetOTAImageFile("test.txt");
     gImageProcessor.SetOTADownloader(&gDownloader);
 
     // Connect the Downloader and Image Processor objects

src/app/clusters/ota-requestor/OTARequestor.cpp

@@ -114,7 +114,8 @@ void OTARequestor::InitState(intptr_t context)
     }
     else
     {
-        // This may have been a reboot from applying an image
+        // This may have been a reboot from applying an image. Once the image has been confirmed, the provider will be notified of
+        // the new version and all relevant states will reset for a new OTA update.
         OtaRequestorServerSetUpdateState(requestorCore->mCurrentUpdateState);
     }
 

src/app/tests/TestDefaultOTARequestorStorage.cpp

@@ -147,10 +147,49 @@ void TestUpdateToken(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR != otaStorage.LoadUpdateToken(readUpdateToken));
 }
 
+void TestCurrentUpdateState(nlTestSuite * inSuite, void * inContext)
+{
+    TestPersistentStorageDelegate persistentStorage;
+    DefaultOTARequestorStorage otaStorage;
+    otaStorage.Init(persistentStorage);
+
+    OTARequestorStorage::OTAUpdateStateEnum updateState = OTARequestorStorage::OTAUpdateStateEnum::kApplying;
+
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.StoreCurrentUpdateState(updateState));
+
+    updateState = OTARequestorStorage::OTAUpdateStateEnum::kUnknown;
+
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.LoadCurrentUpdateState(updateState));
+    NL_TEST_ASSERT(inSuite, updateState == OTARequestorStorage::OTAUpdateStateEnum::kApplying);
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.ClearCurrentUpdateState());
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR != otaStorage.LoadCurrentUpdateState(updateState));
+}
+
+void TestTargetVersion(nlTestSuite * inSuite, void * inContext)
+{
+    TestPersistentStorageDelegate persistentStorage;
+    DefaultOTARequestorStorage otaStorage;
+    otaStorage.Init(persistentStorage);
+
+    uint32_t targetVersion = 2;
+
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.StoreTargetVersion(targetVersion));
+
+    targetVersion = 0;
+
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.LoadTargetVersion(targetVersion));
+    NL_TEST_ASSERT(inSuite, targetVersion == 2);
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == otaStorage.ClearTargetVersion());
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR != otaStorage.LoadTargetVersion(targetVersion));
+}
+
 const nlTest sTests[] = { NL_TEST_DEF("Test default providers", TestDefaultProviders),
                           NL_TEST_DEF("Test default providers (empty list)", TestDefaultProvidersEmpty),
                           NL_TEST_DEF("Test current provider location", TestCurrentProviderLocation),
-                          NL_TEST_DEF("Test update token", TestUpdateToken), NL_TEST_SENTINEL() };
+                          NL_TEST_DEF("Test update token", TestUpdateToken),
+                          NL_TEST_DEF("Test current update state", TestCurrentUpdateState),
+                          NL_TEST_DEF("Test target version", TestTargetVersion),
+                          NL_TEST_SENTINEL() };
 
 int TestSetup(void * inContext)
 {

src/controller/python/test/test_scripts/base.py

@@ -576,8 +576,8 @@ def TestReadBasicAttributes(self, nodeid: int, endpoint: int, group: int):
             "Location": "XX",
             "HardwareVersion": 0,
             "HardwareVersionString": "TEST_VERSION",
-            "SoftwareVersion": 0,
-            "SoftwareVersionString": "prerelease",
+            "SoftwareVersion": 1,
+            "SoftwareVersionString": "1.0",
         }
         failed_zcl = {}
         for basic_attr, expected_value in basic_cluster_attrs.items():

src/platform/CYW30739/PlatformManagerImpl.cpp

@@ -127,6 +127,11 @@ CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask(void)
     return err;
 }
 
+CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask()
+{
+    return CHIP_NO_ERROR;
+}
+
 void PlatformManagerImpl::_LockChipStack(void)
 {
     const wiced_result_t result = wiced_rtos_lock_mutex(mMutex);

src/platform/EFR32/OTAImageProcessorImpl.h

@@ -40,7 +40,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     CHIP_ERROR ConfirmCurrentImage() override { return CHIP_NO_ERROR; }
 
     void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
-    void SetOTAImageFile(CharSpan name) { mImageFile = name; }
+    void SetOTAImageFile(const char * imageFile) { mImageFile = imageFile; }
 
 private:
     //////////// Actual handlers for the OTAImageProcessorInterface ///////////////
@@ -66,7 +66,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     MutableByteSpan mBlock;
     OTADownloader * mDownloader;
     OTAImageHeaderParser mHeaderParser;
-    CharSpan mImageFile;
+    const char * mImageFile = nullptr;
 };
 
 } // namespace chip

src/platform/Linux/OTAImageProcessorImpl.cpp

@@ -27,7 +27,7 @@ namespace chip {
 
 CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
 {
-    if (mImageFile.empty())
+    if (mImageFile == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
         return CHIP_ERROR_INTERNAL;
@@ -51,7 +51,7 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()
 
 CHIP_ERROR OTAImageProcessorImpl::Abort()
 {
-    if (mImageFile.empty())
+    if (mImageFile == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
         return CHIP_ERROR_INTERNAL;
@@ -127,10 +127,10 @@ void OTAImageProcessorImpl::HandlePrepareDownload(intptr_t context)
         return;
     }
 
-    unlink(imageProcessor->mImageFile.data());
+    unlink(imageProcessor->mImageFile);
 
     imageProcessor->mHeaderParser.Init();
-    imageProcessor->mOfs.open(imageProcessor->mImageFile.data(), std::ofstream::out | std::ofstream::ate | std::ofstream::app);
+    imageProcessor->mOfs.open(imageProcessor->mImageFile, std::ofstream::out | std::ofstream::ate | std::ofstream::app);
     if (!imageProcessor->mOfs.good())
     {
         imageProcessor->mDownloader->OnPreparedForDownload(CHIP_ERROR_OPEN_FAILED);
@@ -151,7 +151,7 @@ void OTAImageProcessorImpl::HandleFinalize(intptr_t context)
     imageProcessor->mOfs.close();
     imageProcessor->ReleaseBlock();
 
-    ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mImageFile.data());
+    ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mImageFile);
 }
 
 void OTAImageProcessorImpl::HandleApply(intptr_t context)
@@ -164,7 +164,7 @@ void OTAImageProcessorImpl::HandleApply(intptr_t context)
 
     // Move the downloaded image to th location where the new image is to be executed from
     unlink(kImageExecPath);
-    rename(imageProcessor->mImageFile.data(), kImageExecPath);
+    rename(imageProcessor->mImageFile, kImageExecPath);
     chmod(kImageExecPath, S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
 
     // Shutdown the stack and expect to boot into the new image once shutdown is complete
@@ -180,7 +180,7 @@ void OTAImageProcessorImpl::HandleAbort(intptr_t context)
     }
 
     imageProcessor->mOfs.close();
-    unlink(imageProcessor->mImageFile.data());
+    unlink(imageProcessor->mImageFile);
     imageProcessor->ReleaseBlock();
 }
 

src/platform/Linux/OTAImageProcessorImpl.h

@@ -28,7 +28,7 @@
 namespace chip {
 
 // Full file path to where the new image will be executed from post-download
-static constexpr char kImageExecPath[] = "/tmp/ota.update";
+static char kImageExecPath[] = "/tmp/ota.update";
 
 class OTAImageProcessorImpl : public OTAImageProcessorInterface
 {
@@ -43,7 +43,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     CHIP_ERROR ConfirmCurrentImage() override;
 
     void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
-    void SetOTAImageFile(CharSpan name) { mImageFile = name; }
+    void SetOTAImageFile(const char * imageFile) { mImageFile = imageFile; }
 
 private:
     //////////// Actual handlers for the OTAImageProcessorInterface ///////////////
@@ -69,7 +69,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     MutableByteSpan mBlock;
     OTADownloader * mDownloader;
     OTAImageHeaderParser mHeaderParser;
-    CharSpan mImageFile;
+    const char * mImageFile = nullptr;
 };
 
 } // namespace chip

src/platform/nxp/k32w/k32w0/OTAImageProcessorImpl.cpp

@@ -28,7 +28,7 @@ namespace chip {
 
 CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
 {
-    if (mImageFile.empty())
+    if (mImageFile == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
         return CHIP_ERROR_INTERNAL;
@@ -51,7 +51,7 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()
 
 CHIP_ERROR OTAImageProcessorImpl::Abort()
 {
-    if (mImageFile.empty())
+    if (mImageFile == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
         return CHIP_ERROR_INTERNAL;
@@ -95,7 +95,7 @@ void OTAImageProcessorImpl::HandlePrepareDownload(intptr_t context)
 
     if (gOtaSuccess_c == OTA_ClientInit())
     {
-        if (gOtaSuccess_c == OTA_StartImage(imageProcessor->mImageFile.size()))
+        if (gOtaSuccess_c == OTA_StartImage(strlen(imageProcessor->mImageFile)))
         {
             imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR);
         }
@@ -110,7 +110,7 @@ void OTAImageProcessorImpl::HandleAbort(intptr_t context)
         return;
     }
 
-    remove(imageProcessor->mImageFile.data());
+    remove(imageProcessor->mImageFile);
     imageProcessor->ReleaseBlock();
 }
 

src/platform/nxp/k32w/k32w0/OTAImageProcessorImpl.h

@@ -37,7 +37,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
     CHIP_ERROR ConfirmCurrentImage() override { return CHIP_NO_ERROR; }
 
     void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
-    void SetOTAImageFile(CharSpan name) { mImageFile = name; }
+    void SetOTAImageFile(const char * imageFile) { mImageFile = imageFile; }
 
 private:
     //////////// Actual handlers for the OTAImageProcessorInterface ///////////////
@@ -58,7 +58,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
 
     OTADownloader * mDownloader;
     MutableByteSpan mBlock;
-    CharSpan mImageFile;
+    const char * mImageFile = nullptr;
 };
 
 } // namespace chip