One more decouple from ember from src/app and ensure we can compile w…

…ithout `ember-compatibility-functions.cpp` (#35919) * Add ability to skip compiling of ember-compatibility-functions * Fix build and make sure that CI will validate a data-model-enabled build * Restyle * Fix compilation * Restyled by clang-format * Typo fix * Fix includes * Rename method to something that seems a bit clearer * Fix pwrpc usage to use datamodel interface * Restyle * Remove todo * Update for attribute is list to support not know value * Update src/app/WriteHandler.h Co-authored-by: Boris Zbarsky <[email protected]> --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Oct 8, 2024 · 07a28ac · 07a28ac
1 parent 52a5589
commit 07a28ac
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 15 deletions.
diff --git a/.github/workflows/examples-linux-arm.yaml b/.github/workflows/examples-linux-arm.yaml
@@ -65,7 +65,7 @@ jobs:
                         --target linux-arm64-chip-tool-nodeps-ipv6only \
                         --target linux-arm64-lock-clang \
                         --target linux-arm64-minmdns-clang \
-                        --target linux-arm64-light-rpc-ipv6only-clang \
+                        --target linux-arm64-light-data-model-enabled-rpc-ipv6only-clang \
                         --target linux-arm64-thermostat-no-ble-clang \
                         --target linux-arm64-lit-icd-no-ble-clang \
                         --target linux-arm64-fabric-admin-clang-rpc \

diff --git a/examples/common/pigweed/rpc_services/Attributes.h b/examples/common/pigweed/rpc_services/Attributes.h
@@ -22,6 +22,7 @@
 #include "pigweed/rpc_services/internal/StatusUtils.h"
 
 #include <app-common/zap-generated/attribute-type.h>
+#include <app/AppConfig.h>
 #include <app/InteractionModelEngine.h>
 #include <app/MessageDef/AttributeReportIBs.h>
 #include <app/util/attribute-storage.h>
@@ -32,6 +33,13 @@
 #include <lib/core/TLVTypes.h>
 #include <platform/PlatformManager.h>
 
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+#include <app/AttributeValueEncoder.h>
+#include <app/data-model-provider/ActionReturnStatus.h>
+#include <app/data-model-provider/OperationTypes.h>
+#include <app/data-model-provider/Provider.h>
+#endif
+
 namespace chip {
 namespace rpc {
 
@@ -202,7 +210,37 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
         writer.Init(tlvBuffer);
         PW_TRY(ChipErrorToPwStatus(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)));
         PW_TRY(ChipErrorToPwStatus(attributeReports.Init(&writer, kReportContextTag)));
+
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+        // TODO: this assumes a singleton data model provider
+        app::DataModel::Provider * provider = app::InteractionModelEngine::GetInstance()->GetDataModelProvider();
+
+        app::DataModel::ReadAttributeRequest request;
+        request.path = path;
+        request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
+        request.subjectDescriptor = subjectDescriptor;
+
+        std::optional<app::DataModel::ClusterInfo> info = provider->GetClusterInfo(path);
+        if (!info.has_value())
+        {
+            return ::pw::Status::NotFound();
+        }
+
+        app::AttributeValueEncoder encoder(attributeReports, subjectDescriptor, path, info->dataVersion,
+                                           false /* isFabricFiltered */, nullptr /* attributeEncodingState */);
+        app::DataModel::ActionReturnStatus result = provider->ReadAttribute(request, encoder);
+
+        if (!result.IsSuccess())
+        {
+            app::DataModel::ActionReturnStatus::StringStorage storage;
+            ChipLogError(Support, "Failed to read data: %s", result.c_str(storage));
+            return ::pw::Status::Internal();
+        }
+
+#else
         PW_TRY(ChipErrorToPwStatus(app::ReadSingleClusterData(subjectDescriptor, false, path, attributeReports, nullptr)));
+#endif
+
         attributeReports.EndOfContainer();
         PW_TRY(ChipErrorToPwStatus(writer.EndContainer(outer)));
         PW_TRY(ChipErrorToPwStatus(writer.Finalize()));

diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp
@@ -24,6 +24,7 @@
 #include <app/MessageDef/StatusIB.h>
 #include <app/StatusResponse.h>
 #include <app/WriteHandler.h>
+#include <app/data-model-provider/MetadataTypes.h>
 #include <app/data-model-provider/OperationTypes.h>
 #include <app/reporting/Engine.h>
 #include <app/util/MatterCallbacks.h>
@@ -32,6 +33,7 @@
 #include <lib/core/CHIPError.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/TypeTraits.h>
+#include <lib/support/logging/TextOnlyLogging.h>
 #include <messaging/ExchangeContext.h>
 #include <protocols/interaction_model/StatusCode.h>
 
@@ -41,8 +43,7 @@ namespace chip {
 namespace app {
 
 using namespace Protocols::InteractionModel;
-using Status                         = Protocols::InteractionModel::Status;
-constexpr uint8_t kListAttributeType = 0x48;
+using Status = Protocols::InteractionModel::Status;
 
 CHIP_ERROR WriteHandler::Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate)
 {
@@ -79,6 +80,30 @@ void WriteHandler::Close()
     MoveToState(State::Uninitialized);
 }
 
+std::optional<bool> WriteHandler::IsListAttributePath(const ConcreteAttributePath & path)
+{
+#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+    VerifyOrReturnValue(mDataModelProvider != nullptr, std::nullopt,
+                        ChipLogError(DataManagement, "Null data model while checking attribute properties."));
+
+    auto info = mDataModelProvider->GetAttributeInfo(path);
+    if (!info.has_value())
+    {
+        return std::nullopt;
+    }
+
+    return info->flags.Has(DataModel::AttributeQualityFlags::kListAttribute);
+#else
+    constexpr uint8_t kListAttributeType = 0x48;
+    const auto attributeMetadata         = GetAttributeMetadata(path);
+    if (attributeMetadata == nullptr)
+    {
+        return std::nullopt;
+    }
+    return (attributeMetadata->attributeType == kListAttributeType);
+#endif
+}
+
 Status WriteHandler::HandleWriteRequestMessage(Messaging::ExchangeContext * apExchangeContext,
                                                System::PacketBufferHandle && aPayload, bool aIsTimedWrite)
 {
@@ -317,10 +342,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
         err = element.GetData(&dataReader);
         SuccessOrExit(err);
 
-        const auto attributeMetadata = GetAttributeMetadata(dataAttributePath);
-        bool currentAttributeIsList  = (attributeMetadata != nullptr && attributeMetadata->attributeType == kListAttributeType);
-
-        if (!dataAttributePath.IsListOperation() && currentAttributeIsList)
+        if (!dataAttributePath.IsListOperation() && IsListAttributePath(dataAttributePath).value_or(false))
         {
             dataAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
         }
@@ -446,7 +468,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
             mProcessingAttributePath, mStateFlags.Has(StateBits::kProcessingAttributeIsList), dataAttributePath);
         bool shouldReportListWriteBegin = false; // This will be set below.
 
-        const EmberAfAttributeMetadata * attributeMetadata = nullptr;
+        std::optional<bool> isListAttribute = std::nullopt;
 
         while (iterator->Next(mapping))
         {
@@ -460,11 +482,11 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
             // Try to get the metadata from for the attribute from one of the expanded endpoints (it doesn't really matter which
             // endpoint we pick, as long as it's valid) and update the path info according to it and recheck if we need to report
             // list write begin.
-            if (attributeMetadata == nullptr)
+            if (!isListAttribute.has_value())
             {
-                attributeMetadata = GetAttributeMetadata(dataAttributePath);
-                bool currentAttributeIsList =
-                    (attributeMetadata != nullptr && attributeMetadata->attributeType == kListAttributeType);
+                isListAttribute             = IsListAttributePath(dataAttributePath);
+                bool currentAttributeIsList = isListAttribute.value_or(false);
+
                 if (!dataAttributePath.IsListOperation() && currentAttributeIsList)
                 {
                     dataAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;

diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h
@@ -189,6 +189,12 @@ class WriteHandler : public Messaging::ExchangeDelegate
     CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
                                 TLV::TLVReader & aData);
 
+    /// Checks whether the given path corresponds to a list attribute
+    /// Return values:
+    ///    true/false: valid attribute path, known if list or not
+    ///    std::nulloptr - path not available/valid, unknown if attribute is a list or not
+    std::optional<bool> IsListAttributePath(const ConcreteAttributePath & path);
+
     Messaging::ExchangeHolder mExchangeCtx;
     WriteResponseMessage::Builder mWriteResponseBuilder;
     Optional<ConcreteAttributePath> mProcessingAttributePath;

diff --git a/src/app/chip_data_model.cmake b/src/app/chip_data_model.cmake
@@ -70,12 +70,17 @@ endfunction()
 #
 function(chip_configure_data_model APP_TARGET)
     set(SCOPE PRIVATE)
-    cmake_parse_arguments(ARG "" "SCOPE;ZAP_FILE;IDL" "EXTERNAL_CLUSTERS" ${ARGN})
+    set(ADD_EMBER_INTERFACE_FILES TRUE)
+    cmake_parse_arguments(ARG "SKIP_EMBER_INTERFACE" "SCOPE;ZAP_FILE;IDL" "EXTERNAL_CLUSTERS" ${ARGN})
 
     if(ARG_SCOPE)
         set(SCOPE ${ARG_SCOPE})
     endif()
 
+    if(ARG_SKIP_EMBER_INTERFACE)
+        set(ADD_EMBER_INTERFACE_FILES FALSE)
+    endif()
+
     # CMAKE data model auto-includes the server side implementation
     target_sources(${APP_TARGET} ${SCOPE}
         ${CHIP_APP_BASE_DIR}/server/AclStorage.cpp
@@ -159,7 +164,6 @@ function(chip_configure_data_model APP_TARGET)
         ${CHIP_APP_BASE_DIR}/util/attribute-table.cpp
         ${CHIP_APP_BASE_DIR}/util/binding-table.cpp
         ${CHIP_APP_BASE_DIR}/util/DataModelHandler.cpp
-        ${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
         ${CHIP_APP_BASE_DIR}/util/ember-global-attribute-access-interface.cpp
         ${CHIP_APP_BASE_DIR}/util/ember-io-storage.cpp
         ${CHIP_APP_BASE_DIR}/util/generic-callback-stubs.cpp
@@ -169,4 +173,10 @@ function(chip_configure_data_model APP_TARGET)
         ${APP_GEN_FILES}
         ${APP_TEMPLATES_GEN_FILES}
     )
+
+    if(ADD_EMBER_INTERFACE_FILES)
+        target_sources(${APP_TARGET} ${SCOPE}
+           ${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
+        )
+    endif()
 endfunction()
diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni
@@ -209,11 +209,15 @@ template("chip_data_model") {
         "${_app_root}/util/DataModelHandler.cpp",
         "${_app_root}/util/attribute-storage.cpp",
         "${_app_root}/util/attribute-table.cpp",
-        "${_app_root}/util/ember-compatibility-functions.cpp",
         "${_app_root}/util/ember-global-attribute-access-interface.cpp",
         "${_app_root}/util/ember-io-storage.cpp",
         "${_app_root}/util/util.cpp",
       ]
+
+      if (chip_use_data_model_interface != "enabled") {
+        # This applies to "check" or "disabled" (i.e. use ember-only or use ember for double-checking)
+        sources += [ "${_app_root}/util/ember-compatibility-functions.cpp" ]
+      }
     }
 
     if (defined(invoker.zap_file)) {