From 6768739be4cfb35b6b147bebc424897a874c01cf Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Wed, 24 Jul 2024 22:09:54 +0530 Subject: [PATCH 01/10] [ESP32] Changes to support product-appearance attribute in ESP32. (#34363) * Changes to support product-appearance attribute on ESP32. - Added the support for product-appearance attribute of BasicInformationCluster in factory script. - Added DeviceInstanceInfoProvider implementation for the attribute. * Addressed review comments * Changes to support few more missing attributes --- .../tools/generate_esp32_chip_factory_bin.py | 85 ++++++++++++++++++- src/platform/ESP32/ESP32Config.cpp | 3 + src/platform/ESP32/ESP32Config.h | 3 + .../ESP32/ESP32FactoryDataProvider.cpp | 33 ++++++- src/platform/ESP32/ESP32FactoryDataProvider.h | 2 + 5 files changed, 124 insertions(+), 2 deletions(-) diff --git a/scripts/tools/generate_esp32_chip_factory_bin.py b/scripts/tools/generate_esp32_chip_factory_bin.py index eab2ec6b43c229..b19748ca31b197 100755 --- a/scripts/tools/generate_esp32_chip_factory_bin.py +++ b/scripts/tools/generate_esp32_chip_factory_bin.py @@ -21,6 +21,7 @@ import logging import os import sys +from enum import Enum from types import SimpleNamespace import cryptography.x509 @@ -45,6 +46,40 @@ INVALID_PASSCODES = [00000000, 11111111, 22222222, 33333333, 44444444, 55555555, 66666666, 77777777, 88888888, 99999999, 12345678, 87654321] + +class Product_Finish_Enum(Enum): + other = 0 + matte = 1 + satin = 2 + polished = 3 + rugged = 4 + fabric = 5 + + +class Product_Color_Enum(Enum): + black = 0 + navy = 1 + green = 2 + teal = 3 + maroon = 4 + purple = 5 + olive = 6 + gray = 7 + blue = 8 + lime = 9 + aqua = 10 + red = 11 + fuchsia = 12 + yellow = 13 + white = 14 + nickel = 15 + chrome = 16 + brass = 17 + copper = 18 + silver = 19 + gold = 20 + + TOOLS = {} FACTORY_PARTITION_CSV = 'nvs_partition.csv' @@ -149,6 +184,31 @@ 'encoding': 'hex2bin', 'value': None, }, + 'product-finish': { + 'type': 'data', + 'encoding': 'u32', + 'value': None, + }, + 'product-color': { + 'type': 'data', + 'encoding': 'u32', + 'value': None, + }, + 'part-number': { + 'type': 'data', + 'encoding': 'string', + 'value': None, + }, + 'product-label': { + 'type': 'data', + 'encoding': 'string', + 'value': None, + }, + 'product-url': { + 'type': 'data', + 'encoding': 'string', + 'value': None, + }, } @@ -301,6 +361,16 @@ def populate_factory_data(args, spake2p_params): FACTORY_DATA['hardware-ver']['value'] = args.hw_ver if args.hw_ver_str: FACTORY_DATA['hw-ver-str']['value'] = args.hw_ver_str + if args.product_finish: + FACTORY_DATA['product-finish']['value'] = Product_Finish_Enum[args.product_finish].value + if args.product_color: + FACTORY_DATA['product-color']['value'] = Product_Color_Enum[args.product_color].value + if args.part_number: + FACTORY_DATA['part-number']['value'] = args.part_number + if args.product_url: + FACTORY_DATA['product-url']['value'] = args.product_url + if args.product_label: + FACTORY_DATA['product-label']['value'] = args.product_label # SupportedModes are stored as multiple entries # - sm-sz/ : number of supported modes for the endpoint @@ -471,6 +541,18 @@ def any_base_int(s): return int(s, 0) parser.add_argument('--supported-modes', type=str, nargs='+', required=False, help='List of supported modes, eg: mode1/label1/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode" mode2/label2/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode" mode3/label3/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode"') + product_finish_choices = [finish.name for finish in Product_Finish_Enum] + parser.add_argument("--product-finish", type=str, choices=product_finish_choices, + help='Product finishes choices for product appearance') + + product_color_choices = [color.name for color in Product_Color_Enum] + parser.add_argument("--product-color", type=str, choices=product_color_choices, + help='Product colors choices for product appearance') + + parser.add_argument("--part-number", type=str, help='human readable product number') + parser.add_argument("--product-label", type=str, help='human readable product label') + parser.add_argument("--product-url", type=str, help='link to product specific web page') + parser.add_argument('-s', '--size', type=any_base_int, default=0x6000, help='The size of the partition.bin, default: 0x6000') parser.add_argument('--target', default='esp32', @@ -509,7 +591,8 @@ def set_up_factory_data(args): def generate_factory_partiton_binary(args): generate_nvs_csv(args.output_dir, FACTORY_PARTITION_CSV) if args.generate_bin: - generate_nvs_bin(args.encrypt, args.size, FACTORY_PARTITION_CSV, FACTORY_PARTITION_BIN, args.output_dir) + csv_file = os.path.join(args.output_dir, FACTORY_PARTITION_CSV) + generate_nvs_bin(args.encrypt, args.size, csv_file, FACTORY_PARTITION_BIN, args.output_dir) print_flashing_help(args.encrypt, args.output_dir, FACTORY_PARTITION_BIN) clean_up() diff --git a/src/platform/ESP32/ESP32Config.cpp b/src/platform/ESP32/ESP32Config.cpp index 04e02f63004207..c94de5c04b85b2 100644 --- a/src/platform/ESP32/ESP32Config.cpp +++ b/src/platform/ESP32/ESP32Config.cpp @@ -77,6 +77,9 @@ const ESP32Config::Key ESP32Config::kConfigKey_ProductURL = { kConfig const ESP32Config::Key ESP32Config::kConfigKey_SupportedCalTypes = { kConfigNamespace_ChipFactory, "cal-types" }; const ESP32Config::Key ESP32Config::kConfigKey_SupportedLocaleSize = { kConfigNamespace_ChipFactory, "locale-sz" }; const ESP32Config::Key ESP32Config::kConfigKey_RotatingDevIdUniqueId = { kConfigNamespace_ChipFactory, "rd-id-uid" }; +const ESP32Config::Key ESP32Config::kConfigKey_ProductFinish = { kConfigNamespace_ChipFactory, "product-finish" }; +const ESP32Config::Key ESP32Config::kConfigKey_ProductColor = { kConfigNamespace_ChipFactory, "product-color" }; +const ESP32Config::Key ESP32Config::kConfigKey_PartNumber = { kConfigNamespace_ChipFactory, "part-number" }; const ESP32Config::Key ESP32Config::kConfigKey_LocationCapability = { kConfigNamespace_ChipFactory, "loc-capability" }; // Keys stored in the chip-config namespace diff --git a/src/platform/ESP32/ESP32Config.h b/src/platform/ESP32/ESP32Config.h index 5804f53105c20c..218f2354b2b358 100644 --- a/src/platform/ESP32/ESP32Config.h +++ b/src/platform/ESP32/ESP32Config.h @@ -75,10 +75,13 @@ class ESP32Config static const Key kConfigKey_ProductId; static const Key kConfigKey_ProductName; static const Key kConfigKey_ProductLabel; + static const Key kConfigKey_PartNumber; static const Key kConfigKey_ProductURL; static const Key kConfigKey_SupportedCalTypes; static const Key kConfigKey_SupportedLocaleSize; static const Key kConfigKey_RotatingDevIdUniqueId; + static const Key kConfigKey_ProductFinish; + static const Key kConfigKey_ProductColor; static const Key kConfigKey_LocationCapability; // CHIP Config keys diff --git a/src/platform/ESP32/ESP32FactoryDataProvider.cpp b/src/platform/ESP32/ESP32FactoryDataProvider.cpp index 95d2be5c5d1957..ace2a087de144b 100644 --- a/src/platform/ESP32/ESP32FactoryDataProvider.cpp +++ b/src/platform/ESP32/ESP32FactoryDataProvider.cpp @@ -245,6 +245,32 @@ CHIP_ERROR ESP32FactoryDataProvider::GetManufacturingDate(uint16_t & year, uint8 return GenericDeviceInstanceInfoProvider::GetManufacturingDate(year, month, day); } +CHIP_ERROR ESP32FactoryDataProvider::GetProductFinish(app::Clusters::BasicInformation::ProductFinishEnum * finish) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + uint32_t productFinish = 0; + + err = ESP32Config::ReadConfigValue(ESP32Config::kConfigKey_ProductFinish, productFinish); + ReturnErrorCodeIf(err != CHIP_NO_ERROR, CHIP_ERROR_NOT_IMPLEMENTED); + + *finish = static_cast(productFinish); + + return err; +} + +CHIP_ERROR ESP32FactoryDataProvider::GetProductPrimaryColor(app::Clusters::BasicInformation::ColorEnum * primaryColor) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + uint32_t color = 0; + + err = ESP32Config::ReadConfigValue(ESP32Config::kConfigKey_ProductColor, color); + ReturnErrorCodeIf(err != CHIP_NO_ERROR, CHIP_ERROR_NOT_IMPLEMENTED); + + *primaryColor = static_cast(color); + + return err; +} + CHIP_ERROR ESP32FactoryDataProvider::GetHardwareVersion(uint16_t & hardwareVersion) { return GenericDeviceInstanceInfoProvider::GetHardwareVersion(hardwareVersion); @@ -252,7 +278,12 @@ CHIP_ERROR ESP32FactoryDataProvider::GetHardwareVersion(uint16_t & hardwareVersi CHIP_ERROR ESP32FactoryDataProvider::GetPartNumber(char * buf, size_t bufSize) { - return GenericDeviceInstanceInfoProvider::GetPartNumber(buf, bufSize); + CHIP_ERROR err = ESP32Config::ReadConfigValueStr(ESP32Config::kConfigKey_PartNumber, buf, bufSize, bufSize); + if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + return CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND; + } + return err; } #endif // CHIP_DEVICE_CONFIG_ENABLE_DEVICE_INSTANCE_INFO_PROVIDER diff --git a/src/platform/ESP32/ESP32FactoryDataProvider.h b/src/platform/ESP32/ESP32FactoryDataProvider.h index 1d78f2c2e8fa0b..08227ed4ed143b 100644 --- a/src/platform/ESP32/ESP32FactoryDataProvider.h +++ b/src/platform/ESP32/ESP32FactoryDataProvider.h @@ -96,6 +96,8 @@ class ESP32FactoryDataProvider : public CommissionableDataProvider, CHIP_ERROR GetManufacturingDate(uint16_t & year, uint8_t & month, uint8_t & day) override; CHIP_ERROR GetPartNumber(char * buf, size_t bufSize) override; CHIP_ERROR GetHardwareVersion(uint16_t & hardwareVersion) override; + CHIP_ERROR GetProductFinish(app::Clusters::BasicInformation::ProductFinishEnum * finish) override; + CHIP_ERROR GetProductPrimaryColor(app::Clusters::BasicInformation::ColorEnum * primaryColor) override; #endif // CHIP_DEVICE_CONFIG_ENABLE_DEVICE_INSTANCE_INFO_PROVIDER private: From 8ad0bdfe3f2ff2a45e00981b736059bb7aabd603 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 24 Jul 2024 15:17:38 -0400 Subject: [PATCH 02/10] Fix typo in MTRDevice comment. (#34463) --- src/darwin/Framework/CHIP/MTRDevice.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index 05bc9f0ee6b0ac..1c68d788943ce2 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -913,7 +913,7 @@ - (void)invalidate // Destroy the read client and callback (has to happen on the Matter // queue, to avoid deleting objects that are being referenced), to // tear down the subscription. We will get no more callbacks from - // the subscrption after this point. + // the subscription after this point. std::lock_guard lock(self->_lock); self->_currentReadClient = nullptr; if (self->_currentSubscriptionCallback) { From c88d5cf83cd3e3323ac196630acc34f196a2f405 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 24 Jul 2024 16:30:35 -0400 Subject: [PATCH 03/10] Implement DataModel based read support in the interaction model engine. (#34419) * Implement DataModel based read support in the interacton model engine. This implements reads via ember and via codegen as well as a checked-mode where reads are validated as returning the same status codes and data size (because data content is not available) * Comment update and logic update: we MAY get different sizes if data size changes for numbers * Fix typo for ember implementation while renaming things * Fix override markers * run_tv_casting_test should be executable * Do not report errors on chunking * Typo fix * move chipDie for tests only * move all chipdie to unit test only * fix comment and formatting * Update encoderstate logic a bit - code is cleaner, will have to see if CI tests pass * Restyle * Enable tracing of messages for tv tests, to see what is going on in CI * Restyle * Start adding some log line processing for the tv tests, to have propper timeouts and not block on IO buffers * Significant reformat and start refactoring the tv casting test * TV tests pass * Restyle * Fix ruff * Review comment update: set state in all cases * Added a TODO regarding the awkward "callback on success only" * Merge fix * Update src/app/reporting/Read-Checked.cpp Co-authored-by: Boris Zbarsky * Review updates * Fix placement of dm state * Restyle * Code review comments: double-check that IM not active when setting model, explain why we have the ifdef * Code review: comment why we did not re-use code * Code review feedback: warn if running in checked mode * Restyle * Avoid loop of err/out empty output * Support a log directory argument for the casting tests, so I can debug their content * Better debuggability and error reporting support for shell - this is to debug cast failures --------- Co-authored-by: Andrei Litvin Co-authored-by: Boris Zbarsky --- scripts/tests/linux/log_line_processing.py | 13 +- scripts/tests/run_tv_casting_test.py | 22 ++- src/app/AttributeValueEncoder.h | 1 + src/app/BUILD.gn | 34 ++-- src/app/InteractionModelEngine.cpp | 19 +++ src/app/InteractionModelEngine.h | 11 +- src/app/common_flags.gni | 10 ++ src/app/data-model-interface/OperationTypes.h | 1 - src/app/reporting/Engine.cpp | 24 +-- src/app/reporting/Engine.h | 3 - src/app/reporting/Read-Checked.cpp | 160 ++++++++++++++++++ src/app/reporting/Read-Checked.h | 37 ++++ src/app/reporting/Read-DataModel.cpp | 110 ++++++++++++ src/app/reporting/Read-DataModel.h | 37 ++++ src/app/reporting/Read-Ember.cpp | 55 ++++++ src/app/reporting/Read-Ember.h | 37 ++++ src/app/reporting/Read.h | 47 +++++ src/app/tests/BUILD.gn | 8 + src/app/tests/TestAclAttribute.cpp | 10 ++ src/app/tests/TestReadInteraction.cpp | 3 + src/app/tests/TestReportingEngine.cpp | 15 ++ src/app/tests/test-interaction-model-api.cpp | 116 ++++++++++++- src/app/tests/test-interaction-model-api.h | 39 ++++- src/controller/tests/data_model/BUILD.gn | 2 + .../tests/data_model/DataModelFixtures.cpp | 130 ++++++++++++++ .../tests/data_model/DataModelFixtures.h | 38 +++++ src/controller/tests/data_model/TestRead.cpp | 27 ++- src/lib/shell/MainLoopDefault.cpp | 8 +- 28 files changed, 953 insertions(+), 64 deletions(-) create mode 100644 src/app/reporting/Read-Checked.cpp create mode 100644 src/app/reporting/Read-Checked.h create mode 100644 src/app/reporting/Read-DataModel.cpp create mode 100644 src/app/reporting/Read-DataModel.h create mode 100644 src/app/reporting/Read-Ember.cpp create mode 100644 src/app/reporting/Read-Ember.h create mode 100644 src/app/reporting/Read.h diff --git a/scripts/tests/linux/log_line_processing.py b/scripts/tests/linux/log_line_processing.py index 4c5077944b2fd2..e7624c12d5f2f2 100644 --- a/scripts/tests/linux/log_line_processing.py +++ b/scripts/tests/linux/log_line_processing.py @@ -17,6 +17,7 @@ import select import subprocess import threading +import time from typing import List @@ -57,23 +58,31 @@ def _io_thread(self): reading """ out_wait = select.poll() - out_wait.register(self.process.stdout, select.POLLIN) + out_wait.register(self.process.stdout, select.POLLIN | select.POLLHUP) err_wait = select.poll() - err_wait.register(self.process.stderr, select.POLLIN) + err_wait.register(self.process.stderr, select.POLLIN | select.POLLHUP) with open(self.output_path, "wt") as f: + f.write("PROCESS START: %s\n" % time.ctime()) while not self.done: changes = out_wait.poll(0.1) if changes: out_line = self.process.stdout.readline() + if not out_line: + # stdout closed (otherwise readline should have at least \n) + continue f.write(out_line) self.output_lines.put(out_line) changes = err_wait.poll(0) if changes: err_line = self.process.stderr.readline() + if not err_line: + # stderr closed (otherwise readline should have at least \n) + continue f.write(f"!!STDERR!! : {err_line}") + f.write("PROCESS END: %s\n" % time.ctime()) def __enter__(self): self.done = False diff --git a/scripts/tests/run_tv_casting_test.py b/scripts/tests/run_tv_casting_test.py index 3c963923d9bfc7..7b530c7a4a1780 100755 --- a/scripts/tests/run_tv_casting_test.py +++ b/scripts/tests/run_tv_casting_test.py @@ -295,8 +295,14 @@ def cmd_execute_list(app_path): default=False, help="Enable the commissioner generated passcode test flow.", ) +@click.option( + "--log-directory", + type=str, + default=None, + help="Where to place output logs", +) def test_casting_fn( - tv_app_rel_path, tv_casting_app_rel_path, commissioner_generated_passcode + tv_app_rel_path, tv_casting_app_rel_path, commissioner_generated_passcode, log_directory ): """Test if the casting experience between the Linux tv-casting-app and the Linux tv-app continues to work. @@ -320,10 +326,16 @@ def test_casting_fn( # Store the log files to a temporary directory. with tempfile.TemporaryDirectory() as temp_dir: - linux_tv_app_log_path = os.path.join(temp_dir, LINUX_TV_APP_LOGS) - linux_tv_casting_app_log_path = os.path.join( - temp_dir, LINUX_TV_CASTING_APP_LOGS - ) + if log_directory: + linux_tv_app_log_path = os.path.join(log_directory, LINUX_TV_APP_LOGS) + linux_tv_casting_app_log_path = os.path.join( + log_directory, LINUX_TV_CASTING_APP_LOGS + ) + else: + linux_tv_app_log_path = os.path.join(temp_dir, LINUX_TV_APP_LOGS) + linux_tv_casting_app_log_path = os.path.join( + temp_dir, LINUX_TV_CASTING_APP_LOGS + ) # Get all the test sequences. test_sequences = Sequence.get_test_sequences() diff --git a/src/app/AttributeValueEncoder.h b/src/app/AttributeValueEncoder.h index 5c196f91f2e426..89436789f91a84 100644 --- a/src/app/AttributeValueEncoder.h +++ b/src/app/AttributeValueEncoder.h @@ -162,6 +162,7 @@ class AttributeValueEncoder private: // We made EncodeListItem() private, and ListEncoderHelper will expose it by Encode() friend class ListEncodeHelper; + friend class TestOnlyAttributeValueEncoderAccessor; template CHIP_ERROR EncodeListItem(Ts &&... aArgs) diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index ac9c21b01adfab..8e6ca6dadc569b 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -58,16 +58,6 @@ declare_args() { chip_im_static_global_interaction_model_engine = current_os != "linux" && current_os != "mac" && current_os != "ios" && current_os != "android" - - # Data model interface usage: - # - disabled: does not use data model interface at all - # - check: runs BOTH datamodel and non-data-model (if possible) functionality and compares results - # - enabled: runs only the data model interface (does not use the legacy code) - if (current_os == "linux") { - chip_use_data_model_interface = "check" - } else { - chip_use_data_model_interface = "disabled" - } } buildconfig_header("app_buildconfig") { @@ -219,6 +209,7 @@ static_library("interaction-model") { "WriteClient.h", "reporting/Engine.cpp", "reporting/Engine.h", + "reporting/Read.h", "reporting/ReportScheduler.h", "reporting/ReportSchedulerImpl.cpp", "reporting/ReportSchedulerImpl.h", @@ -260,6 +251,29 @@ static_library("interaction-model") { public_configs = [ "${chip_root}/src:includes" ] + if (chip_use_data_model_interface == "disabled") { + sources += [ + "reporting/Read-Ember.cpp", + "reporting/Read-Ember.h", + ] + } else if (chip_use_data_model_interface == "check") { + sources += [ + "reporting/Read-Checked.cpp", + "reporting/Read-Checked.h", + "reporting/Read-DataModel.cpp", + "reporting/Read-DataModel.h", + "reporting/Read-Ember.cpp", + "reporting/Read-Ember.h", + ] + public_deps += [ "${chip_root}/src/app/data-model-interface" ] + } else { # enabled + sources += [ + "reporting/Read-DataModel.cpp", + "reporting/Read-DataModel.h", + ] + public_deps += [ "${chip_root}/src/app/data-model-interface" ] + } + if (chip_enable_read_client) { sources += [ "ReadClient.cpp" ] } diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 4d7a01f650dba2..dcdbd3f10d2f85 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -93,6 +93,15 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM StatusIB::RegisterErrorFormatter(); +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + ChipLogError(InteractionModel, "WARNING ┌────────────────────────────────────────────────────"); + ChipLogError(InteractionModel, "WARNING │ Interaction Model Engine running in 'Checked' mode."); + ChipLogError(InteractionModel, "WARNING │ This executes BOTH ember and data-model code paths."); + ChipLogError(InteractionModel, "WARNING │ which is inefficient and consumes more flash space."); + ChipLogError(InteractionModel, "WARNING │ This should be done for testing only."); + ChipLogError(InteractionModel, "WARNING └────────────────────────────────────────────────────"); +#endif + return CHIP_NO_ERROR; } @@ -1697,6 +1706,16 @@ Protocols::InteractionModel::Status InteractionModelEngine::CommandExists(const return ServerClusterCommandExists(aCommandPath); } +InteractionModel::DataModel * InteractionModelEngine::SetDataModel(InteractionModel::DataModel * model) +{ + // Alternting data model should not be done while IM is actively handling requests. + VerifyOrDie(mReadHandlers.begin() == mReadHandlers.end()); + + InteractionModel::DataModel * oldModel = GetDataModel(); + mDataModel = model; + return oldModel; +} + InteractionModel::DataModel * InteractionModelEngine::GetDataModel() const { #if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 2d143f14be50a1..9d420f7f654176 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -125,10 +125,6 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, * @param[in] apFabricTable A pointer to the FabricTable object. * @param[in] apCASESessionMgr An optional pointer to a CASESessionManager (used for re-subscriptions). * - * @retval #CHIP_ERROR_INCORRECT_STATE If the state is not equal to - * kState_NotInitialized. - * @retval #CHIP_NO_ERROR On success. - * */ CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable, reporting::ReportScheduler * reportScheduler, CASESessionManager * apCASESessionMgr = nullptr, @@ -408,6 +404,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, InteractionModel::DataModel * GetDataModel() const; + // MUST NOT be used while the interaction model engine is running as interaction + // model functionality (e.g. active reads/writes/subscriptions) rely on data model + // state + // + // Returns the old data model value. + InteractionModel::DataModel * SetDataModel(InteractionModel::DataModel * model); + private: friend class reporting::Engine; friend class TestCommandInteraction; diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni index 91057654fdcccf..be1149b2b67eb5 100644 --- a/src/app/common_flags.gni +++ b/src/app/common_flags.gni @@ -21,4 +21,14 @@ declare_args() { # Flag that controls whether the time-to-wait from BUSY responses is # communicated to OperationalSessionSetup API consumers. chip_enable_busy_handling_for_operational_session_setup = true + + # Data model interface usage: + # - disabled: does not use data model interface at all + # - check: runs BOTH datamodel and non-data-model (if possible) functionality and compares results + # - enabled: runs only the data model interface (does not use the legacy code) + if (current_os == "linux") { + chip_use_data_model_interface = "check" + } else { + chip_use_data_model_interface = "disabled" + } } diff --git a/src/app/data-model-interface/OperationTypes.h b/src/app/data-model-interface/OperationTypes.h index d19621db71d26a..c9e5ae642158b3 100644 --- a/src/app/data-model-interface/OperationTypes.h +++ b/src/app/data-model-interface/OperationTypes.h @@ -65,7 +65,6 @@ enum class ReadFlags : uint32_t struct ReadAttributeRequest : OperationRequest { ConcreteAttributePath path; - std::optional dataVersion; BitFlags readFlags; }; diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index dfa64cf3b8f9de..50e23498aeeadb 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -79,25 +80,6 @@ bool Engine::IsClusterDataVersionMatch(const SingleLinkedListNode Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", aPath.mClusterId, - aPath.mAttributeId); - - DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, - DataModelCallbacks::OperationOrder::Pre, aPath); - - ReturnErrorOnFailure(ReadSingleClusterData(aSubjectDescriptor, aIsFabricFiltered, aPath, aAttributeReportIBs, aEncoderState)); - - DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, - DataModelCallbacks::OperationOrder::Post, aPath); - - return CHIP_NO_ERROR; -} - static bool IsOutOfWriterSpaceError(CHIP_ERROR err) { return err == CHIP_ERROR_NO_MEMORY || err == CHIP_ERROR_BUFFER_TOO_SMALL; @@ -200,8 +182,8 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu ConcreteReadAttributePath pathForRetrieval(readPath); // Load the saved state from previous encoding session for chunking of one single attribute (list chunking). AttributeEncodeState encodeState = apReadHandler->GetAttributeEncodeState(); - err = RetrieveClusterData(apReadHandler->GetSubjectDescriptor(), apReadHandler->IsFabricFiltered(), attributeReportIBs, - pathForRetrieval, &encodeState); + err = Impl::RetrieveClusterData(mpImEngine->GetDataModel(), apReadHandler->GetSubjectDescriptor(), + apReadHandler->IsFabricFiltered(), attributeReportIBs, pathForRetrieval, &encodeState); if (err != CHIP_NO_ERROR) { // If error is not an "out of writer space" error, rollback and encode status. diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index 0bf7a9f83de1ae..070db9947b5f1c 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -168,9 +168,6 @@ class Engine bool * apHasMoreChunks, bool * apHasEncodedData); CHIP_ERROR BuildSingleReportDataEventReports(ReportDataMessage::Builder & reportDataBuilder, ReadHandler * apReadHandler, bool aBufferIsUsed, bool * apHasMoreChunks, bool * apHasEncodedData); - CHIP_ERROR RetrieveClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered, - AttributeReportIBs::Builder & aAttributeReportIBs, - const ConcreteReadAttributePath & aClusterInfo, AttributeEncodeState * apEncoderState); CHIP_ERROR CheckAccessDeniedEventPaths(TLV::TLVWriter & aWriter, bool & aHasEncodedData, ReadHandler * apReadHandler); // If version match, it means don't send, if version mismatch, it means send. diff --git a/src/app/reporting/Read-Checked.cpp b/src/app/reporting/Read-Checked.cpp new file mode 100644 index 00000000000000..76a6d1378eb653 --- /dev/null +++ b/src/app/reporting/Read-Checked.cpp @@ -0,0 +1,160 @@ +/* + * 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. + */ +#include + +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace CheckedImpl { +namespace { + +/// Checkpoints and saves the state (including error state) for a +/// AttributeReportIBs::Builder +class ScopedAttributeReportIBsBuilderState +{ +public: + ScopedAttributeReportIBsBuilderState(AttributeReportIBs::Builder & builder) : mBuilder(builder), mError(mBuilder.GetError()) + { + mBuilder.Checkpoint(mCheckpoint); + } + + ~ScopedAttributeReportIBsBuilderState() + { + mBuilder.Rollback(mCheckpoint); + mBuilder.ResetError(mError); + } + +private: + AttributeReportIBs::Builder & mBuilder; + chip::TLV::TLVWriter mCheckpoint; + CHIP_ERROR mError; +}; + +} // namespace + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState) +{ + ChipLogDetail(DataManagement, " Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", path.mClusterId, + path.mAttributeId); + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Pre, path); + + CHIP_ERROR errEmber = CHIP_NO_ERROR; + uint32_t lengthWrittenEmber = 0; + + // a copy for DM logic only. Ember changes state directly + // IMPORTANT: the copy MUST be taken BEFORE ember processes/changes encoderState inline. + AttributeEncodeState stateDm(encoderState); + + { + ScopedAttributeReportIBsBuilderState builderState(reportBuilder); // temporary only + errEmber = + EmberImpl::RetrieveClusterData(dataModel, subjectDescriptor, isFabricFiltered, reportBuilder, path, encoderState); + lengthWrittenEmber = reportBuilder.GetWriter()->GetLengthWritten(); + } + + CHIP_ERROR errDM = DataModelImpl::RetrieveClusterData(dataModel, subjectDescriptor, isFabricFiltered, reportBuilder, path, + encoderState != nullptr ? &stateDm : nullptr); + + if (errEmber != errDM) + { + // Note log + chipDie instead of VerifyOrDie so that breakpoints (and usage of rr) + // is easier to debug. + ChipLogError(Test, "Different return codes between ember and DM"); + ChipLogError(Test, " Ember error: %" CHIP_ERROR_FORMAT, errEmber.Format()); + ChipLogError(Test, " DM error: %" CHIP_ERROR_FORMAT, errDM.Format()); + + // For time-dependent data, we may have size differences here: one data fitting in buffer + // while another not, resulting in different errors (success vs out of space). + // + // Make unit tests strict; otherwise allow it with potentially odd mismatch errors + // (in which case logs will be odd, however we also expect Checked versions to only + // run for a short period until we switch over to either ember or DM completely). +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + chipDie(); +#endif + } + + // data should be identical for most cases EXCEPT that for time-deltas (e.g. seconds since boot or similar) + // it may actually differ. As a result, the amount of data written in bytes MUST be the same, however if the rest of the + // data is not the same, we just print it out as a warning for manual inspection + // + // We have no direct access to TLV buffer data (especially given backing store splits) + // so for now we check that data length was identical. + // + // NOTE: RetrieveClusterData is responsible for encoding StatusIB errors in case of failures + // so we validate length written requirements for BOTH success and failure. + // + // NOTE: data length is NOT reliable if the data content differs in encoding length. E.g. numbers changing + // from 0xFF to 0x100 or similar will use up more space. + // For unit tests we make the validation strict, however for runtime we just report an + // error for different sizes. + if (lengthWrittenEmber != reportBuilder.GetWriter()->GetLengthWritten()) + { + ChipLogError(Test, "Different written length: %" PRIu32 " (Ember) vs %" PRIu32 " (DataModel)", lengthWrittenEmber, + reportBuilder.GetWriter()->GetLengthWritten()); +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + chipDie(); +#endif + } + + // For chunked reads, the encoder state MUST be identical (since this is what controls + // where chunking resumes). + if ((errEmber == CHIP_ERROR_NO_MEMORY) || (errEmber == CHIP_ERROR_BUFFER_TOO_SMALL)) + { + // Encoder state MUST match on partial reads (used by chunking) + // specifically ReadViaAccessInterface in ember-compatibility-functions only + // sets the encoder state in case an error occurs. + if (encoderState != nullptr) + { + if (encoderState->AllowPartialData() != stateDm.AllowPartialData()) + { + ChipLogError(Test, "Different partial data"); + // NOTE: die on unit tests only, since partial data size may differ across + // time-dependent data (very rarely because fast code, but still possible) +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + chipDie(); +#endif + } + if (encoderState->CurrentEncodingListIndex() != stateDm.CurrentEncodingListIndex()) + { + ChipLogError(Test, "Different partial data"); + // NOTE: die on unit tests only, since partial data size may differ across + // time-dependent data (very rarely because fast code, but still possible) +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + chipDie(); +#endif + } + } + } + + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Post, path); + + return errDM; +} + +} // namespace CheckedImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read-Checked.h b/src/app/reporting/Read-Checked.h new file mode 100644 index 00000000000000..6df9715fcc3da9 --- /dev/null +++ b/src/app/reporting/Read-Checked.h @@ -0,0 +1,37 @@ +/* + * 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. + */ +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace CheckedImpl { + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState); + +} // namespace CheckedImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read-DataModel.cpp b/src/app/reporting/Read-DataModel.cpp new file mode 100644 index 00000000000000..c004853cdaa495 --- /dev/null +++ b/src/app/reporting/Read-DataModel.cpp @@ -0,0 +1,110 @@ +/* + * 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. + */ +#include + +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace DataModelImpl { +namespace { + +bool IsOutOfSpaceError(CHIP_ERROR err) +{ + return (err == CHIP_ERROR_NO_MEMORY || err == CHIP_ERROR_BUFFER_TOO_SMALL); +} + +} // namespace + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState) +{ + // Odd ifdef is to only do this if the `Read-Check` does not do it already. +#if !CHIP_CONFIG_USE_EMBER_DATA_MODEL + ChipLogDetail(DataManagement, " Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", path.mClusterId, + path.mAttributeId); + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Pre, path); +#endif // !CHIP_CONFIG_USE_EMBER_DATA_MODEL + + InteractionModel::ReadAttributeRequest readRequest; + + if (isFabricFiltered) + { + readRequest.readFlags.Set(InteractionModel::ReadFlags::kFabricFiltered); + } + readRequest.subjectDescriptor = subjectDescriptor; + readRequest.path = path; + + DataVersion version = 0; + if (std::optional clusterInfo = dataModel->GetClusterInfo(path); clusterInfo.has_value()) + { + version = clusterInfo->dataVersion; + } + else + { + ChipLogError(DataManagement, "Read request on unknown cluster - no data version available"); + } + + TLV::TLVWriter checkpoint; + reportBuilder.Checkpoint(checkpoint); + + AttributeValueEncoder attributeValueEncoder(reportBuilder, subjectDescriptor, path, version, isFabricFiltered, encoderState); + CHIP_ERROR err = dataModel->ReadAttribute(readRequest, attributeValueEncoder); + + if (err == CHIP_NO_ERROR) + { + // Odd ifdef is to only do this if the `Read-Check` does not do it already. +#if !CHIP_CONFIG_USE_EMBER_DATA_MODEL + // TODO: this callback being only executed on success is awkward. The Write callback is always done + // for both read and write. + // + // For now this preserves existing/previous code logic, however we should consider to ALWAYS + // call this. + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Post, path); +#endif // !CHIP_CONFIG_USE_EMBER_DATA_MODEL + return CHIP_NO_ERROR; + } + + // Encoder state is relevant for errors in case they are retryable. + // + // Generally only IsOutOfSpaceError(err) would be retryable, however we save the state + // for all errors in case this is information that is useful (retry or error position). + if (encoderState != nullptr) + { + *encoderState = attributeValueEncoder.GetState(); + } + + // Out of space errors may be chunked data, reporting those cases would be very confusing + // as they are not fully errors. Report only others (which presumably are not recoverable + // and will be sent to the client as well). + if (!IsOutOfSpaceError(err)) + { + ChipLogError(DataManagement, "Failed to read attribute: %" CHIP_ERROR_FORMAT, err.Format()); + } + return err; +} + +} // namespace DataModelImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read-DataModel.h b/src/app/reporting/Read-DataModel.h new file mode 100644 index 00000000000000..231429c4a2e242 --- /dev/null +++ b/src/app/reporting/Read-DataModel.h @@ -0,0 +1,37 @@ +/* + * 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. + */ +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace DataModelImpl { + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState); + +} // namespace DataModelImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read-Ember.cpp b/src/app/reporting/Read-Ember.cpp new file mode 100644 index 00000000000000..290c4e627ac7bb --- /dev/null +++ b/src/app/reporting/Read-Ember.cpp @@ -0,0 +1,55 @@ +/* + * 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. + */ +#include + +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace EmberImpl { + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState) +{ + // Odd ifdef is to only do this if the `Read-Check` does not do it already. +#if !CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + ChipLogDetail(DataManagement, " Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", path.mClusterId, + path.mAttributeId); + + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Pre, path); +#endif // !CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + + ReturnErrorOnFailure(ReadSingleClusterData(subjectDescriptor, isFabricFiltered, path, reportBuilder, encoderState)); + + // Odd ifdef is to only do this if the `Read-Check` does not do it already. +#if !CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read, + DataModelCallbacks::OperationOrder::Post, path); +#endif // !CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + + return CHIP_NO_ERROR; +} + +} // namespace EmberImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read-Ember.h b/src/app/reporting/Read-Ember.h new file mode 100644 index 00000000000000..0181734d205053 --- /dev/null +++ b/src/app/reporting/Read-Ember.h @@ -0,0 +1,37 @@ +/* + * 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. + */ +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { +namespace app { +namespace reporting { +namespace EmberImpl { + +CHIP_ERROR RetrieveClusterData(InteractionModel::DataModel * dataModel, const Access::SubjectDescriptor & subjectDescriptor, + bool isFabricFiltered, AttributeReportIBs::Builder & reportBuilder, + const ConcreteReadAttributePath & path, AttributeEncodeState * encoderState); + +} // namespace EmberImpl +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/Read.h b/src/app/reporting/Read.h new file mode 100644 index 00000000000000..c568c5356ab472 --- /dev/null +++ b/src/app/reporting/Read.h @@ -0,0 +1,47 @@ +/* + * 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. + */ +#pragma once + +#include + +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +#include +#else +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +#include +#else +#include +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + +namespace chip { +namespace app { +namespace reporting { + +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +namespace Impl = CheckedImpl; +#else +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +namespace Impl = DataModelImpl; +#else +namespace Impl = EmberImpl; +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 0f0694dcc51574..59d8f2c325fd8c 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -143,9 +143,17 @@ source_set("app-test-stubs") { sources = [ "test-ember-api.cpp", "test-ember-api.h", + + # The overrides in these files are overrides from ember-compatibility-functions + # and the data model interface is NOT aware of such functionality + # + # TODO: ideally tests should have been written via mock ember, however mock ember did + # not exist at that time. We should completely re-write how these tests access + # the data via the DataModel interface "test-interaction-model-api.cpp", "test-interaction-model-api.h", ] + public_configs = [ "${chip_root}/src/lib/support/pw_log_chip:config" ] public_deps = [ diff --git a/src/app/tests/TestAclAttribute.cpp b/src/app/tests/TestAclAttribute.cpp index fac82e484d7450..c11adde08d6208 100644 --- a/src/app/tests/TestAclAttribute.cpp +++ b/src/app/tests/TestAclAttribute.cpp @@ -118,7 +118,17 @@ class TestAclAttribute : public Test::AppContext Access::GetAccessControl().Finish(); Access::GetAccessControl().Init(GetTestAccessControlDelegate(), gDeviceTypeResolver); + mOldModel = InteractionModelEngine::GetInstance()->SetDataModel(&TestImCustomDataModel::Instance()); } + + void TearDown() override + { + AppContext::TearDown(); + InteractionModelEngine::GetInstance()->SetDataModel(mOldModel); + } + +private: + chip::app::InteractionModel::DataModel * mOldModel = nullptr; }; // Read Client sends a malformed subscribe request, interaction model engine fails to parse the request and generates a status diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 57d23d1eb0db81..5c712b7c0a9a4a 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -272,9 +272,11 @@ class TestReadInteraction : public chip::Test::AppContext ASSERT_EQ(mEventCounter.Init(0), CHIP_NO_ERROR); chip::app::EventManagement::CreateEventManagement(&GetExchangeManager(), ArraySize(logStorageResources), gCircularEventBuffer, logStorageResources, &mEventCounter); + mOldModel = InteractionModelEngine::GetInstance()->SetDataModel(&TestImCustomDataModel::Instance()); } void TearDown() { + InteractionModelEngine::GetInstance()->SetDataModel(mOldModel); chip::app::EventManagement::DestroyEventManagement(); AppContext::TearDown(); } @@ -329,6 +331,7 @@ class TestReadInteraction : public chip::Test::AppContext protected: chip::MonotonicallyIncreasingCounter mEventCounter; static bool sSyncScheduler; + chip::app::InteractionModel::DataModel * mOldModel = nullptr; }; bool TestReadInteraction::sSyncScheduler = false; diff --git a/src/app/tests/TestReportingEngine.cpp b/src/app/tests/TestReportingEngine.cpp index 4468eeaaf12ade..c18fbdbf1f2dcc 100644 --- a/src/app/tests/TestReportingEngine.cpp +++ b/src/app/tests/TestReportingEngine.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -55,6 +56,18 @@ namespace reporting { class TestReportingEngine : public chip::Test::AppContext { public: + void SetUp() override + { + chip::Test::AppContext::SetUp(); + mOldModel = InteractionModelEngine::GetInstance()->SetDataModel(&TestImCustomDataModel::Instance()); + } + + void TearDown() override + { + InteractionModelEngine::GetInstance()->SetDataModel(mOldModel); + chip::Test::AppContext::TearDown(); + } + template static bool VerifyDirtySetContent(const Args &... args); static bool InsertToDirtySet(const AttributePathParams & aPath); @@ -64,6 +77,8 @@ class TestReportingEngine : public chip::Test::AppContext void TestMergeAttributePathWhenDirtySetPoolExhausted(); private: + chip::app::InteractionModel::DataModel * mOldModel = nullptr; + struct ExpectedDirtySetContent : public AttributePathParams { ExpectedDirtySetContent(const AttributePathParams & path) : AttributePathParams(path) {} diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp index ae3f559424f97c..444516a79ddf1a 100644 --- a/src/app/tests/test-interaction-model-api.cpp +++ b/src/app/tests/test-interaction-model-api.cpp @@ -13,25 +13,38 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -#include "lib/support/CHIPMem.h" #include -#include #include #include +#include #include #include #include #include #include +using namespace chip::app::InteractionModel; + namespace chip { uint8_t Test::attributeDataTLV[CHIP_CONFIG_DEFAULT_UDP_MTU_SIZE]; size_t Test::attributeDataTLVLen = 0; namespace app { +class TestOnlyAttributeValueEncoderAccessor +{ +public: + TestOnlyAttributeValueEncoderAccessor(AttributeValueEncoder & encoder) : mEncoder(encoder) {} + + AttributeReportIBs::Builder & Builder() { return mEncoder.mAttributeReportIBsBuilder; } + + void SetState(const AttributeEncodeState & state) { mEncoder.mEncodeState = state; } + +private: + AttributeValueEncoder & mEncoder; +}; + // Used by the code in TestWriteInteraction.cpp (and generally tests that interact with the WriteHandler may need this). const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath) { @@ -134,6 +147,101 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr return AttributeValueEncoder(aAttributeReports, aSubjectDescriptor, aPath, 0 /* dataVersion */).Encode(Test::kTestFieldValue1); } -} // namespace app +TestImCustomDataModel & TestImCustomDataModel::Instance() +{ + static TestImCustomDataModel model; + return model; +} + +CHIP_ERROR TestImCustomDataModel::ReadAttribute(const ReadAttributeRequest & request, AttributeValueEncoder & encoder) +{ + AttributeEncodeState mutableState(&encoder.GetState()); // provide a state copy to start. + + CHIP_ERROR err = ReadSingleClusterData(request.subjectDescriptor.value_or(Access::SubjectDescriptor()), + request.readFlags.Has(ReadFlags::kFabricFiltered), request.path, + TestOnlyAttributeValueEncoderAccessor(encoder).Builder(), &mutableState); + + // state must survive CHIP_ERRORs as it is used for chunking + TestOnlyAttributeValueEncoderAccessor(encoder).SetState(mutableState); + + return err; +} + +CHIP_ERROR TestImCustomDataModel::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR TestImCustomDataModel::Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +EndpointId TestImCustomDataModel::FirstEndpoint() +{ + return CodegenDataModelInstance()->FirstEndpoint(); +} + +EndpointId TestImCustomDataModel::NextEndpoint(EndpointId before) +{ + return CodegenDataModelInstance()->NextEndpoint(before); +} + +ClusterEntry TestImCustomDataModel::FirstCluster(EndpointId endpoint) +{ + return CodegenDataModelInstance()->FirstCluster(endpoint); +} +ClusterEntry TestImCustomDataModel::NextCluster(const ConcreteClusterPath & before) +{ + return CodegenDataModelInstance()->NextCluster(before); +} + +std::optional TestImCustomDataModel::GetClusterInfo(const ConcreteClusterPath & path) +{ + return CodegenDataModelInstance()->GetClusterInfo(path); +} + +AttributeEntry TestImCustomDataModel::FirstAttribute(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstAttribute(cluster); +} + +AttributeEntry TestImCustomDataModel::NextAttribute(const ConcreteAttributePath & before) +{ + return CodegenDataModelInstance()->NextAttribute(before); +} + +std::optional TestImCustomDataModel::GetAttributeInfo(const ConcreteAttributePath & path) +{ + return CodegenDataModelInstance()->GetAttributeInfo(path); +} + +CommandEntry TestImCustomDataModel::FirstAcceptedCommand(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstAcceptedCommand(cluster); +} + +CommandEntry TestImCustomDataModel::NextAcceptedCommand(const ConcreteCommandPath & before) +{ + return CodegenDataModelInstance()->NextAcceptedCommand(before); +} + +std::optional TestImCustomDataModel::GetAcceptedCommandInfo(const ConcreteCommandPath & path) +{ + return CodegenDataModelInstance()->GetAcceptedCommandInfo(path); +} + +ConcreteCommandPath TestImCustomDataModel::FirstGeneratedCommand(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstGeneratedCommand(cluster); +} + +ConcreteCommandPath TestImCustomDataModel::NextGeneratedCommand(const ConcreteCommandPath & before) +{ + return CodegenDataModelInstance()->NextGeneratedCommand(before); +} + +} // namespace app } // namespace chip diff --git a/src/app/tests/test-interaction-model-api.h b/src/app/tests/test-interaction-model-api.h index a4f91add7f42f8..e88fee62ec1d23 100644 --- a/src/app/tests/test-interaction-model-api.h +++ b/src/app/tests/test-interaction-model-api.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -64,7 +65,6 @@ } namespace chip { - namespace Test { constexpr chip::ClusterId kTestDeniedClusterId1 = 1000; @@ -101,5 +101,42 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPat bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint); +/// A customized class for read/write/invoke that matches functionality +/// with the ember-compatibility-functions functionality here. +/// +/// TODO: these functions currently redirect to ember functions, so could +/// be merged with DataModelFixtures.h/cpp as well. This is not done since +/// if we remove the direct ember dependency from IM, we can implement +/// distinct functional classes. +/// TODO items for above: +/// - once IM only supports DataModel +/// - break ember-overrides in this h/cpp file +class TestImCustomDataModel : public InteractionModel::DataModel +{ +public: + static TestImCustomDataModel & Instance(); + + CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } + + CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, AttributeValueEncoder & encoder) override; + CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; + CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override; + + EndpointId FirstEndpoint() override; + EndpointId NextEndpoint(EndpointId before) override; + InteractionModel::ClusterEntry FirstCluster(EndpointId endpoint) override; + InteractionModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; + std::optional GetClusterInfo(const ConcreteClusterPath & path) override; + InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; + InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; + std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; + InteractionModel::CommandEntry FirstAcceptedCommand(const ConcreteClusterPath & cluster) override; + InteractionModel::CommandEntry NextAcceptedCommand(const ConcreteCommandPath & before) override; + std::optional GetAcceptedCommandInfo(const ConcreteCommandPath & path) override; + ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override; + ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override; +}; + } // namespace app } // namespace chip diff --git a/src/controller/tests/data_model/BUILD.gn b/src/controller/tests/data_model/BUILD.gn index 9439bd33b83458..692696d3be2548 100644 --- a/src/controller/tests/data_model/BUILD.gn +++ b/src/controller/tests/data_model/BUILD.gn @@ -17,6 +17,7 @@ import("//build_overrides/chip.gni") import("//build_overrides/pigweed.gni") import("${chip_root}/build/chip/chip_test_suite.gni") +import("${chip_root}/src/app/common_flags.gni") import("${chip_root}/src/platform/device.gni") chip_test_suite("data_model") { @@ -40,6 +41,7 @@ chip_test_suite("data_model") { public_deps = [ "${chip_root}/src/app", "${chip_root}/src/app/common:cluster-objects", + "${chip_root}/src/app/data-model-interface", "${chip_root}/src/app/tests:helpers", "${chip_root}/src/app/util/mock:mock_codegen_data_model", "${chip_root}/src/app/util/mock:mock_ember", diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp index 2e5d7e65f7ee42..eb043acd04353d 100644 --- a/src/controller/tests/data_model/DataModelFixtures.cpp +++ b/src/controller/tests/data_model/DataModelFixtures.cpp @@ -21,11 +21,14 @@ #include #include #include +#include #include +#include using namespace chip; using namespace chip::app; using namespace chip::app::DataModelTests; +using namespace chip::app::InteractionModel; using namespace chip::app::Clusters; using namespace chip::app::Clusters::UnitTesting; using namespace chip::Protocols; @@ -33,6 +36,19 @@ using namespace chip::Protocols; namespace chip { namespace app { +class TestOnlyAttributeValueEncoderAccessor +{ +public: + TestOnlyAttributeValueEncoderAccessor(AttributeValueEncoder & encoder) : mEncoder(encoder) {} + + AttributeReportIBs::Builder & Builder() { return mEncoder.mAttributeReportIBsBuilder; } + + void SetState(const AttributeEncodeState & state) { mEncoder.mEncodeState = state; } + +private: + AttributeValueEncoder & mEncoder; +}; + namespace DataModelTests { ScopedChangeOnly gReadResponseDirective(ReadResponseDirective::kSendDataResponse); @@ -41,6 +57,11 @@ ScopedChangeOnly gCommandResponseDirective(CommandResp ScopedChangeOnly gIsLitIcd(false); +// TODO: usage of a global value that changes as a READ sideffect is problematic for +// dual-read use cases (i.e. during checked ember/datamodel tests) +// +// For now see the hack "change undo" in CustomDataModel::ReadAttribute, however +// overall this is problematic. uint16_t gInt16uTotalReadCount = 0; CommandHandler::Handle gAsyncCommandHandle; @@ -464,5 +485,114 @@ Protocols::InteractionModel::Status ServerClusterCommandExists(const ConcreteCom return Status::Success; } +CustomDataModel & CustomDataModel::Instance() +{ + static CustomDataModel model; + return model; +} + +CHIP_ERROR CustomDataModel::ReadAttribute(const ReadAttributeRequest & request, AttributeValueEncoder & encoder) +{ + AttributeEncodeState mutableState(&encoder.GetState()); // provide a state copy to start. + +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + if ((request.path.mEndpointId < chip::Test::kMockEndpointMin) && + (gReadResponseDirective == ReadResponseDirective::kSendDataResponse) && + (request.path.mClusterId == app::Clusters::UnitTesting::Id) && + (request.path.mAttributeId == app::Clusters::UnitTesting::Attributes::Int16u::Id)) + { + // gInt16uTotalReadCount is a global that keeps changing. Further more, encoding + // size differs when moving from 0xFF to 0x100, so encoding sizes in TLV differ. + // + // This is a HACKISH workaround as it relies that we ember-read before datamodel-read + gInt16uTotalReadCount--; + } +#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + + CHIP_ERROR err = ReadSingleClusterData(request.subjectDescriptor.value_or(Access::SubjectDescriptor()), + request.readFlags.Has(ReadFlags::kFabricFiltered), request.path, + TestOnlyAttributeValueEncoderAccessor(encoder).Builder(), &mutableState); + + // state must survive CHIP_ERRORs as it is used for chunking + TestOnlyAttributeValueEncoderAccessor(encoder).SetState(mutableState); + + return err; +} + +CHIP_ERROR CustomDataModel::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR CustomDataModel::Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) +{ + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +EndpointId CustomDataModel::FirstEndpoint() +{ + return CodegenDataModelInstance()->FirstEndpoint(); +} + +EndpointId CustomDataModel::NextEndpoint(EndpointId before) +{ + return CodegenDataModelInstance()->NextEndpoint(before); +} + +ClusterEntry CustomDataModel::FirstCluster(EndpointId endpoint) +{ + return CodegenDataModelInstance()->FirstCluster(endpoint); +} + +ClusterEntry CustomDataModel::NextCluster(const ConcreteClusterPath & before) +{ + return CodegenDataModelInstance()->NextCluster(before); +} + +std::optional CustomDataModel::GetClusterInfo(const ConcreteClusterPath & path) +{ + return CodegenDataModelInstance()->GetClusterInfo(path); +} + +AttributeEntry CustomDataModel::FirstAttribute(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstAttribute(cluster); +} + +AttributeEntry CustomDataModel::NextAttribute(const ConcreteAttributePath & before) +{ + return CodegenDataModelInstance()->NextAttribute(before); +} + +std::optional CustomDataModel::GetAttributeInfo(const ConcreteAttributePath & path) +{ + return CodegenDataModelInstance()->GetAttributeInfo(path); +} + +CommandEntry CustomDataModel::FirstAcceptedCommand(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstAcceptedCommand(cluster); +} + +CommandEntry CustomDataModel::NextAcceptedCommand(const ConcreteCommandPath & before) +{ + return CodegenDataModelInstance()->NextAcceptedCommand(before); +} + +std::optional CustomDataModel::GetAcceptedCommandInfo(const ConcreteCommandPath & path) +{ + return CodegenDataModelInstance()->GetAcceptedCommandInfo(path); +} + +ConcreteCommandPath CustomDataModel::FirstGeneratedCommand(const ConcreteClusterPath & cluster) +{ + return CodegenDataModelInstance()->FirstGeneratedCommand(cluster); +} + +ConcreteCommandPath CustomDataModel::NextGeneratedCommand(const ConcreteCommandPath & before) +{ + return CodegenDataModelInstance()->NextGeneratedCommand(before); +} + } // namespace app } // namespace chip diff --git a/src/controller/tests/data_model/DataModelFixtures.h b/src/controller/tests/data_model/DataModelFixtures.h index dfdc60e5b59b06..405b4ff4d4e7d8 100644 --- a/src/controller/tests/data_model/DataModelFixtures.h +++ b/src/controller/tests/data_model/DataModelFixtures.h @@ -22,6 +22,7 @@ #pragma once #include +#include #include #include #include @@ -97,6 +98,43 @@ extern ScopedChangeOnly gCommandResponseDirective; // Populated with the command handle when gCommandResponseDirective == kAsync extern CommandHandler::Handle gAsyncCommandHandle; +/// A customized class for read/write/invoke that matches functionality +/// with the ember-compatibility-functions functionality here. +/// +/// TODO: these functions currently redirect to ember functions, so could +/// be merged with test-interaction-model-api.h/cpp as well. This is not done since +/// if we remove the direct ember dependency from IM, we can implement +/// distinct functional classes. +/// TODO items for above: +/// - once IM only supports DataModel +/// - break ember-overrides in this h/cpp file +class CustomDataModel : public InteractionModel::DataModel +{ +public: + static CustomDataModel & Instance(); + + CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } + + CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, AttributeValueEncoder & encoder) override; + CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; + CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override; + + EndpointId FirstEndpoint() override; + EndpointId NextEndpoint(EndpointId before) override; + InteractionModel::ClusterEntry FirstCluster(EndpointId endpoint) override; + InteractionModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; + std::optional GetClusterInfo(const ConcreteClusterPath & path) override; + InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; + InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; + std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; + InteractionModel::CommandEntry FirstAcceptedCommand(const ConcreteClusterPath & cluster) override; + InteractionModel::CommandEntry NextAcceptedCommand(const ConcreteCommandPath & before) override; + std::optional GetAcceptedCommandInfo(const ConcreteCommandPath & path) override; + ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override; + ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override; +}; + } // namespace DataModelTests } // namespace app } // namespace chip diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 9df501c45e7d78..1281d9190c56bc 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -52,7 +52,21 @@ class TestRead : public chip::Test::AppContext, public app::ReadHandler::Applica protected: static uint16_t mMaxInterval; - CHIP_ERROR OnSubscriptionRequested(app::ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) + // Performs setup for each individual test in the test suite + void SetUp() override + { + chip::Test::AppContext::SetUp(); + mOldModel = InteractionModelEngine::GetInstance()->SetDataModel(&CustomDataModel::Instance()); + } + + // Performs teardown for each individual test in the test suite + void TearDown() override + { + InteractionModelEngine::GetInstance()->SetDataModel(mOldModel); + chip::Test::AppContext::TearDown(); + } + + CHIP_ERROR OnSubscriptionRequested(app::ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) override { VerifyOrReturnError(!mEmitSubscriptionError, CHIP_ERROR_INVALID_ARGUMENT); @@ -63,9 +77,9 @@ class TestRead : public chip::Test::AppContext, public app::ReadHandler::Applica return CHIP_NO_ERROR; } - void OnSubscriptionEstablished(app::ReadHandler & aReadHandler) { mNumActiveSubscriptions++; } + void OnSubscriptionEstablished(app::ReadHandler & aReadHandler) override { mNumActiveSubscriptions++; } - void OnSubscriptionTerminated(app::ReadHandler & aReadHandler) { mNumActiveSubscriptions--; } + void OnSubscriptionTerminated(app::ReadHandler & aReadHandler) override { mNumActiveSubscriptions--; } // Issue the given number of reads in parallel and wait for them all to // succeed. @@ -83,9 +97,10 @@ class TestRead : public chip::Test::AppContext, public app::ReadHandler::Applica // max-interval to time out. static System::Clock::Timeout ComputeSubscriptionTimeout(System::Clock::Seconds16 aMaxInterval); - bool mEmitSubscriptionError = false; - int32_t mNumActiveSubscriptions = 0; - bool mAlterSubscriptionIntervals = false; + bool mEmitSubscriptionError = false; + int32_t mNumActiveSubscriptions = 0; + bool mAlterSubscriptionIntervals = false; + chip::app::InteractionModel::DataModel * mOldModel = nullptr; }; uint16_t TestRead::mMaxInterval = 66; diff --git a/src/lib/shell/MainLoopDefault.cpp b/src/lib/shell/MainLoopDefault.cpp index c83fc96e4a1413..dba76397927f2c 100644 --- a/src/lib/shell/MainLoopDefault.cpp +++ b/src/lib/shell/MainLoopDefault.cpp @@ -168,13 +168,7 @@ void ProcessShellLine(intptr_t args) if (retval != CHIP_NO_ERROR) { - char errorStr[160]; - bool errorStrFound = FormatCHIPError(errorStr, sizeof(errorStr), retval); - if (!errorStrFound) - { - errorStr[0] = 0; - } - streamer_printf(streamer_get(), "Error %s: %s\r\n", argv[0], errorStr); + streamer_printf(streamer_get(), "Error %s: %" CHIP_ERROR_FORMAT "\r\n", argv[0], retval.Format()); } else { From 6a98248d5e1083bb52ac801ab2469e2d8e8cc7bd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 24 Jul 2024 19:26:28 -0400 Subject: [PATCH 04/10] Fix global scope for data provider in tv casting app (#34483) * Fix global scope for data provider * Restyle --------- Co-authored-by: Andrei Litvin --- examples/tv-casting-app/linux/simple-app-helper.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/examples/tv-casting-app/linux/simple-app-helper.cpp b/examples/tv-casting-app/linux/simple-app-helper.cpp index 3cf13fac8606f2..ac60b7fb4740f4 100644 --- a/examples/tv-casting-app/linux/simple-app-helper.cpp +++ b/examples/tv-casting-app/linux/simple-app-helper.cpp @@ -41,6 +41,7 @@ bool gCommissionerGeneratedPasscodeFlowRunning = false; DiscoveryDelegateImpl * DiscoveryDelegateImpl::_discoveryDelegateImpl = nullptr; bool gAwaitingCommissionerPasscodeInput = false; +LinuxCommissionableDataProvider gSimpleAppCommissionableDataProvider; std::shared_ptr targetCastingPlayer; DiscoveryDelegateImpl * DiscoveryDelegateImpl::GetInstance() @@ -470,9 +471,8 @@ CHIP_ERROR CommandHandler(int argc, char ** argv) // Commissioner-generated passcode, and then update the CastigApp's AppParameters to update the commissioning session's // passcode. LinuxDeviceOptions::GetInstance().payload.setUpPINCode = userEnteredPasscode; - LinuxCommissionableDataProvider gCommissionableDataProvider; - CHIP_ERROR err = CHIP_NO_ERROR; - err = InitCommissionableDataProvider(gCommissionableDataProvider, LinuxDeviceOptions::GetInstance()); + CHIP_ERROR err = CHIP_NO_ERROR; + err = InitCommissionableDataProvider(gSimpleAppCommissionableDataProvider, LinuxDeviceOptions::GetInstance()); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, @@ -482,7 +482,8 @@ CHIP_ERROR CommandHandler(int argc, char ** argv) } // Update the CommissionableDataProvider stored in this CastingApp's AppParameters and the CommissionableDataProvider to // be used for the commissioning session. - err = matter::casting::core::CastingApp::GetInstance()->UpdateCommissionableDataProvider(&gCommissionableDataProvider); + err = matter::casting::core::CastingApp::GetInstance()->UpdateCommissionableDataProvider( + &gSimpleAppCommissionableDataProvider); if (err != CHIP_NO_ERROR) { ChipLogError(AppServer, From c4a25e68e235440df7958c3d79f10b3a2afc0358 Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Wed, 24 Jul 2024 22:34:04 -0400 Subject: [PATCH 05/10] Add checks for OffWithEffect command parameters contraints (#34396) * Add checks for OffWithEffect command parameters contraints * Update src/app/clusters/on-off-server/on-off-server.cpp Co-authored-by: Andrei Litvin * address comment * For unknownd effect variant, Default to the respective 0 value enum variant * Update src/app/clusters/on-off-server/on-off-server.cpp Co-authored-by: Boris Zbarsky * fix up rename --------- Co-authored-by: Andrei Litvin Co-authored-by: Boris Zbarsky --- .../clusters/on-off-server/on-off-server.cpp | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp index 8332403d68ebd4..726da2d29f7c4d 100644 --- a/src/app/clusters/on-off-server/on-off-server.cpp +++ b/src/app/clusters/on-off-server/on-off-server.cpp @@ -89,6 +89,12 @@ void UpdateModeBaseCurrentModeToOnMode(EndpointId endpoint) #endif // MATTER_DM_PLUGIN_MODE_BASE +template +bool IsKnownEnumValue(EnumType value) +{ + return (EnsureKnownEnumValue(value) != EnumType::kUnknownEnumValue); +} + } // namespace #ifdef MATTER_DM_PLUGIN_LEVEL_CONTROL @@ -609,6 +615,35 @@ bool OnOffServer::offWithEffectCommand(app::CommandHandler * commandObj, const a chip::EndpointId endpoint = commandPath.mEndpointId; Status status = Status::Success; + if (effectId != EffectIdentifierEnum::kUnknownEnumValue) + { + // Depending on effectId value, effectVariant enum type varies. + // The following check validates that effectVariant value is valid in relation to the applicable enum type. + // DelayedAllOffEffectVariantEnum or DyingLightEffectVariantEnum + if (effectId == EffectIdentifierEnum::kDelayedAllOff && + !IsKnownEnumValue(static_cast(effectVariant))) + { + // The server does not support the given variant, it SHALL use the default variant. + effectVariant = to_underlying(DelayedAllOffEffectVariantEnum::kDelayedOffFastFade); + } + else if (effectId == EffectIdentifierEnum::kDyingLight && + !IsKnownEnumValue(static_cast(effectVariant))) + { + // The server does not support the given variant, it SHALL use the default variant. + effectVariant = to_underlying(DyingLightEffectVariantEnum::kDyingLightFadeOff); + } + } + else + { + status = Status::ConstraintError; + } + + if (status != Status::Success) + { + commandObj->AddStatus(commandPath, status); + return true; + } + if (SupportsLightingApplications(endpoint)) { #ifdef MATTER_DM_PLUGIN_SCENES_MANAGEMENT From 21a5bd6a402b699c0d64283918e266092a771bd1 Mon Sep 17 00:00:00 2001 From: fesseha-eve <88329315+fessehaeve@users.noreply.github.com> Date: Thu, 25 Jul 2024 08:53:51 +0200 Subject: [PATCH 06/10] update constraints for LocalTemperatureCalibration and MinSetpointDeadBand attributes (#34473) --- .../outputs/all-clusters-app/app-templates/endpoint_config.h | 2 +- .../zap-templates/zcl/data-model/chip/thermostat-cluster.xml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h index f2c082b1d0d4a1..a77bd74f11f267 100644 --- a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h +++ b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h @@ -307,7 +307,7 @@ { (uint16_t) 0xBB8, (uint16_t) -0x6AB3, (uint16_t) 0x7FFF }, /* MaxHeatSetpointLimit */ \ { (uint16_t) 0x640, (uint16_t) -0x6AB3, (uint16_t) 0x7FFF }, /* MinCoolSetpointLimit */ \ { (uint16_t) 0xC80, (uint16_t) -0x6AB3, (uint16_t) 0x7FFF }, /* MaxCoolSetpointLimit */ \ - { (uint16_t) 0x19, (uint16_t) 0x0, (uint16_t) 0x19 }, /* MinSetpointDeadBand */ \ + { (uint16_t) 0x19, (uint16_t) 0x0, (uint16_t) 0x7F }, /* MinSetpointDeadBand */ \ { (uint16_t) 0x4, (uint16_t) 0x0, (uint16_t) 0x5 }, /* ControlSequenceOfOperation */ \ { (uint16_t) 0x1, (uint16_t) 0x0, (uint16_t) 0x9 }, /* SystemMode */ \ \ diff --git a/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml index 91a89497d7c8cc..c2979487e45ae3 100644 --- a/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml @@ -337,7 +337,7 @@ limitations under the License. - + LocalTemperatureCalibration @@ -361,7 +361,7 @@ limitations under the License. MaxCoolSetpointLimit - + MinSetpointDeadBand From 335ac96cdde78fd81d69d6fb451427e769b7f413 Mon Sep 17 00:00:00 2001 From: fesseha-eve <88329315+fessehaeve@users.noreply.github.com> Date: Thu, 25 Jul 2024 08:54:38 +0200 Subject: [PATCH 07/10] update tests and thermostat server cluster for new constraints for LocalTemperatureCalibration and MinSetpointDeadBand (#34474) --- src/app/clusters/thermostat-server/thermostat-server.cpp | 2 +- src/app/tests/suites/certification/Test_TC_TSTAT_2_1.yaml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp index c650a30c871989..e802a9201f33dc 100644 --- a/src/app/clusters/thermostat-server/thermostat-server.cpp +++ b/src/app/clusters/thermostat-server/thermostat-server.cpp @@ -409,7 +409,7 @@ MatterThermostatClusterServerPreAttributeChangedCallback(const app::ConcreteAttr requested = *value; if (!AutoSupported) return imcode::UnsupportedAttribute; - if (requested < 0 || requested > 25) + if (requested < 0 || requested > 127) return imcode::InvalidValue; return imcode::Success; } diff --git a/src/app/tests/suites/certification/Test_TC_TSTAT_2_1.yaml b/src/app/tests/suites/certification/Test_TC_TSTAT_2_1.yaml index faf34fdea43e92..4558b08745a949 100644 --- a/src/app/tests/suites/certification/Test_TC_TSTAT_2_1.yaml +++ b/src/app/tests/suites/certification/Test_TC_TSTAT_2_1.yaml @@ -243,8 +243,8 @@ tests: response: constraints: type: int8s - minValue: -25 - maxValue: 25 + minValue: -127 + maxValue: 127 - label: "Step 13a: TH reads attribute OccupiedCoolingSetpoint from the DUT" PICS: TSTAT.S.F01 && TSTAT.S.A0017 && TSTAT.S.A0018 @@ -426,7 +426,7 @@ tests: constraints: type: int8s minValue: 0 - maxValue: 25 + maxValue: 127 - label: "Step 22: TH reads the RemoteSensing attribute from the DUT" PICS: TSTAT.S.A001a From 34462f1ffadf26ec31195c85d2ec24a3922cad69 Mon Sep 17 00:00:00 2001 From: Abdul Samad Date: Thu, 25 Jul 2024 01:56:13 -0500 Subject: [PATCH 08/10] Add target endpoint to `CommissioningWindowOpener` (#34425) * Add target endpoint id to commissioning window opener params Co-authored-by: Yufeng Wang * Set endpoint id in fabric-admin app * Set root endpoint id by default in commissioning window params Co-authored-by: Boris Zbarsky --------- Co-authored-by: Yufeng Wang Co-authored-by: Boris Zbarsky --- .../commands/pairing/OpenCommissioningWindowCommand.cpp | 2 ++ src/controller/CommissioningWindowOpener.cpp | 6 +++--- src/controller/CommissioningWindowOpener.h | 3 ++- src/controller/CommissioningWindowParams.h | 9 +++++++++ 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp index 480cfbca35a20b..b2d811fdc8b114 100644 --- a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp +++ b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp @@ -38,6 +38,7 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() VerifyOrReturnError(mSalt.HasValue(), CHIP_ERROR_INVALID_ARGUMENT); return mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowVerifierParams() .SetNodeId(mNodeId) + .SetEndpointId(mEndpointId) .SetTimeout(mCommissioningWindowTimeout) .SetIteration(mIteration) .SetDiscriminator(mDiscriminator) @@ -50,6 +51,7 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand() SetupPayload ignored; return mWindowOpener->OpenCommissioningWindow(Controller::CommissioningWindowPasscodeParams() .SetNodeId(mNodeId) + .SetEndpointId(mEndpointId) .SetTimeout(mCommissioningWindowTimeout) .SetIteration(mIteration) .SetDiscriminator(mDiscriminator) diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp index 35011e69565c4a..e8d1b29cb6437d 100644 --- a/src/controller/CommissioningWindowOpener.cpp +++ b/src/controller/CommissioningWindowOpener.cpp @@ -126,6 +126,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin mCommissioningWindowVerifierCallback = nullptr; mNodeId = params.GetNodeId(); mCommissioningWindowTimeout = params.GetTimeout(); + mTargetEndpointId = params.GetEndpointId(); if (params.GetReadVIDPIDAttributes()) { @@ -162,6 +163,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindow(const Commissionin mPBKDFIterations = params.GetIteration(); mCommissioningWindowOption = CommissioningWindowOption::kTokenWithProvidedPIN; mDiscriminator.SetLongValue(params.GetDiscriminator()); + mTargetEndpointId = params.GetEndpointId(); mNextStep = Step::kOpenCommissioningWindow; @@ -173,9 +175,7 @@ CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Messaging: { ChipLogProgress(Controller, "OpenCommissioningWindow for device ID 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId)); - constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0; - - ClusterBase cluster(exchangeMgr, sessionHandle, kAdministratorCommissioningClusterEndpoint); + ClusterBase cluster(exchangeMgr, sessionHandle, mTargetEndpointId); if (mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode) { diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h index b657345ca53219..28a25d77f7a014 100644 --- a/src/controller/CommissioningWindowOpener.h +++ b/src/controller/CommissioningWindowOpener.h @@ -165,7 +165,8 @@ class CommissioningWindowOpener Callback::Callback * mBasicCommissioningWindowCallback = nullptr; SetupPayload mSetupPayload; SetupDiscriminator mDiscriminator{}; - NodeId mNodeId = kUndefinedNodeId; + NodeId mNodeId = kUndefinedNodeId; + EndpointId mTargetEndpointId = kRootEndpointId; // Default endpoint for Administrator Commissioning Cluster System::Clock::Seconds16 mCommissioningWindowTimeout = System::Clock::kZero; CommissioningWindowOption mCommissioningWindowOption = CommissioningWindowOption::kOriginalSetupCode; Crypto::Spake2pVerifier mVerifier; // Used for non-basic commissioning. diff --git a/src/controller/CommissioningWindowParams.h b/src/controller/CommissioningWindowParams.h index a4f0e43fa096c2..c845ecf2c80135 100644 --- a/src/controller/CommissioningWindowParams.h +++ b/src/controller/CommissioningWindowParams.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -53,6 +54,13 @@ class CommissioningWindowCommonParams return static_cast(*this); } + EndpointId GetEndpointId() const { return mEndpointId; } + Derived & SetEndpointId(EndpointId endpointId) + { + mEndpointId = endpointId; + return static_cast(*this); + } + System::Clock::Seconds16 GetTimeout() const { return mTimeout; } // The duration for which the commissioning window should remain open. Derived & SetTimeout(System::Clock::Seconds16 timeout) @@ -82,6 +90,7 @@ class CommissioningWindowCommonParams private: NodeId mNodeId = kUndefinedNodeId; + EndpointId mEndpointId = kRootEndpointId; // Default endpoint for Administrator Commissioning Cluster System::Clock::Seconds16 mTimeout = System::Clock::Seconds16(300); // Defaulting uint32_t mIteration = 1000; // Defaulting Optional mDiscriminator = NullOptional; // Using optional type to avoid picking a sentinnel in valid range From 1b83c3a3dd124d2fe6f4938430bc6ed418e6ce9f Mon Sep 17 00:00:00 2001 From: C Freeman Date: Thu, 25 Jul 2024 08:22:16 -0400 Subject: [PATCH 09/10] TC-IDM-10.2: Add tests for choice conformance (#34445) * TC-IDM-10.2: Add tests for choice conformance Adds functions to evaluate conformance on elements (features, attributes and commands). Tests: - unit tests included - tested on all-clusters-app, and all-clusters-app with switch feature conformance changed to be incorrect. * styling and linter * Restyled by autopep8 * Restyled by isort * Apply suggestions from code review Co-authored-by: Andrei Litvin --------- Co-authored-by: Restyled.io Co-authored-by: Andrei Litvin --- src/python_testing/TC_DeviceConformance.py | 14 +- .../TestChoiceConformanceSupport.py | 211 ++++++++++++++++++ .../choice_conformance_support.py | 74 ++++++ 3 files changed, 298 insertions(+), 1 deletion(-) create mode 100644 src/python_testing/TestChoiceConformanceSupport.py create mode 100644 src/python_testing/choice_conformance_support.py diff --git a/src/python_testing/TC_DeviceConformance.py b/src/python_testing/TC_DeviceConformance.py index 851921df60f0a4..c7fa19c45cec2c 100644 --- a/src/python_testing/TC_DeviceConformance.py +++ b/src/python_testing/TC_DeviceConformance.py @@ -32,6 +32,8 @@ import chip.clusters as Clusters from basic_composition_support import BasicCompositionTests from chip.tlv import uint +from choice_conformance_support import (evaluate_attribute_choice_conformance, evaluate_command_choice_conformance, + evaluate_feature_choice_conformance) from conformance_support import ConformanceDecision, conformance_allowed from global_attribute_ids import GlobalAttributeIds from matter_testing_support import (AttributePathLocation, ClusterPathLocation, CommandPathLocation, MatterBaseTest, ProblemNotice, @@ -188,7 +190,17 @@ def check_spec_conformance_for_commands(command_type: CommandType): check_spec_conformance_for_commands(CommandType.ACCEPTED) check_spec_conformance_for_commands(CommandType.GENERATED) - # TODO: Add choice checkers + feature_choice_problems = evaluate_feature_choice_conformance( + endpoint_id, cluster_id, self.xml_clusters, feature_map, attribute_list, all_command_list) + attribute_choice_problems = evaluate_attribute_choice_conformance( + endpoint_id, cluster_id, self.xml_clusters, feature_map, attribute_list, all_command_list) + command_choice_problem = evaluate_command_choice_conformance( + endpoint_id, cluster_id, self.xml_clusters, feature_map, attribute_list, all_command_list) + + if feature_choice_problems or attribute_choice_problems or command_choice_problem: + success = False + problems.extend(feature_choice_problems + attribute_choice_problems + command_choice_problem) + print(f'success = {success}') return success, problems diff --git a/src/python_testing/TestChoiceConformanceSupport.py b/src/python_testing/TestChoiceConformanceSupport.py new file mode 100644 index 00000000000000..8436bc8418a804 --- /dev/null +++ b/src/python_testing/TestChoiceConformanceSupport.py @@ -0,0 +1,211 @@ +# +# Copyright (c) 2023 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 itertools +import xml.etree.ElementTree as ElementTree + +import jinja2 +from choice_conformance_support import (evaluate_attribute_choice_conformance, evaluate_command_choice_conformance, + evaluate_feature_choice_conformance) +from matter_testing_support import MatterBaseTest, ProblemNotice, default_matter_test_main +from mobly import asserts +from spec_parsing_support import XmlCluster, add_cluster_data_from_xml + +FEATURE_TEMPLATE = '''\ + + + {%- if XXX %}' + + {% endif %} + + +''' + +ATTRIBUTE_TEMPLATE = ( + ' \n' + ' \n' + ' {% if XXX %}' + ' \n' + ' {% endif %}' + ' \n' + ' \n' +) + +COMMAND_TEMPLATE = ( + ' \n' + ' \n' + ' {% if XXX %}' + ' \n' + ' {% endif %}' + ' \n' + ' \n' +) + +CLUSTER_TEMPLATE = ( + '\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + ' {{ feature_string }}\n' + ' \n' + ' \n' + ' {{ attribute_string}}\n' + ' \n' + ' \n' + ' {{ command_string }}\n' + ' \n' + '\n') + + +def _create_elements(template_str: str, base_name: str) -> list[str]: + xml_str = [] + + def add_elements(curr_choice: str, starting_id: int, more: str, XXX: bool): + for i in range(3): + element_name = f'{base_name}{curr_choice.upper()*(i+1)}' + environment = jinja2.Environment() + template = environment.from_string(template_str) + xml_str.append(template.render(id=(i + starting_id), name=element_name, choice=curr_choice, more=more, XXX=XXX)) + add_elements('a', 1, 'false', False) + add_elements('b', 4, 'true', False) + add_elements('c', 7, 'false', True) + add_elements('d', 10, 'true', True) + + return xml_str + +# TODO: this setup makes my life easy because it assumes that choice conformances apply only within one table +# if this is not true (ex, you can have choose 1 of a feature or an attribute), then this gets more complex +# in this case we need to have this test evaluate the same choice conformance value between multiple tables, and all +# the conformances need to be assessed for the entire element set. +# I've done it this way specifically so I can hardcode the choice values and test the features, attributes and commands +# separately, even though I load them all at the start. + +# Cluster with choices on all elements +# 3 of each element with O.a +# 3 of each element with O.b+ +# 3 of each element with [XXX].c +# 3 of each element with [XXX].d+ +# 1 element named XXX + + +def _create_features(): + xml = _create_elements(FEATURE_TEMPLATE, 'F') + xxx = (' \n' + ' \n' + ' \n') + xml.append(xxx) + return '\n'.join(xml) + + +def _create_attributes(): + xml = _create_elements(ATTRIBUTE_TEMPLATE, "attr") + xxx = (' \n' + ' \n' + ' \n') + xml.append(xxx) + return '\n'.join(xml) + + +def _create_commands(): + xml = _create_elements(COMMAND_TEMPLATE, 'cmd') + xxx = (' \n' + ' \n' + ' \n') + xml.append(xxx) + return '\n'.join(xml) + + +def _create_cluster(): + environment = jinja2.Environment() + template = environment.from_string(CLUSTER_TEMPLATE) + return template.render(feature_string=_create_features(), attribute_string=_create_attributes(), command_string=_create_commands()) + + +class TestConformanceSupport(MatterBaseTest): + def setup_class(self): + super().setup_class() + + clusters: dict[int, XmlCluster] = {} + pure_base_clusters: dict[str, XmlCluster] = {} + ids_by_name: dict[str, int] = {} + problems: list[ProblemNotice] = [] + cluster_xml = ElementTree.fromstring(_create_cluster()) + add_cluster_data_from_xml(cluster_xml, clusters, pure_base_clusters, ids_by_name, problems) + self.clusters = clusters + # each element type uses 13 IDs from 1-13 (or bits for the features) and we want to test all the combinations + num_elements = 13 + ids = range(1, num_elements + 1) + self.all_id_combos = [] + combos = [] + for r in range(1, num_elements + 1): + combos.extend(list(itertools.combinations(ids, r))) + for combo in combos: + # The first three IDs are all O.a, so we need exactly one for the conformance to be valid + expected_failures = set() + if len(set([1, 2, 3]) & set(combo)) != 1: + expected_failures.add('a') + if len(set([4, 5, 6]) & set(combo)) < 1: + expected_failures.add('b') + # For these, we are checking that choice conformance checkers + # - Correctly report errors and correct cases when the gating feature is ON + # - Do not report any errors when the gating features is off. + # Errors where we incorrectly set disallowed features based on the gating feature are checked + # elsewhere in the cert test in a comprehensive way. We just want to ensure that we are not + # incorrectly reporting choice conformance error as well + if 13 in combo and ((len(set([7, 8, 9]) & set(combo)) != 1)): + expected_failures.add('c') + if 13 in combo and (len(set([10, 11, 12]) & set(combo)) < 1): + expected_failures.add('d') + + self.all_id_combos.append((combo, expected_failures)) + + def _evaluate_problems(self, problems, expected_failures=list[str]): + if len(expected_failures) != len(problems): + print(problems) + asserts.assert_equal(len(expected_failures), len(problems), 'Unexpected number of choice conformance problems') + actual_failures = set([p.choice.marker for p in problems]) + asserts.assert_equal(actual_failures, expected_failures, "Mismatch between failures") + + def test_features(self): + def make_feature_map(combo: tuple[int]) -> int: + feature_map = 0 + for bit in combo: + feature_map += pow(2, bit) + return feature_map + + for combo, expected_failures in self.all_id_combos: + problems = evaluate_feature_choice_conformance(0, 1, self.clusters, make_feature_map(combo), [], []) + self._evaluate_problems(problems, expected_failures) + + def test_attributes(self): + for combo, expected_failures in self.all_id_combos: + problems = evaluate_attribute_choice_conformance(0, 1, self.clusters, 0, list(combo), []) + self._evaluate_problems(problems, expected_failures) + + def test_commands(self): + for combo, expected_failures in self.all_id_combos: + problems = evaluate_command_choice_conformance(0, 1, self.clusters, 0, [], list(combo)) + self._evaluate_problems(problems, expected_failures) + + +if __name__ == "__main__": + default_matter_test_main() diff --git a/src/python_testing/choice_conformance_support.py b/src/python_testing/choice_conformance_support.py new file mode 100644 index 00000000000000..58d37bf10180b0 --- /dev/null +++ b/src/python_testing/choice_conformance_support.py @@ -0,0 +1,74 @@ +from chip.tlv import uint +from conformance_support import Choice, ConformanceDecisionWithChoice +from global_attribute_ids import GlobalAttributeIds +from matter_testing_support import AttributePathLocation, ProblemNotice, ProblemSeverity +from spec_parsing_support import XmlCluster + + +class ChoiceConformanceProblemNotice(ProblemNotice): + def __init__(self, location: AttributePathLocation, choice: Choice, count: int): + problem = f'Problem with choice conformance {choice} - {count} selected' + super().__init__(test_name='Choice conformance', location=location, severity=ProblemSeverity.ERROR, problem=problem, spec_location='') + self.choice = choice + self.count = count + + +def _add_to_counts_if_required(conformance_decision_with_choice: ConformanceDecisionWithChoice, element_present: bool, counts: dict[Choice, int]): + choice = conformance_decision_with_choice.choice + if not choice: + return + counts[choice] = counts.get(choice, 0) + if element_present: + counts[choice] += 1 + + +def _evaluate_choices(location: AttributePathLocation, counts: dict[Choice, int]) -> list[ChoiceConformanceProblemNotice]: + problems: list[ChoiceConformanceProblemNotice] = [] + for choice, count in counts.items(): + if count == 0 or (not choice.more and count > 1): + problems.append(ChoiceConformanceProblemNotice(location, choice, count)) + return problems + + +def evaluate_feature_choice_conformance(endpoint_id: int, cluster_id: int, xml_clusters: dict[int, XmlCluster], feature_map: uint, attribute_list: list[uint], all_command_list: list[uint]) -> list[ChoiceConformanceProblemNotice]: + all_features = [1 << i for i in range(32)] + all_features = [f for f in all_features if f in xml_clusters[cluster_id].features.keys()] + + # Other pieces of the 10.2 test check for unknown features, so just remove them here to check choice conformance + counts: dict[Choice, int] = {} + for f in all_features: + xml_feature = xml_clusters[cluster_id].features[f] + conformance_decision_with_choice = xml_feature.conformance(feature_map, attribute_list, all_command_list) + _add_to_counts_if_required(conformance_decision_with_choice, (feature_map & f), counts) + + location = AttributePathLocation(endpoint_id=endpoint_id, cluster_id=cluster_id, + attribute_id=GlobalAttributeIds.FEATURE_MAP_ID) + return _evaluate_choices(location, counts) + + +def evaluate_attribute_choice_conformance(endpoint_id: int, cluster_id: int, xml_clusters: dict[int, XmlCluster], feature_map: uint, attribute_list: list[uint], all_command_list: list[uint]) -> list[ChoiceConformanceProblemNotice]: + all_attributes = xml_clusters[cluster_id].attributes.keys() + + counts: dict[Choice, int] = {} + for attribute_id in all_attributes: + conformance_decision_with_choice = xml_clusters[cluster_id].attributes[attribute_id].conformance( + feature_map, attribute_list, all_command_list) + _add_to_counts_if_required(conformance_decision_with_choice, attribute_id in attribute_list, counts) + + location = AttributePathLocation(endpoint_id=endpoint_id, cluster_id=cluster_id, + attribute_id=GlobalAttributeIds.ATTRIBUTE_LIST_ID) + return _evaluate_choices(location, counts) + + +def evaluate_command_choice_conformance(endpoint_id: int, cluster_id: int, xml_clusters: dict[int, XmlCluster], feature_map: uint, attribute_list: list[uint], all_command_list: list[uint]) -> list[ChoiceConformanceProblemNotice]: + all_commands = xml_clusters[cluster_id].accepted_commands.keys() + + counts: dict[Choice, int] = {} + for command_id in all_commands: + conformance_decision_with_choice = xml_clusters[cluster_id].accepted_commands[command_id].conformance( + feature_map, attribute_list, all_command_list) + _add_to_counts_if_required(conformance_decision_with_choice, command_id in all_command_list, counts) + + location = AttributePathLocation(endpoint_id=endpoint_id, cluster_id=cluster_id, + attribute_id=GlobalAttributeIds.ACCEPTED_COMMAND_LIST_ID) + return _evaluate_choices(location, counts) From 1b1340f36557a7cf1bac3f4b99ee0ce622269c40 Mon Sep 17 00:00:00 2001 From: Kamil Kasperczyk <66371704+kkasperczyk-no@users.noreply.github.com> Date: Thu, 25 Jul 2024 15:46:38 +0200 Subject: [PATCH 10/10] [examples] Fixed shell build for nrfconnect (#34500) Shell example build for nrfconnect was broken due to missing sysbuild.conf file that enabled Matter. --- examples/shell/nrfconnect/sysbuild.conf | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 examples/shell/nrfconnect/sysbuild.conf diff --git a/examples/shell/nrfconnect/sysbuild.conf b/examples/shell/nrfconnect/sysbuild.conf new file mode 100644 index 00000000000000..e63a92c6b2bbab --- /dev/null +++ b/examples/shell/nrfconnect/sysbuild.conf @@ -0,0 +1,18 @@ +# +# Copyright (c) 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. +# 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. +# + +SB_CONFIG_MATTER=y +SB_CONFIG_MATTER_OTA=n