Use GlobalAttributesNotInMetadata instead of hardcoding the names. (#22453)
* Use GlobalAttributesNotInMetadata instead of hardcoding the names.
This way when we add EventList things should work right most places
without needing to hunt them down and fix them.
Fixes https://github.com/project-chip/connectedhomeip/issues/22443
* Address review comment.
diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
index 31390a9..6905527 100644
--- a/src/app/util/ember-compatibility-functions.cpp
+++ b/src/app/util/ember-compatibility-functions.cpp
@@ -415,6 +415,12 @@
return EncodeCommandList(aPath, aEncoder, &CommandHandlerInterface::EnumerateGeneratedCommands,
mCluster->generatedCommandList);
default:
+ // This function is only called if attributeCluster is non-null in
+ // ReadSingleClusterData, which only happens for attributes listed in
+ // GlobalAttributesNotInMetadata. If we reach this code, someone added
+ // a global attribute to that list but not the above switch.
+ VerifyOrDieWithMsg(false, DataManagement, "Unexpected global attribute: " ChipLogFormatMEI,
+ ChipLogValueMEI(aPath.mAttributeId));
return CHIP_NO_ERROR;
}
}
@@ -551,16 +557,19 @@
const EmberAfCluster * attributeCluster = nullptr;
const EmberAfAttributeMetadata * attributeMetadata = nullptr;
- switch (aPath.mAttributeId)
+ bool isGlobalAttributeNotInMetadata = false;
+ for (auto & attr : GlobalAttributesNotInMetadata)
{
- case Clusters::Globals::Attributes::AttributeList::Id:
- FALLTHROUGH;
- case Clusters::Globals::Attributes::AcceptedCommandList::Id:
- FALLTHROUGH;
- case Clusters::Globals::Attributes::GeneratedCommandList::Id:
- attributeCluster = emberAfFindCluster(aPath.mEndpointId, aPath.mClusterId, CLUSTER_MASK_SERVER);
- break;
- default:
+ if (attr == aPath.mAttributeId)
+ {
+ isGlobalAttributeNotInMetadata = true;
+ attributeCluster = emberAfFindCluster(aPath.mEndpointId, aPath.mClusterId, CLUSTER_MASK_SERVER);
+ break;
+ }
+ }
+
+ if (!isGlobalAttributeNotInMetadata)
+ {
attributeMetadata = emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId);
}
diff --git a/src/darwin/Framework/CHIP/MTRIMDispatch.mm b/src/darwin/Framework/CHIP/MTRIMDispatch.mm
index 7979c34..7d0594b 100644
--- a/src/darwin/Framework/CHIP/MTRIMDispatch.mm
+++ b/src/darwin/Framework/CHIP/MTRIMDispatch.mm
@@ -28,6 +28,7 @@
#include <app/CommandHandler.h>
#include <app/ConcreteAttributePath.h>
#include <app/ConcreteCommandPath.h>
+#include <app/GlobalAttributes.h>
#include <app/MessageDef/AttributeReportIBs.h>
#include <app/MessageDef/StatusIB.h>
#include <app/WriteHandler.h>
@@ -66,11 +67,29 @@
namespace {
- Status DetermineAttributeStatus(const ConcreteAttributePath & aPath, bool aIsWrite)
+ bool IsSupportedGlobalAttribute(AttributeId aAttribute)
{
// We don't have any non-global attributes.
using namespace Globals::Attributes;
+ for (auto & attr : GlobalAttributesNotInMetadata) {
+ if (attr == aAttribute) {
+ return true;
+ }
+ }
+
+ switch (aAttribute) {
+ case FeatureMap::Id:
+ FALLTHROUGH;
+ case ClusterRevision::Id:
+ return true;
+ }
+
+ return false;
+ }
+
+ Status DetermineAttributeStatus(const ConcreteAttributePath & aPath, bool aIsWrite)
+ {
// TODO: Consider making this configurable for applications that are not
// trying to be an OTA provider, though in practice it just affects which
// error is returned.
@@ -85,30 +104,13 @@
return Status::UnsupportedCluster;
}
- switch (aPath.mAttributeId) {
- case AttributeList::Id:
- FALLTHROUGH;
- case AcceptedCommandList::Id:
- FALLTHROUGH;
- case GeneratedCommandList::Id:
- FALLTHROUGH;
- // When EventList is supported, include it here.
-#if 0
- case EventList::Id:
- FALLTHROUGH;
-#endif
- case FeatureMap::Id:
- FALLTHROUGH;
- case ClusterRevision::Id:
- // No permissions for this for read, and none of these are writable for
- // write. The writable-or-not check happens before the ACL check.
- return aIsWrite ? Status::UnsupportedWrite : Status::UnsupportedAccess;
- default:
- // No other attributes.
- break;
+ if (!IsSupportedGlobalAttribute(aPath.mAttributeId)) {
+ return Status::UnsupportedAttribute;
}
- return Status::UnsupportedAttribute;
+ // No permissions for this for read, and none of these are writable for
+ // write. The writable-or-not check happens before the ACL check.
+ return aIsWrite ? Status::UnsupportedWrite : Status::UnsupportedAccess;
}
} // anonymous namespace
