Skip to content

Commit bb9363d

Browse files
andy31415andreilitvinrestyled-commits
authored
remove a call of emberAfContainsAttribute when using the data model provider interface (project-chip#35746)
* Remove a direct ember call from InteractionModelEngine if data model interface is enabled * Enforce identical DM vs ember logic * Fix up unit tests: because dynamic endpoints are reset in an incompatible manner, the data model needs resetting * Include fixes * Restyled by clang-format * Update src/app/codegen-data-model-provider/CodegenDataModelProvider.h * Comment update to kick CI again * make it more readable based on code review feedback * Restyle --------- Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Restyled.io <commits@restyled.io>
1 parent e6b726f commit bb9363d

File tree

4 files changed

+52
-6
lines changed

4 files changed

+52
-6
lines changed

src/app/InteractionModelEngine.cpp

+23-1
Original file line numberDiff line numberDiff line change
@@ -1584,6 +1584,26 @@ CHIP_ERROR InteractionModelEngine::PushFrontAttributePathList(SingleLinkedListNo
15841584
return err;
15851585
}
15861586

1587+
bool InteractionModelEngine::IsExistingAttributePath(const ConcreteAttributePath & path)
1588+
{
1589+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
1590+
#if CHIP_CONFIG_USE_EMBER_DATA_MODEL
1591+
// Ensure that Provider interface and ember are IDENTICAL in attribute location (i.e. "check" mode)
1592+
VerifyOrDie(GetDataModelProvider()
1593+
->GetAttributeInfo(ConcreteAttributePath(path.mEndpointId, path.mClusterId, path.mAttributeId))
1594+
.has_value() == emberAfContainsAttribute(path.mEndpointId, path.mClusterId, path.mAttributeId)
1595+
1596+
);
1597+
#endif
1598+
1599+
return GetDataModelProvider()
1600+
->GetAttributeInfo(ConcreteAttributePath(path.mEndpointId, path.mClusterId, path.mAttributeId))
1601+
.has_value();
1602+
#else
1603+
return emberAfContainsAttribute(path.mEndpointId, path.mClusterId, path.mAttributeId);
1604+
#endif
1605+
}
1606+
15871607
void InteractionModelEngine::RemoveDuplicateConcreteAttributePath(SingleLinkedListNode<AttributePathParams> *& aAttributePaths)
15881608
{
15891609
SingleLinkedListNode<AttributePathParams> * prev = nullptr;
@@ -1592,9 +1612,11 @@ void InteractionModelEngine::RemoveDuplicateConcreteAttributePath(SingleLinkedLi
15921612
while (path1 != nullptr)
15931613
{
15941614
bool duplicate = false;
1615+
15951616
// skip all wildcard paths and invalid concrete attribute
15961617
if (path1->mValue.IsWildcardPath() ||
1597-
!emberAfContainsAttribute(path1->mValue.mEndpointId, path1->mValue.mClusterId, path1->mValue.mAttributeId))
1618+
!IsExistingAttributePath(
1619+
ConcreteAttributePath(path1->mValue.mEndpointId, path1->mValue.mClusterId, path1->mValue.mAttributeId)))
15981620
{
15991621
prev = path1;
16001622
path1 = path1->mpNext;

src/app/InteractionModelEngine.h

+5
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,11 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
612612
void ShutdownMatchingSubscriptions(const Optional<FabricIndex> & aFabricIndex = NullOptional,
613613
const Optional<NodeId> & aPeerNodeId = NullOptional);
614614

615+
/**
616+
* Check if the given attribute path is a valid path in the data model provider.
617+
*/
618+
bool IsExistingAttributePath(const ConcreteAttributePath & path);
619+
615620
static void ResumeSubscriptionsTimerCallback(System::Layer * apSystemLayer, void * apAppState);
616621

617622
template <typename T, size_t N>

src/app/codegen-data-model-provider/CodegenDataModelProvider.h

+16-1
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,26 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider
6363

6464
/// Checks if the given command id exists in the given list
6565
bool Exists(const CommandId * list, CommandId toCheck);
66+
67+
void Reset() { mCurrentList = mCurrentHint = nullptr; }
6668
};
6769

6870
public:
71+
/// clears out internal caching. Especially useful in unit tests,
72+
/// where path caching does not really apply (the same path may result in different outcomes)
73+
void Reset()
74+
{
75+
mAcceptedCommandsIterator.Reset();
76+
mGeneratedCommandsIterator.Reset();
77+
mPreviouslyFoundCluster = std::nullopt;
78+
}
79+
6980
/// Generic model implementations
70-
CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; }
81+
CHIP_ERROR Shutdown() override
82+
{
83+
Reset();
84+
return CHIP_NO_ERROR;
85+
}
7186

7287
DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request,
7388
AttributeValueEncoder & encoder) override;

src/controller/tests/TestEventChunking.cpp

+8-4
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@
1818

1919
#include <pw_unit_test/framework.h>
2020

21-
#include "app-common/zap-generated/ids/Attributes.h"
22-
#include "app-common/zap-generated/ids/Clusters.h"
23-
#include "app/ConcreteAttributePath.h"
24-
#include "protocols/interaction_model/Constants.h"
2521
#include <app-common/zap-generated/cluster-objects.h>
22+
#include <app-common/zap-generated/ids/Attributes.h>
23+
#include <app-common/zap-generated/ids/Clusters.h>
2624
#include <app/AppConfig.h>
2725
#include <app/AttributeAccessInterface.h>
2826
#include <app/AttributeAccessInterfaceRegistry.h>
2927
#include <app/BufferedReadCallback.h>
3028
#include <app/CommandHandlerInterface.h>
29+
#include <app/ConcreteAttributePath.h>
3130
#include <app/EventLogging.h>
3231
#include <app/GlobalAttributes.h>
3332
#include <app/InteractionModelEngine.h>
33+
#include <app/codegen-data-model-provider/Instance.h>
3434
#include <app/data-model/Decode.h>
3535
#include <app/tests/AppTestContext.h>
3636
#include <app/util/DataModelHandler.h>
@@ -42,6 +42,7 @@
4242
#include <lib/support/CHIPCounter.h>
4343
#include <lib/support/TimeUtils.h>
4444
#include <lib/support/logging/CHIPLogging.h>
45+
#include <protocols/interaction_model/Constants.h>
4546

4647
using namespace chip;
4748
using namespace chip::app;
@@ -293,6 +294,7 @@ TEST_F(TestEventChunking, TestEventChunking)
293294
app::InteractionModelEngine * engine = app::InteractionModelEngine::GetInstance();
294295

295296
// Initialize the ember side server logic
297+
CodegenDataModelProviderInstance()->Shutdown();
296298
InitDataModelHandler();
297299

298300
// Register our fake dynamic endpoint.
@@ -359,6 +361,7 @@ TEST_F(TestEventChunking, TestMixedEventsAndAttributesChunking)
359361
app::InteractionModelEngine * engine = app::InteractionModelEngine::GetInstance();
360362

361363
// Initialize the ember side server logic
364+
CodegenDataModelProviderInstance()->Shutdown();
362365
InitDataModelHandler();
363366

364367
// Register our fake dynamic endpoint.
@@ -435,6 +438,7 @@ TEST_F(TestEventChunking, TestMixedEventsAndLargeAttributesChunking)
435438
app::InteractionModelEngine * engine = app::InteractionModelEngine::GetInstance();
436439

437440
// Initialize the ember side server logic
441+
CodegenDataModelProviderInstance()->Shutdown();
438442
InitDataModelHandler();
439443

440444
// Register our fake dynamic endpoint.

0 commit comments

Comments
 (0)