Skip to content

Commit

Permalink
Assume external attributes that must be non-volatile actually are. (#…
Browse files Browse the repository at this point in the history
…20144)

We have clusters doing non-volatility sanity checks, but for external
attributes we can't conclusively say they are volatile, and the
clusters don't work right if we assume they are.  So instead assume
that if the spec says N the external attribute is in fact
non-volatile.

Fixes #20093
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 9, 2023
1 parent 0b7b739 commit b2f0227
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 26 deletions.
8 changes: 4 additions & 4 deletions src/app/clusters/ias-zone-server/ias-zone-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,10 @@ uint8_t emberAfPluginIasZoneServerGetZoneId(EndpointId endpoint)
//------------------------------------------------------------------------------
static bool areZoneServerAttributesNonVolatile(EndpointId endpoint)
{
if (!emberAfIsNonVolatileAttribute(endpoint, IasZone::Id, Attributes::IasCieAddress::Id) ||
!emberAfIsNonVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneState::Id) ||
!emberAfIsNonVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneType::Id) ||
!emberAfIsNonVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneId::Id))
if (emberAfIsKnownVolatileAttribute(endpoint, IasZone::Id, Attributes::IasCieAddress::Id) ||
emberAfIsKnownVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneState::Id) ||
emberAfIsKnownVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneType::Id) ||
emberAfIsKnownVolatileAttribute(endpoint, IasZone::Id, Attributes::ZoneId::Id))
{
return false;
}
Expand Down
8 changes: 2 additions & 6 deletions src/app/clusters/level-control/level-control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1122,12 +1122,8 @@ void emberAfLevelControlClusterServerInitCallback(EndpointId endpoint)
#ifndef IGNORE_LEVEL_CONTROL_CLUSTER_START_UP_CURRENT_LEVEL
static bool areStartUpLevelControlServerAttributesNonVolatile(EndpointId endpoint)
{
if (emberAfIsNonVolatileAttribute(endpoint, LevelControl::Id, Attributes::CurrentLevel::Id))
{
return emberAfIsNonVolatileAttribute(endpoint, LevelControl::Id, Attributes::StartUpCurrentLevel::Id);
}

return false;
return !emberAfIsKnownVolatileAttribute(endpoint, LevelControl::Id, Attributes::CurrentLevel::Id) &&
!emberAfIsKnownVolatileAttribute(endpoint, LevelControl::Id, Attributes::StartUpCurrentLevel::Id);
}
#endif // IGNORE_LEVEL_CONTROL_CLUSTER_START_UP_CURRENT_LEVEL

Expand Down
6 changes: 3 additions & 3 deletions src/app/clusters/mode-select-server/mode-select-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ void emberAfModeSelectClusterServerInitCallback(EndpointId endpointId)
Attributes::OnMode::TypeInfo::Type onMode;
bool onOffValueForStartUp = 0;
if (Attributes::OnMode::Get(endpointId, onMode) == EMBER_ZCL_STATUS_SUCCESS &&
emberAfIsNonVolatileAttribute(endpointId, OnOff::Id, OnOff::Attributes::StartUpOnOff::Id) &&
!emberAfIsKnownVolatileAttribute(endpointId, OnOff::Id, OnOff::Attributes::StartUpOnOff::Id) &&
OnOffServer::Instance().getOnOffValueForStartUp(endpointId, onOffValueForStartUp) == EMBER_ZCL_STATUS_SUCCESS)
{
if (onOffValueForStartUp && !onMode.IsNull())
Expand Down Expand Up @@ -189,8 +189,8 @@ namespace {
*/
inline bool areStartUpModeAndCurrentModeNonVolatile(EndpointId endpointId)
{
return emberAfIsNonVolatileAttribute(endpointId, ModeSelect::Id, Attributes::CurrentMode::Id) &&
emberAfIsNonVolatileAttribute(endpointId, ModeSelect::Id, Attributes::StartUpMode::Id);
return !emberAfIsKnownVolatileAttribute(endpointId, ModeSelect::Id, Attributes::CurrentMode::Id) &&
!emberAfIsKnownVolatileAttribute(endpointId, ModeSelect::Id, Attributes::StartUpMode::Id);
}

} // namespace
Expand Down
8 changes: 2 additions & 6 deletions src/app/clusters/on-off-server/on-off-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,8 @@ void OnOffServer::updateOnOffTimeCommand(chip::EndpointId endpoint)
#ifndef IGNORE_ON_OFF_CLUSTER_START_UP_ON_OFF
bool OnOffServer::areStartUpOnOffServerAttributesNonVolatile(EndpointId endpoint)
{
if (emberAfIsNonVolatileAttribute(endpoint, OnOff::Id, Attributes::OnOff::Id))
{
return emberAfIsNonVolatileAttribute(endpoint, OnOff::Id, Attributes::StartUpOnOff::Id);
}

return false;
return !emberAfIsKnownVolatileAttribute(endpoint, OnOff::Id, Attributes::OnOff::Id) &&
!emberAfIsKnownVolatileAttribute(endpoint, OnOff::Id, Attributes::StartUpOnOff::Id);
}
#endif // IGNORE_ON_OFF_CLUSTER_START_UP_ON_OFF

Expand Down
2 changes: 1 addition & 1 deletion src/app/util/attribute-metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ struct EmberAfAttributeMetadata
* Check whether this attribute is automatically stored in non-volatile
* memory.
*/
bool IsNonVolatile() const { return (mask & ATTRIBUTE_MASK_NONVOLATILE) && !IsExternal(); }
bool IsAutomaticallyPersisted() const { return (mask & ATTRIBUTE_MASK_NONVOLATILE) && !IsExternal(); }
};

