Skip to content

Commit

Permalink
Ignore StartupMode when rebooting due to OTA. (project-chip#21074)
Browse files Browse the repository at this point in the history
Fixes project-chip#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>
  • Loading branch information
bzbarsky-apple and isiu-apple committed Sep 16, 2022
1 parent 876b0b2 commit da8d6b5
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 7 deletions.
2 changes: 1 addition & 1 deletion examples/ota-requestor-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 24 additions & 4 deletions src/app/clusters/mode-select-server/mode-select-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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, &currentMode);
#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
Expand All @@ -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, &currentMode);

if ((status == EMBER_ZCL_STATUS_SUCCESS) && (startUpMode.Value() != currentMode))
{
status = Attributes::CurrentMode::Set(endpointId, startUpMode.Value());
if (status != EMBER_ZCL_STATUS_SUCCESS)
Expand Down
9 changes: 7 additions & 2 deletions src/lib/support/SafeInt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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
Expand Down Expand Up @@ -105,6 +104,12 @@ bool CanCastTo(U arg)
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.
*
Expand Down
12 changes: 12 additions & 0 deletions src/platform/Darwin/DiagnosticDataProviderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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<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
Expand Down
1 change: 1 addition & 0 deletions src/platform/Darwin/DiagnosticDataProviderImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down

0 comments on commit da8d6b5

Please sign in to comment.