Address followup issues for preset/atomic-write implementation. (#35175)

* Puts some file-local functions in an anonymous namespace.
* Fixes the "is this attribute supported?" check to correctly check for global
  attributes that are not in Ember metadata.
* Moves the comment explaining why it's OK to skip the spec-required ACL check
  to the place where that check is being skipped.
* Removes a non-spec-compliant "error if the timeout is 0" bit.

Fixes #35168

src/app/GlobalAttributes.h

@@ -40,5 +40,18 @@ constexpr AttributeId GlobalAttributesNotInMetadata[] = {
 
 static_assert(ArrayIsSorted(GlobalAttributesNotInMetadata), "Array of global attribute ids must be sorted");
 
+inline bool IsSupportedGlobalAttributeNotInMetadata(AttributeId attributeId)
+{
+ for (auto & attr : GlobalAttributesNotInMetadata)
+ {
+ if (attr == attributeId)
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
 } // namespace app
 } // namespace chip

src/app/clusters/thermostat-server/thermostat-server-atomic.cpp

@@ -17,6 +17,7 @@
 
 #include "thermostat-server.h"
 
+#include <app/GlobalAttributes.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
 
 using namespace chip;
@@ -47,6 +48,8 @@ void TimerExpiredCallback(System::Layer * systemLayer, void * callbackContext)
  gThermostatAttrAccess.ResetAtomicWrite(endpoint);
 }
 
+namespace {
+
 /**
  * @brief Schedules a timer for the given timeout in milliseconds.
  *
@@ -185,15 +188,25 @@ Status BuildAttributeStatuses(const EndpointId endpoint, const DataModel::Decoda
  const EmberAfAttributeMetadata * metadata =
  emberAfLocateAttributeMetadata(endpoint, Thermostat::Id, attributeStatus.attributeID);
 
- if (metadata == nullptr)
+ if (metadata != nullptr)
+ {
+ // This is definitely an attribute we know about.
+ continue;
+ }
+
+ if (IsSupportedGlobalAttributeNotInMetadata(attributeStatus.attributeID))
  {
- // This is not a valid attribute on the Thermostat cluster on the supplied endpoint
- return Status::InvalidCommand;
+ continue;
  }
+
+ // This is not a valid attribute on the Thermostat cluster on the supplied endpoint
+ return Status::InvalidCommand;
  }
  return Status::Success;
 }
 
+} // anonymous namespace
+
 bool ThermostatAttrAccess::InAtomicWrite(EndpointId endpoint, Optional<AttributeId> attributeId)
 {
 
@@ -422,6 +435,10 @@ void ThermostatAttrAccess::BeginAtomicWrite(CommandHandler * commandObj, const C
  status = Status::Success;
  for (size_t i = 0; i < attributeStatuses.AllocatedSize(); ++i)
  {
+ // If we've gotten this far, then the client has manage permission to call AtomicRequest,
+ // which is also the privilege necessary to write to the atomic attributes, so no need to do
+ // the "If the client does not have sufficient privilege to write to the attribute" check
+ // from the spec.
  auto & attributeStatus = attributeStatuses[i];
  auto statusCode = Status::Success;
  switch (attributeStatus.attributeID)
@@ -442,11 +459,6 @@ void ThermostatAttrAccess::BeginAtomicWrite(CommandHandler * commandObj, const C
  }
 
  auto timeout = std::min(System::Clock::Milliseconds16(commandData.timeout.Value()), maximumTimeout);
- if (timeout.count() == 0)
- {
- commandObj->AddStatus(commandPath, Status::InvalidInState);
- return;
- }
 
  if (status == Status::Success)
  {
@@ -605,9 +617,6 @@ bool emberAfThermostatClusterAtomicRequestCallback(CommandHandler * commandObj,
 {
  auto & requestType = commandData.requestType;
 
- // If we've gotten this far, then the client has manage permission to call AtomicRequest, which is also the
- // privilege necessary to write to the atomic attributes, so no need to check
-
  switch (requestType)
  {
  case Globals::AtomicRequestTypeEnum::kBeginWrite: