Switch DataModel::Provider interface to default enabled (except on …

…linux where it stays as check) (#36042) * Switch data model provider usage to default to enabled * Decouple pump configuration and control from compatibility functions. Still 100% ember tied. * Fix up some private/public dependencies: instance header should always be a private dependency * Allow java (or generally dynamic server) controller builds to compile with dynamic dispatcher. Dynamic dispatcher seems to only care about commands so implemented all the rest as empty, which is not ideal * Restyled by clang-format * Restyled by gn * Better dynamic dispatch implementation, removed extra TODO functions * Restyled by whitespace * Update dep a bit more * Fix linter * Fix linter again * Update NXP to support various logging targets and for K32W0 enable progress logging and up (no detail/automation) * Fix nxp compile (ordering issue) * Update logic a bit to use log level for low power * Update builds to consider low power already removing logs * Make microwave oven cluster compilable * Restyle * Add codegen data model to darwin xproject: given that we add ember, we need these to access the ember bits as there is no app or data model to provide these * Add missing override for darwin dispatch when codegen data model uses lower level methods for ember access * Fix linter * Add ability to track structural changes in ember for the purpose of codegen data model caching * Fix typo: uint is unsigned * Restyle * Fix android to also provide a generation version * Fix another uint/unsigned typo ... oops * Add assertion of stack lock in darwin override of emberAfWriteAttribute * Restyled by isort * Remove formatting changes from python files * Pick up restyle changes too * Add comment on attribute laregest * Use TLV-based encoding for ember data buffer. This saves 2K of flash on some test devices, being much more efficient in code size compared to using datamodel code. * Restyled by clang-format * Restyled by shfmt * Undo unrelated change * Add some casts to make android compiler happy * Update darwin build project as well with the new files * Updates based on code review * Added unit tests for min/max int64 values * Rename PascalString to PascalStringType * Fix rename * Restyle * Add helper methods inside odd sized integers to localize code logic * Restyled by clang-format * Fix up negative ranges * Fixed ranges * Fix signed max bug and update unit tests * Make android happy * Typo fix * Switch up unit tests * Update a nullable check to make use of ValueToNullValue * Add namespace prefix * Update src/app/codegen-data-model-provider/EmberDataBuffer.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/codegen-data-model-provider/EmberDataBuffer.h Co-authored-by: Boris Zbarsky <[email protected]> * Correct comments:signed, not unsigned * Use constructors for the buffer info * Rename things to EmberAttributeDataBuffer * Undo submodule updates * Restyled by clang-format * Use EmberAttributeDataBuffer for codegen provider _Read * Fix comments * Restyled by clang-format * Update DynamicDispatcher comment * Rename file * Use a pointer for the subject descriptor. This seems to save about 88 bytes of flash on a test NRF board. * Restyle * Fix include * Fix include * Also fix PW rpc * This saves 136 bytes... * More changes to save slightly more flash for code * Restyle * Fix typo * Fix includes * make more detailed logging optional in the codegen data model and enable it only on known large platforms * Pull back the event path validity mixin, start with a datamodel implementation * Fix compile logic after I moved things away * Add one more check * Restyle * Fix typo * Fix includes * Restyle * Move decodable lists bits as a non-template class to save flash * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Rename method * Restyle * More renames * Restyle * Fix some renames * Restyle * A few more renames * Restyle * Use chip::app::IsSignedAttributeType * Fix up put as well as naming for null value and comment * Fix up nullable tests * Test that you cannot decode a null value for non-nullable double and single * Allow NAN for non-nullable floating points * Add test case for non nullable bool * Restyle * Add a header for efr32 * Update src/app/codegen-data-model-provider/EmberAttributeDataBuffer.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/codegen-data-model-provider/EmberAttributeDataBuffer.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Remove extra comment * Replace switch with if * Comment fix * Another try to make efr32 build of tests happy * Move includes around, to try to work around issues within efr32 compiles... * more updates, this time local efr32 compiles * Remove lookup tables from ember attribute data buffer --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Oct 31, 2024 · daf2bce · daf2bce
1 parent a6f0168
commit daf2bce
Show file tree

Hide file tree

Showing 16 changed files with 196 additions and 18 deletions.
diff --git a/.github/workflows/examples-nxp.yaml b/.github/workflows/examples-nxp.yaml
@@ -70,7 +70,7 @@ jobs:
               run: |
                   scripts/run_in_build_env.sh "\
                       ./scripts/build/build_examples.py \
-                      --target nxp-k32w0-freertos-lighting-factory \
+                      --target nxp-k32w0-freertos-lighting-factory-log-progress \
                       --target nxp-k32w0-freertos-contact-sensor-low-power-factory \
                       build \
                       --copy-artifacts-to out/artifacts \
@@ -79,7 +79,7 @@ jobs:
               run: |
                   .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
                     nxp k32w0+release light \
-                    out/artifacts/nxp-k32w0-freertos-lighting-factory/chip-k32w0x-light-example.elf \
+                    out/artifacts/nxp-k32w0-freertos-lighting-factory-log-progress/chip-k32w0x-light-example.elf \
                     /tmp/bloat_reports/
             - name: Get K32W0 contact sensor size stats
               run: |

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -105,10 +105,8 @@ jobs:
                      --known-failure app/reporting/reporting.cpp \
                      --known-failure app/reporting/tests/MockReportScheduler.cpp \
                      --known-failure app/reporting/tests/MockReportScheduler.h \
-                     --known-failure app/util/attribute-storage.cpp \
                      --known-failure app/util/attribute-storage-detail.h \
                      --known-failure app/util/attribute-storage.h \
-                     --known-failure app/util/attribute-table.cpp \
                      --known-failure app/util/attribute-table-detail.h \
                      --known-failure app/util/attribute-table.h \
                      --known-failure app/util/binding-table.cpp \
@@ -118,9 +116,7 @@ jobs:
                      --known-failure app/util/DataModelHandler.h \
                      --known-failure app/util/ember-compatibility-functions.cpp \
                      --known-failure app/util/ember-compatibility-functions.h \
-                     --known-failure app/util/ember-global-attribute-access-interface.cpp \
                      --known-failure app/util/ember-global-attribute-access-interface.h \
-                     --known-failure app/util/ember-io-storage.cpp \
                      --known-failure app/util/ember-io-storage.h \
                      --known-failure app/util/endpoint-config-api.h \
                      --known-failure app/util/generic-callbacks.h \
@@ -135,7 +131,12 @@ jobs:
                      --known-failure platform/GLibTypeDeleter.h \
                      --known-failure platform/SingletonConfigurationManager.cpp \
                   "
-
+                  # These ARE actually orphaned but due to dynamic-server we have code paths
+                  # for them. Keeping them as a list as they still need review ...
+                  #   --known-failure app/util/attribute-table.cpp \
+                  #   --known-failure app/util/ember-io-storage.cpp \
+                  #   --known-failure app/util/ember-global-attribute-access-interface.cpp \
+                  #   --known-failure app/util/attribute-storage.cpp \
             - name: Check for matter lint errors
               if: always()
               run: |
@@ -295,11 +296,13 @@ jobs:
                       ':(exclude)examples/common/pigweed/rpc_services/Attributes.h'                          \
                       ':(exclude)src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp'     \
                       ':(exclude)src/app/codegen-data-model-provider/tests/EmberReadWriteOverride.cpp'       \
+                      ':(exclude)src/app/dynamic_server/DynamicDispatcher.cpp'                               \
                       ':(exclude)src/app/util/attribute-table.cpp'                                           \
                       ':(exclude)src/app/util/attribute-table.h'                                             \
                       ':(exclude)src/app/util/ember-compatibility-functions.cpp'                             \
                       ':(exclude)src/app/util/mock/CodegenEmberMocks.cpp'                                    \
                       ':(exclude)src/app/zap-templates/templates/app/attributes/Accessors-src.zapt'          \
+                      ':(exclude)src/darwin/Framework/CHIP/ServerEndpoint/MTRIMDispatch.mm'                  \
                       ':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' \
                       && exit 1 || exit 0
 

diff --git a/build/chip/esp32/esp32_codegen.cmake b/build/chip/esp32/esp32_codegen.cmake
@@ -75,7 +75,7 @@ macro(chip_app_component_zapgen ZAP_NAME)
 
     # When data model interface is used, provide a default code-generation data model as
     # part of zapgen. See `chip_data_model.cmake` for similar logic
-    set(CHIP_DATA_MODEL_INTERFACE "disabled" CACHE STRING "Data model interface option to use: enabled or disabled")
+    set(CHIP_DATA_MODEL_INTERFACE "enabled" CACHE STRING "Data model interface option to use: enabled or disabled")
 
     if ("${CHIP_DATA_MODEL_INTERFACE}" STREQUAL "enabled")
       target_sources(${COMPONENT_LIB} PRIVATE ${CODEGEN_DATA_MODEL_SOURCES})

diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt
@@ -33,7 +33,7 @@ include(${CMAKE_CURRENT_LIST_DIR}/ota-image.cmake)
 
 set(CHIP_REQUIRE_COMPONENTS esp_eth freertos lwip bt mbedtls fatfs app_update console openthread nvs_flash spi_flash)
 
-set(CHIP_DATA_MODEL_INTERFACE "disabled" CACHE STRING "Data model interface option to use: enabled or disabled")
+set(CHIP_DATA_MODEL_INTERFACE "enabled" CACHE STRING "Data model interface option to use: enabled or disabled")
 
 if(NOT "${IDF_TARGET}" STREQUAL "esp32h2")
     list(APPEND CHIP_REQUIRE_COMPONENTS mdns)

diff --git a/config/mbed/CMakeLists.txt b/config/mbed/CMakeLists.txt
@@ -113,7 +113,7 @@ if (CONFIG_MBED_BSD_SOCKET_TRACE)
 endif()
 
 # Option can be set with `-DCHIP_DATA_MODEL_INTERFACE=enabled` or similar on the command line
-set(CHIP_DATA_MODEL_INTERFACE "disabled" CACHE STRING "Data model interface option to use: enabled or disabled")
+set(CHIP_DATA_MODEL_INTERFACE "enabled" CACHE STRING "Data model interface option to use: enabled or disabled")
 
 # ==============================================================================
 # Generate configuration for CHIP GN build system

diff --git a/config/zephyr/Kconfig b/config/zephyr/Kconfig
@@ -583,6 +583,7 @@ config CHIP_BLE_ADVERTISING_DURATION
 
 config USE_CHIP_DATA_MODEL_INTERFACE
     bool "Use a DataModel::Provider interface for data access"
+    default y
     help
       This enables a level of indiraction in the CHIP interaction model engine in 
       accessing underlying data and executing operations such as 

diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -226,14 +226,17 @@ static_library("interaction-model") {
 
   deps = [ "${chip_root}/src/app:events" ]
 
+  # Temporary dependency: codegen data provider instance should be provided
+  # by the application
+  deps += [ "${chip_root}/src/app/codegen-data-model-provider:instance-header" ]
+
   public_deps = [
     ":app_config",
     ":command-handler-impl",
     ":constants",
     ":paths",
     ":subscription-info-provider",
     "${chip_root}/src/app/MessageDef",
-    "${chip_root}/src/app/codegen-data-model-provider:instance-header",
     "${chip_root}/src/app/data-model-provider",
     "${chip_root}/src/app/icd/server:icd-server-config",
     "${chip_root}/src/app/icd/server:manager",
@@ -291,13 +294,23 @@ static_library("interaction-model") {
       "clusters/ota-provider/ota-provider.cpp",
       "dynamic_server/AccessControl.cpp",
       "dynamic_server/AccessControl.h",
+    ]
+
+    # DynamicDispatcher is actually an ember-override that takes over
+    # util/attribute-storage.cpp and util/attribute-table.cpp functions.
+    #
+    # We likely should formalize and change this with a proper DataModel::Provider that
+    # is consistent instead
+    sources += [
+      "${chip_root}/src/app/util/ember-global-attribute-access-interface.cpp",
+      "${chip_root}/src/app/util/ember-io-storage.cpp",
       "dynamic_server/DynamicDispatcher.cpp",
     ]
 
     public_deps += [
       ":global-attributes",
       "${chip_root}/src/access",
-      "${chip_root}/src/app/dynamic_server:mock-codegen-includes",
+      "${chip_root}/src/app/common:attribute-type",
     ]
 
     public_configs += [ ":config-controller-dynamic-server" ]

diff --git a/src/app/clusters/microwave-oven-control-server/microwave-oven-control-server.cpp b/src/app/clusters/microwave-oven-control-server/microwave-oven-control-server.cpp
@@ -248,11 +248,16 @@ void Instance::HandleSetCookingParameters(HandlerContext & ctx, const Commands::
 
     if (startAfterSetting.HasValue())
     {
+        ConcreteCommandPath commandPath(mEndpointId, OperationalState::Id, OperationalState::Commands::Start::Id);
+
+#if CHIP_CONFIG_USE_EMBER_DATA_MODEL
+        bool commandExists = ServerClusterCommandExists(commandPath) == Status::Success;
+#else
+        bool commandExists =
+            InteractionModelEngine::GetInstance()->GetDataModelProvider()->GetAcceptedCommandInfo(commandPath).has_value();
+#endif
         VerifyOrExit(
-            ServerClusterCommandExists(
-                ConcreteCommandPath(mEndpointId, OperationalState::Id, OperationalState::Commands::Start::Id)) == Status::Success,
-            status = Status::InvalidCommand;
-            ChipLogError(
+            commandExists, status = Status::InvalidCommand; ChipLogError(
                 Zcl,
                 "Microwave Oven Control: Failed to set cooking parameters, Start command of operational state is not supported"));
     }

diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni
@@ -32,7 +32,7 @@ declare_args() {
   if (matter_enable_recommended && current_os == "linux") {
     chip_use_data_model_interface = "check"
   } else {
-    chip_use_data_model_interface = "disabled"
+    chip_use_data_model_interface = "enabled"
   }
 
   # Whether we call `chipDie` on DM `check` errors

diff --git a/src/app/dynamic_server/DynamicDispatcher.cpp b/src/app/dynamic_server/DynamicDispatcher.cpp
@@ -32,7 +32,9 @@
 #include <app/WriteHandler.h>
 #include <app/data-model/Decode.h>
 #include <app/util/att-storage.h>
+#include <app/util/attribute-table.h>
 #include <app/util/endpoint-config-api.h>
+#include <cstddef>
 #include <lib/core/CHIPError.h>
 #include <lib/core/DataModelTypes.h>
 #include <lib/core/Optional.h>
@@ -57,6 +59,8 @@ namespace {
 // AccessControl.cpp.
 constexpr EndpointId kSupportedEndpoint = 0;
 
+DataVersion gMockDataVersion = 0;
+
 } // anonymous namespace
 
 namespace chip {
@@ -381,3 +385,63 @@ const EmberAfCluster * emberAfFindServerCluster(EndpointId endpoint, ClusterId c
 
     return nullptr;
 }
+
+unsigned emberAfMetadataStructureGeneration()
+{
+    // DynamicDispatcher at this point hardcodes a single OTA provider cluster.
+    // The structure does not change over time, so the current version stays at 0.
+    return 0;
+}
+
+Protocols::InteractionModel::Status emberAfWriteAttribute(const ConcreteAttributePath & path, const EmberAfWriteDataInput & input)
+{
+    return Protocols::InteractionModel::Status::UnsupportedAttribute;
+}
+
+Protocols::InteractionModel::Status emAfReadOrWriteAttribute(const EmberAfAttributeSearchRecord * attRecord,
+                                                             const EmberAfAttributeMetadata ** metadata, uint8_t * buffer,
+                                                             uint16_t readLength, bool write)
+{
+    return Protocols::InteractionModel::Status::UnsupportedAttribute;
+}
+
+void emberAfAttributeChanged(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId,
+                             AttributesChangedListener * listener)
+{
+    gMockDataVersion++;
+    listener->MarkDirty(AttributePathParams(endpoint, clusterId, attributeId));
+}
+
+DataVersion * emberAfDataVersionStorage(const ConcreteClusterPath & aConcreteClusterPath)
+{
+    return &gMockDataVersion;
+}
+
+Protocols::InteractionModel::Status emAfWriteAttributeExternal(const ConcreteAttributePath & path,
+                                                               const EmberAfWriteDataInput & input)
+{
+    return Protocols::InteractionModel::Status::UnsupportedAttribute;
+}
+
+Span<const EmberAfDeviceType> emberAfDeviceTypeListFromEndpointIndex(unsigned endpointIndex, CHIP_ERROR & err)
+{
+    err = CHIP_ERROR_NOT_IMPLEMENTED;
+    return Span<const EmberAfDeviceType>();
+}
+
+const EmberAfCluster * emberAfFindClusterInType(const EmberAfEndpointType * endpointType, ClusterId clusterId,
+                                                EmberAfClusterMask mask, uint8_t * index)
+{
+    if ((endpointType == &otaProviderEndpoint) && (clusterId == Clusters::OtaSoftwareUpdateProvider::Id))
+    {
+        return &otaProviderCluster;
+    }
+
+    return nullptr;
+}
+
+const EmberAfAttributeMetadata * emberAfLocateAttributeMetadata(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
+{
+    // no known attributes even for OTA
+    return nullptr;
+}
diff --git a/src/app/dynamic_server/mock_includes/zap-generated/endpoint_config.h b/src/app/dynamic_server/mock_includes/zap-generated/endpoint_config.h
diff --git a/src/app/icd/client/BUILD.gn b/src/app/icd/client/BUILD.gn
@@ -46,7 +46,6 @@ source_set("handler") {
   ]
   public_deps = [
     ":manager",
-    "${chip_root}/src/app",
     "${chip_root}/src/controller",
     "${chip_root}/src/lib/core",
     "${chip_root}/src/messaging",

diff --git a/src/app/util/mock/include/zap-generated/endpoint_config.h b/src/app/util/mock/include/zap-generated/endpoint_config.h
diff --git a/src/controller/java/BUILD.gn b/src/controller/java/BUILD.gn
@@ -19,6 +19,7 @@ import("${chip_root}/build/chip/buildconfig_header.gni")
 import("${chip_root}/build/chip/java/config.gni")
 import("${chip_root}/build/chip/java/rules.gni")
 import("${chip_root}/build/chip/tests.gni")
+import("${chip_root}/src/app/codegen-data-model-provider/model.gni")
 import("${chip_root}/src/app/common_flags.gni")
 import("${chip_root}/src/platform/device.gni")
 
@@ -171,6 +172,15 @@ shared_library("jni") {
     defines += [ "CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES=1" ]
 
     deps += [ "${chip_root}/src/controller:nodatamodel" ]
+
+    # Temporary dependency: InteractionModelEngine NEEDS a codegen data model instance
+    # defined and application is supposed to provide it. This adds the sources
+    # in the same way "data_model" implementations do
+    #
+    # DynamicDispatcher in src/app:interaction-model implements the actual required
+    # ember callbacks in this case...
+    sources += codegen_data_model_SOURCES
+    public_deps = codegen_data_model_PUBLIC_DEPS
   } else {
     deps += [ "${chip_root}/src/controller/data_model" ]
   }

diff --git a/src/darwin/Framework/CHIP/ServerEndpoint/MTRIMDispatch.mm b/src/darwin/Framework/CHIP/ServerEndpoint/MTRIMDispatch.mm
@@ -50,6 +50,15 @@ void emberAfClusterInitCallback(EndpointId endpoint, ClusterId clusterId)
     return Protocols::InteractionModel::Status::UnsupportedAttribute;
 }
 
+Protocols::InteractionModel::Status emberAfWriteAttribute(const ConcreteAttributePath & path, const EmberAfWriteDataInput & input)
+{
+    assertChipStackLockedByCurrentThread();
+
+    // All of our attributes are handled via AttributeAccessInterface, so this
+    // should be unreached.
+    return Protocols::InteractionModel::Status::UnsupportedAttribute;
+}
+
 namespace chip {
 namespace app {