From c88d5cf83cd3e3323ac196630acc34f196a2f405 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 24 Jul 2024 16:30:35 -0400 Subject: [PATCH] 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 {