From b647dfb48c88fbbdedc2dc6df6af44b5bf2da616 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 22 Jul 2022 23:04:04 -0400 Subject: [PATCH] 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> --- examples/ota-requestor-app/linux/main.cpp | 2 +- .../mode-select-server/mode-select-server.cpp | 28 ++++++++++++++++--- src/lib/support/SafeInt.h | 9 ++++-- .../Darwin/DiagnosticDataProviderImpl.cpp | 12 ++++++++ .../Darwin/DiagnosticDataProviderImpl.h | 1 + 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 7507e201646da9..174b3d7cd9809b 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -347,7 +347,7 @@ int main(int argc, char * argv[]) 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 0266c4dc6e0992..a25f66915a7525 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 #include #include +#include using namespace std; using namespace chip; @@ -41,6 +42,8 @@ using namespace chip::app::Clusters; 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 @@ void emberAfModeSelectClusterServerInitCallback(EndpointId endpointId) 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 @@ void emberAfModeSelectClusterServerInitCallback(EndpointId endpointId) } } #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 efbaa23b376bea..ecf6d8721dcb1d 100644 --- a/src/lib/support/SafeInt.h +++ b/src/lib/support/SafeInt.h @@ -34,14 +34,13 @@ namespace chip { * 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 +template ::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::type V; // V for "value" - static_assert(is_integral::value, "Must be assigning to an integral type"); static_assert(is_integral::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 @@ bool CanCastTo(U arg) return 0 <= arg && static_cast(arg) <= static_cast(numeric_limits::max()); } +template ::value, int> = 0> +bool CanCastTo(U arg) +{ + return CanCastTo>(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 4ad47ac1dba41b..640a8eca27cd58 100644 --- a/src/platform/Darwin/DiagnosticDataProviderImpl.cpp +++ b/src/platform/Darwin/DiagnosticDataProviderImpl.cpp @@ -23,6 +23,7 @@ #include +#include #include #include #include @@ -68,6 +69,17 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetTotalOperationalHours(uint32_t & total return CHIP_ERROR_INVALID_TIME; } +CHIP_ERROR DiagnosticDataProviderImpl::GetBootReason(BootReasonType & bootReason) +{ + uint32_t reason = 0; + ReturnErrorOnFailure(ConfigurationMgr().GetBootReason(reason)); + + VerifyOrReturnError(CanCastTo(reason), CHIP_ERROR_INVALID_INTEGER_VALUE); + bootReason = static_cast(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 6554c03041346b..3cfc31bc1f9678 100644 --- a/src/platform/Darwin/DiagnosticDataProviderImpl.h +++ b/src/platform/Darwin/DiagnosticDataProviderImpl.h @@ -38,6 +38,7 @@ class DiagnosticDataProviderImpl : public DiagnosticDataProvider // ===== 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; }