/** @brief Returns true if the given attribute type is a string. */
Expand Down
4 changes: 2 additions & 2 deletions src/app/util/attribute-storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1227,7 +1227,7 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
ptr = nullptr; // Will get set to the value to write, as needed.

// First check for a persisted value.
if (!ignoreStorage && am->IsNonVolatile())
if (!ignoreStorage && am->IsAutomaticallyPersisted())
{
VerifyOrDie(attrStorage && "Attribute persistence needs a persistence provider");
MutableByteSpan bytes(attrData);
Expand Down Expand Up @@ -1331,7 +1331,7 @@ void emAfSaveAttributeToStorageIfNeeded(uint8_t * data, EndpointId endpoint, Clu
const EmberAfAttributeMetadata * metadata)
{
// Get out of here if this attribute isn't marked non-volatile.
if (!metadata->IsNonVolatile())
if (!metadata->IsAutomaticallyPersisted())
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/util/attribute-table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void emberAfPrintAttributeTable(void)
emberAfAttributesPrint(
" / %x (%x) / %p / %p / ", metaData->attributeType, emberAfAttributeSize(metaData),
(metaData->IsReadOnly() ? "RO" : "RW"),
(metaData->IsNonVolatile() ? " nonvolatile " : (metaData->IsExternal() ? " extern " : " RAM ")));
(metaData->IsAutomaticallyPersisted() ? " nonvolatile " : (metaData->IsExternal() ? " extern " : " RAM ")));
emberAfAttributesFlush();
status =
emAfReadAttribute(ep->endpoint, cluster->clusterId, metaData->attributeId, data, ATTRIBUTE_LARGEST, nullptr);
Expand Down
4 changes: 2 additions & 2 deletions src/app/util/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ bool emberAfContainsAttribute(chip::EndpointId endpoint, chip::ClusterId cluster
return (emberAfGetServerAttributeIndexByAttributeId(endpoint, clusterId, attributeId) != UINT16_MAX);
}

bool emberAfIsNonVolatileAttribute(chip::EndpointId endpoint, chip::ClusterId clusterId, chip::AttributeId attributeId)
bool emberAfIsKnownVolatileAttribute(chip::EndpointId endpoint, chip::ClusterId clusterId, chip::AttributeId attributeId)
{
const EmberAfAttributeMetadata * metadata = emberAfLocateAttributeMetadata(endpoint, clusterId, attributeId);

Expand All @@ -777,7 +777,7 @@ bool emberAfIsNonVolatileAttribute(chip::EndpointId endpoint, chip::ClusterId cl
return false;
}

return metadata->IsNonVolatile();
return !metadata->IsAutomaticallyPersisted() && !metadata->IsExternal();
}

chip::Messaging::ExchangeManager * chip::ExchangeManager()
Expand Down
6 changes: 5 additions & 1 deletion src/app/util/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ uint8_t emberAfGetChannelFrom8bitEncodedChanPg(uint8_t chanPg);
uint8_t emberAfMake8bitEncodedChanPg(uint8_t page, uint8_t channel);

bool emberAfContainsAttribute(chip::EndpointId endpoint, chip::ClusterId clusterId, chip::AttributeId attributeId);
bool emberAfIsNonVolatileAttribute(chip::EndpointId endpoint, chip::ClusterId clusterId, chip::AttributeId attributeId);

/* @brief returns true if the attribute is known to be volatile (i.e. RAM
* storage).
*/
bool emberAfIsKnownVolatileAttribute(chip::EndpointId endpoint, chip::ClusterId clusterId, chip::AttributeId attributeId);

namespace chip {
chip::Messaging::ExchangeManager * ExchangeManager();
Expand Down

0 comments on commit b2f0227

Please sign in to comment.