Add the ability to read extra attributes during commissioning

src/app/AttributePathParams.h

@@ -30,27 +30,27 @@ struct AttributePathParams
 {
     AttributePathParams() = default;
 
-    explicit AttributePathParams(EndpointId aEndpointId) :
+    constexpr explicit AttributePathParams(EndpointId aEndpointId) :
         AttributePathParams(aEndpointId, kInvalidClusterId, kInvalidAttributeId, kInvalidListIndex)
     {}
 
     //
     // TODO: (Issue #10596) Need to ensure that we do not encode the NodeId over the wire
     // if it is either not 'set', or is set to a value that matches accessing fabric
     // on which the interaction is undertaken.
-    AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId) :
+    constexpr AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId) :
         AttributePathParams(aEndpointId, aClusterId, kInvalidAttributeId, kInvalidListIndex)
     {}
 
-    AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) :
+    constexpr AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) :
         AttributePathParams(aEndpointId, aClusterId, aAttributeId, kInvalidListIndex)
     {}
 
-    AttributePathParams(ClusterId aClusterId, AttributeId aAttributeId) :
+    constexpr AttributePathParams(ClusterId aClusterId, AttributeId aAttributeId) :
         AttributePathParams(kInvalidEndpointId, aClusterId, aAttributeId, kInvalidListIndex)
     {}
 
-    AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListIndex aListIndex) :
+    constexpr AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListIndex aListIndex) :
         mClusterId(aClusterId), mAttributeId(aAttributeId), mEndpointId(aEndpointId), mListIndex(aListIndex)
     {}
 

src/controller/AutoCommissioner.cpp

@@ -18,13 +18,14 @@
 
 #include <controller/AutoCommissioner.h>
 
-#include <cstring>
-
 #include <app/InteractionModelTimeout.h>
 #include <controller/CHIPDeviceController.h>
 #include <credentials/CHIPCert.h>
 #include <lib/support/SafeInt.h>
 
+#include <cstring>
+#include <type_traits>
+
 namespace chip {
 namespace Controller {
 
@@ -225,6 +226,32 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
         mParams.SetICDClientType(params.GetICDClientType().Value());
     }
 
+    auto extraReadPaths = params.GetExtraReadPaths();
+    if (extraReadPaths.size() > 0)
+    {
+        using ReadPath = std::remove_pointer_t<decltype(extraReadPaths.data())>;
+        static_assert(std::is_trivially_copyable_v<ReadPath>, "can't use memmove / memcpy, not trivially copyable");
+
+        if (mExtraReadPaths.AllocatedSize() == extraReadPaths.size())
+        {
+            memmove(mExtraReadPaths.Get(), extraReadPaths.data(), extraReadPaths.size() * sizeof(ReadPath));
+        }
+        else
+        {
+            // We can't reallocate mExtraReadPaths yet as this would free the old buffer,
+            // and the caller might be passing a sub-span of the old paths.
+            decltype(mExtraReadPaths) oldReadPaths(std::move(mExtraReadPaths));
+            VerifyOrReturnError(mExtraReadPaths.Alloc(extraReadPaths.size()).Get() != nullptr, CHIP_ERROR_NO_MEMORY);
+            memcpy(mExtraReadPaths.Get(), extraReadPaths.data(), extraReadPaths.size() * sizeof(ReadPath));
+        }
+
+        mParams.SetExtraReadPaths(mExtraReadPaths.Span());
+    }
+    else
+    {
+        mExtraReadPaths.Free();
+    }
+
     return CHIP_NO_ERROR;
 }
 

src/controller/AutoCommissioner.h

@@ -16,10 +16,13 @@
  *    limitations under the License.
  */
 #pragma once
+
+#include <app/AttributePathParams.h>
 #include <controller/CommissioneeDeviceProxy.h>
 #include <controller/CommissioningDelegate.h>
 #include <credentials/DeviceAttestationConstructor.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <lib/support/ScopedBuffer.h>
 #include <protocols/secure_channel/RendezvousParameters.h>
 
 namespace chip {
@@ -116,7 +119,9 @@ class AutoCommissioner : public CommissioningDelegate
     CommissioneeDeviceProxy * mCommissioneeDeviceProxy               = nullptr;
     OperationalCredentialsDelegate * mOperationalCredentialsDelegate = nullptr;
     OperationalDeviceProxy mOperationalDeviceProxy;
-    // Memory space for the commisisoning parameters that come in as ByteSpans - the caller is not guaranteed to retain this memory
+
+    // BEGIN memory space for commissioning parameters that are Spans or similar pointers:
+    // The caller is not guaranteed to retain the memory these parameters point to.
     uint8_t mSsid[CommissioningParameters::kMaxSsidLen];
     uint8_t mCredentials[CommissioningParameters::kMaxCredentialsLen];
     uint8_t mThreadOperationalDataset[CommissioningParameters::kMaxThreadDatasetLen];
@@ -136,6 +141,11 @@ class AutoCommissioner : public CommissioningDelegate
     static constexpr size_t kMaxDefaultNtpSize = 128;
     char mDefaultNtp[kMaxDefaultNtpSize];
 
+    uint8_t mICDSymmetricKey[Crypto::kAES_CCM128_Key_Length];
+    Platform::ScopedMemoryBufferWithSize<app::AttributePathParams> mExtraReadPaths;
+
+    // END memory space for commisisoning parameters
+
     bool mNeedsNetworkSetup = false;
     ReadCommissioningInfo mDeviceCommissioningInfo;
     bool mNeedsDST = false;
@@ -155,8 +165,6 @@ class AutoCommissioner : public CommissioningDelegate
     uint8_t mAttestationElements[Credentials::kMaxRspLen];
     uint16_t mAttestationSignatureLen = 0;
     uint8_t mAttestationSignature[Crypto::kMax_ECDSA_Signature_Length];
-
-    uint8_t mICDSymmetricKey[Crypto::kAES_CCM128_Key_Length];
 };
 } // namespace Controller
 } // namespace chip

