Ignore StartupMode when rebooting due to OTA. (#21074)
Fixes https://github.com/project-chip/connectedhomeip/issues/19272
This will only work on platforms that actually set boot reasons
properly (i.e. probably not Mac/Linux).
Co-authored-by: Irene Siu (Apple) <98558756+isiu-apple@users.noreply.github.com>
diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp
index 7507e20..174b3d7 100644
--- a/examples/ota-requestor-app/linux/main.cpp
+++ b/examples/ota-requestor-app/linux/main.cpp
@@ -347,7 +347,7 @@
argv[0] = kImageExecPath;
execv(argv[0], argv);
- // If successfully executing the new iamge, execv should not return
+ // If successfully executing the new image, execv should not return
ChipLogError(SoftwareUpdate, "The OTA image is invalid");
}
return 0;
diff --git a/src/app/clusters/mode-select-server/mode-select-server.cpp b/src/app/clusters/mode-select-server/mode-select-server.cpp
index 0266c4d..a25f669 100644
--- a/src/app/clusters/mode-select-server/mode-select-server.cpp
+++ b/src/app/clusters/mode-select-server/mode-select-server.cpp
@@ -33,6 +33,7 @@
#include <app/util/odd-sized-integers.h>
#include <app/util/util.h>
#include <lib/support/CodeUtils.h>
+#include <platform/DiagnosticDataProvider.h>
using namespace std;
using namespace chip;
@@ -41,6 +42,8 @@
using namespace chip::app::Clusters::ModeSelect;
using namespace chip::Protocols;
+using BootReasonType = GeneralDiagnostics::BootReasonType;
+
static InteractionModel::Status verifyModeValue(const EndpointId endpointId, const uint8_t newMode);
namespace {
@@ -132,9 +135,6 @@
EmberAfStatus status = Attributes::StartUpMode::Get(endpointId, startUpMode);
if (status == EMBER_ZCL_STATUS_SUCCESS && !startUpMode.IsNull())
{
- // Initialise currentMode to 0
- uint8_t currentMode = 0;
- status = Attributes::CurrentMode::Get(endpointId, ¤tMode);
#ifdef EMBER_AF_PLUGIN_ON_OFF
// OnMode with Power Up
// If the On/Off feature is supported and the On/Off cluster attribute StartUpOnOff is present, with a
@@ -158,7 +158,27 @@
}
}
#endif // EMBER_AF_PLUGIN_ON_OFF
- if (status == EMBER_ZCL_STATUS_SUCCESS && startUpMode.Value() != currentMode)
+
+ BootReasonType bootReason = BootReasonType::kUnspecified;
+ CHIP_ERROR error = DeviceLayer::GetDiagnosticDataProvider().GetBootReason(bootReason);
+
+ if (error != CHIP_NO_ERROR)
+ {
+ ChipLogError(Zcl, "Unable to retrieve boot reason: %" CHIP_ERROR_FORMAT, error.Format());
+ // We really only care whether the boot reason is OTA. Assume it's not.
+ bootReason = BootReasonType::kUnspecified;
+ }
+ if (bootReason == BootReasonType::kSoftwareUpdateCompleted)
+ {
+ ChipLogDetail(Zcl, "ModeSelect: CurrentMode is ignored for OTA reboot");
+ return;
+ }
+
+ // Initialise currentMode to 0
+ uint8_t currentMode = 0;
+ status = Attributes::CurrentMode::Get(endpointId, ¤tMode);
+
+ if ((status == EMBER_ZCL_STATUS_SUCCESS) && (startUpMode.Value() != currentMode))
{
status = Attributes::CurrentMode::Set(endpointId, startUpMode.Value());
if (status != EMBER_ZCL_STATUS_SUCCESS)
diff --git a/src/lib/support/SafeInt.h b/src/lib/support/SafeInt.h
index efbaa23..ecf6d87 100644
--- a/src/lib/support/SafeInt.h
+++ b/src/lib/support/SafeInt.h
@@ -34,14 +34,13 @@
* of type U to the given type T. It does this by verifying that the value is
* in the range of valid values for T.
*/
-template <typename T, typename U>
+template <typename T, typename U, std::enable_if_t<std::is_integral<T>::value, int> = 0>
bool CanCastTo(U arg)
{
using namespace std;
// U might be a reference to an integer type, if we're assigning from
// something passed by reference.
typedef typename remove_reference<U>::type V; // V for "value"
- static_assert(is_integral<T>::value, "Must be assigning to an integral type");
static_assert(is_integral<V>::value, "Must be assigning from an integral type");
// We want to check that "arg" can fit inside T but without doing any tests
@@ -105,6 +104,12 @@
return 0 <= arg && static_cast<uintmax_t>(arg) <= static_cast<uintmax_t>(numeric_limits<T>::max());
}
+template <typename T, typename U, std::enable_if_t<std::is_enum<T>::value, int> = 0>
+bool CanCastTo(U arg)
+{
+ return CanCastTo<std::underlying_type_t<T>>(arg);
+}
+
/**
* A function to reverse the effects of a signed-to-unsigned integer cast.
*
diff --git a/src/platform/Darwin/DiagnosticDataProviderImpl.cpp b/src/platform/Darwin/DiagnosticDataProviderImpl.cpp
index 4ad47ac..640a8ec 100644
--- a/src/platform/Darwin/DiagnosticDataProviderImpl.cpp
+++ b/src/platform/Darwin/DiagnosticDataProviderImpl.cpp
@@ -23,6 +23,7 @@
#include <platform/internal/CHIPDeviceLayerInternal.h>
+#include <lib/support/SafeInt.h>
#include <lib/support/logging/CHIPLogging.h>
#include <platform/Darwin/DiagnosticDataProviderImpl.h>
#include <platform/DiagnosticDataProvider.h>
@@ -68,6 +69,17 @@
return CHIP_ERROR_INVALID_TIME;
}
+CHIP_ERROR DiagnosticDataProviderImpl::GetBootReason(BootReasonType & bootReason)
+{
+ uint32_t reason = 0;
+ ReturnErrorOnFailure(ConfigurationMgr().GetBootReason(reason));
+
+ VerifyOrReturnError(CanCastTo<BootReasonType>(reason), CHIP_ERROR_INVALID_INTEGER_VALUE);
+ bootReason = static_cast<BootReasonType>(reason);
+
+ return CHIP_NO_ERROR;
+}
+
CHIP_ERROR DiagnosticDataProviderImpl::ResetWatermarks()
{
// If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
diff --git a/src/platform/Darwin/DiagnosticDataProviderImpl.h b/src/platform/Darwin/DiagnosticDataProviderImpl.h
index 6554c03..3cfc31b 100644
--- a/src/platform/Darwin/DiagnosticDataProviderImpl.h
+++ b/src/platform/Darwin/DiagnosticDataProviderImpl.h
@@ -38,6 +38,7 @@
// ===== Methods that implement the PlatformManager abstract interface.
CHIP_ERROR GetUpTime(uint64_t & upTime) override;
CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;
+ CHIP_ERROR GetBootReason(BootReasonType & bootReason) override;
// ===== Methods that implement the DiagnosticDataProvider abstract interface.
bool SupportsWatermarks() override { return true; }