src/controller/CHIPDeviceController.cpp

@@ -2329,6 +2329,12 @@ void DeviceCommissioner::ContinueReadingCommissioningInfo(const CommissioningPar
                                                 Clusters::IcdManagement::Attributes::ActiveModeDuration::Id));
         VerifyOrReturn(builder.AddAttributePath(kRootEndpointId, Clusters::IcdManagement::Id,
                                                 Clusters::IcdManagement::Attributes::ActiveModeThreshold::Id));
+
+        // Extra paths requested via CommissioningParameters
+        for (auto const & path : params.GetExtraReadPaths())
+        {
+            VerifyOrReturn(builder.AddAttributePath(path));
+        }
     }();
 
     VerifyOrDie(builder.size() > 0); // our logic is broken if there is nothing to read
@@ -2363,6 +2369,7 @@ void DeviceCommissioner::FinishReadingCommissioningInfo()
     // up returning an error (e.g. because some mandatory information was missing).
     CHIP_ERROR err = CHIP_NO_ERROR;
     ReadCommissioningInfo info;
+    info.attributes = mAttributeCache.get();
     AccumulateErrors(err, ParseGeneralCommissioningInfo(info));
     AccumulateErrors(err, ParseBasicInformation(info));
     AccumulateErrors(err, ParseNetworkCommissioningInfo(info));

src/controller/CHIPDeviceController.h

@@ -816,6 +816,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
 
     Optional<CommissioningParameters> GetCommissioningParameters()
     {
+        // TODO: Return a non-optional const & to avoid a copy, mDefaultCommissioner is never null
         return mDefaultCommissioner == nullptr ? NullOptional : MakeOptional(mDefaultCommissioner->GetCommissioningParameters());
     }
 

src/controller/CommissioningDelegate.h

@@ -18,11 +18,14 @@
 
 #pragma once
 #include <app-common/zap-generated/cluster-objects.h>
+#include <app/AttributePathParams.h>
+#include <app/ClusterStateCache.h>
 #include <app/OperationalSessionSetup.h>
 #include <controller/CommissioneeDeviceProxy.h>
 #include <credentials/attestation_verifier/DeviceAttestationDelegate.h>
 #include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <lib/support/Span.h>
 #include <lib/support/Variant.h>
 #include <matter/tracing/build_config.h>
 #include <system/SystemClock.h>
@@ -599,6 +602,18 @@ class CommissioningParameters
     }
     void ClearICDStayActiveDurationMsec() { mICDStayActiveDurationMsec.ClearValue(); }
 
+    Span<const app::AttributePathParams> GetExtraReadPaths() const { return mExtraReadPaths; }
+
+    // Additional attribute paths to read as part of the kReadCommissioningInfo stage.
+    // These values read from the device will be available in ReadCommissioningInfo.attributes.
+    // Clients should avoid requesting paths that are already read internally by the commissioner
+    // as no consolidation of internally read and extra paths provided here will be performed.
+    CommissioningParameters & SetExtraReadPaths(Span<const app::AttributePathParams> paths)
+    {
+        mExtraReadPaths = paths;
+        return *this;
+    }
+
     // Clear all members that depend on some sort of external buffer.  Can be
     // used to make sure that we are not holding any dangling pointers.
     void ClearExternalBufferDependentValues()
@@ -621,6 +636,7 @@ class CommissioningParameters
         mDSTOffsets.ClearValue();
         mDefaultNTP.ClearValue();
         mICDSymmetricKey.ClearValue();
+        mExtraReadPaths = decltype(mExtraReadPaths)();
     }
 
 private:
@@ -671,6 +687,7 @@ class CommissioningParameters
     ICDRegistrationStrategy mICDRegistrationStrategy = ICDRegistrationStrategy::kIgnore;
     bool mCheckForMatchingFabric                     = false;
     bool mRequireTermsAndConditionsAcknowledgement   = false;
+    Span<const app::AttributePathParams> mExtraReadPaths;
 };
 
 struct RequestedCertificate
@@ -771,6 +788,9 @@ struct ICDManagementClusterInfo
 
 struct ReadCommissioningInfo
 {
+#if CHIP_CONFIG_ENABLE_READ_CLIENT
+    app::ClusterStateCache const * attributes = nullptr;
+#endif
     NetworkClusters network;
     BasicClusterInfo basic;
     GeneralCommissioningInfo general;

src/darwin/Framework/CHIP/MTRAttributeTLVValueDecoder_Internal.h

@@ -1,6 +1,5 @@
-/*
- *
- *    Copyright (c) 2021 Project CHIP Authors
+/**
+ *    Copyright (c) 2021-2024 Project CHIP Authors
  *    All rights reserved.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -16,17 +15,21 @@
  *    limitations under the License.
  */
 
-#pragma once
-
 #import <Foundation/Foundation.h>
 
+#include <app/ClusterStateCache.h>
 #include <app/ConcreteAttributePath.h>
 #include <lib/core/CHIPError.h>
 #include <lib/core/TLV.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
+// Decodes an attribute value TLV into a typed ObjC value (see MTRStructsObjc.h)
 id _Nullable MTRDecodeAttributeValue(const chip::app::ConcreteAttributePath & aPath, chip::TLV::TLVReader & aReader,
                                      CHIP_ERROR * aError);
 
+// Wrapper around the precending function that reads the attribute from a ClusterStateCache.
+id _Nullable MTRDecodeAttributeValue(const chip::app::ConcreteAttributePath & aPath, const chip::app::ClusterStateCache & aCache,
+                                     CHIP_ERROR * aError);
+
 NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTRAttributeTLVValueDecoder_Internal.mm

@@ -0,0 +1,34 @@
+/**
+ *    Copyright (c) 2024 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#import "MTRAttributeTLVValueDecoder_Internal.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+using namespace chip;
+
+id _Nullable MTRDecodeAttributeValue(const chip::app::ConcreteAttributePath & aPath,
+    const chip::app::ClusterStateCache & aCache,
+    CHIP_ERROR * aError)
+{
+    TLV::TLVReader reader;
+    *aError = aCache.Get(aPath, reader);
+    VerifyOrReturnValue(*aError == CHIP_NO_ERROR, nil);
+    return MTRDecodeAttributeValue(aPath, reader, aError);
+}
+
+NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTRCommissioningParameters.h

@@ -1,6 +1,5 @@
 /**
- *
- *    Copyright (c) 2022-2023 Project CHIP Authors
+ *    Copyright (c) 2022-2024 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -97,6 +96,12 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  */
 @property (nonatomic, copy, nullable) NSString * countryCode MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0));
 
+/**
+ * Read device type information from all endpoints during commissioning.
+ * Defaults to NO.
+ */
+@property (nonatomic, assign) BOOL readEndpointInformation MTR_NEWLY_AVAILABLE;
+
 @end
 
 @interface MTRCommissioningParameters (Deprecated)

src/darwin/Framework/CHIP/MTRDefines_Internal.h

@@ -1,5 +1,5 @@
 /**
- *    Copyright (c) 2023 Project CHIP Authors
+ *    Copyright (c) 2023-2024 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -30,8 +30,12 @@
 
 #ifdef DEBUG
 #define MTR_TESTABLE MTR_EXPORT
+#define MTR_TESTABLE_DIRECT
+#define MTR_TESTABLE_DIRECT_MEMBERS
 #else
 #define MTR_TESTABLE
+#define MTR_TESTABLE_DIRECT MTR_DIRECT
+#define MTR_TESTABLE_DIRECT_MEMBERS MTR_DIRECT_MEMBERS
 #endif
 
 // clang-format off

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -1,6 +1,5 @@
 /**
- *
- *    Copyright (c) 2020-2023 Project CHIP Authors
+ *    Copyright (c) 2020-2024 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -656,11 +655,13 @@ - (void)controller:(MTRDeviceController *)controller
     } logString:__PRETTY_FUNCTION__];
 }
 
-- (void)controller:(MTRDeviceController *)controller readCommissioningInfo:(MTRProductIdentity *)info
+- (void)controller:(MTRDeviceController *)controller readCommissioneeInfo:(MTRCommissioneeInfo *)info
 {
     [self _callDelegatesWithBlock:^(id<MTRDeviceControllerDelegate> delegate) {
-        if ([delegate respondsToSelector:@selector(controller:readCommissioningInfo:)]) {
-            [delegate controller:controller readCommissioningInfo:info];
+        if ([delegate respondsToSelector:@selector(controller:readCommissioneeInfo:)]) {
+            [delegate controller:controller readCommissioneeInfo:info];
+        } else if ([delegate respondsToSelector:@selector(controller:readCommissioningInfo:)]) {
+            [delegate controller:controller readCommissioningInfo:info.productIdentity];
         }
     } logString:__PRETTY_FUNCTION__];
 }