Skip to content

Commit 483af54

Browse files
andy31415andreilitvinbzbarsky-apple
authored
Update AttributePathExpandIterator to be able to use the IM/DM DataModel split (#34288)
* Ember invoke implementation with unit tests inside DataModel * Support ember/datamodel/dual(checked) attribute path expand iterators * Code review comments * Fix interactionmodel::status compilation in icd-management-server when ICD is on * Do not include CodegeDataModelInstance if we do not use the codegen data model * Fix linter * Fix java controller linkage. This is still broken: there should be no reason java should want ember, yet here we are (with odd dependencies inside libCHIP) * Fix Amebad compilation: OUT is defined in Ameba * Update log formatting * Review comment: use GlobalAttributesNotInMetadata directly * Remove debug bits * Code review update: we just need to match next logic in the ember code in the dm code. * Restyle * Do not check for differences if result is false ... this is what people expect even if we guarantee perfect overlap * Fix typo * Restyle * Update src/app/AttributePathExpandIterator-DataModel.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 4762aaa commit 483af54

33 files changed

+1042
-193
lines changed

.github/workflows/lint.yml

+1
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ jobs:
296296
':(exclude)src/app/util/attribute-table.cpp' \
297297
':(exclude)src/app/util/attribute-table.h' \
298298
':(exclude)src/app/util/ember-compatibility-functions.cpp' \
299+
':(exclude)src/app/util/mock/CodegenEmberMocks.cpp' \
299300
':(exclude)src/app/zap-templates/templates/app/attributes/Accessors-src.zapt' \
300301
':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' \
301302
&& exit 1 || exit 0

scripts/build/builders/host.py

+4
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,10 @@ def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE,
454454
if self.app == HostApp.SIMULATED_APP2:
455455
self.extra_gn_options.append('chip_tests_zap_config="app2"')
456456

457+
if self.app in {HostApp.JAVA_MATTER_CONTROLLER, HostApp.KOTLIN_MATTER_CONTROLLER}:
458+
# TODO: controllers depending on a datamodel is odd. For now fix compile dependencies on ember.
459+
self.extra_gn_options.append('chip_use_data_model_interface="disabled"')
460+
457461
if self.app == HostApp.TESTS and fuzzing_type != HostFuzzingType.NONE:
458462
self.build_command = 'fuzz_tests'
459463

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright (c) 2024 Project CHIP Authors
3+
* All rights reserved.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
#include "lib/support/logging/TextOnlyLogging.h"
19+
#include <app/AttributePathExpandIterator-Checked.h>
20+
21+
namespace chip {
22+
namespace app {
23+
AttributePathExpandIteratorChecked::AttributePathExpandIteratorChecked(InteractionModel::DataModel * dataModel,
24+
SingleLinkedListNode<AttributePathParams> * attributePath) :
25+
mDataModelIterator(dataModel, attributePath),
26+
mEmberIterator(dataModel, attributePath)
27+
{
28+
CheckOutputsIdentical("Constructor");
29+
}
30+
31+
bool AttributePathExpandIteratorChecked::Next()
32+
{
33+
bool dmResult = mDataModelIterator.Next();
34+
bool emResult = mEmberIterator.Next();
35+
36+
CheckOutputsIdentical("Next");
37+
38+
VerifyOrDie(dmResult == emResult);
39+
40+
return emResult;
41+
}
42+
43+
bool AttributePathExpandIteratorChecked::Get(ConcreteAttributePath & aPath)
44+
{
45+
CheckOutputsIdentical("Get");
46+
return mEmberIterator.Get(aPath);
47+
}
48+
49+
void AttributePathExpandIteratorChecked::ResetCurrentCluster()
50+
{
51+
mDataModelIterator.ResetCurrentCluster();
52+
mEmberIterator.ResetCurrentCluster();
53+
54+
CheckOutputsIdentical("ResetCurrentCluster");
55+
}
56+
57+
void AttributePathExpandIteratorChecked::ResetTo(SingleLinkedListNode<AttributePathParams> * paths)
58+
59+
{
60+
mDataModelIterator.ResetTo(paths);
61+
mEmberIterator.ResetTo(paths);
62+
CheckOutputsIdentical("ResetTo");
63+
}
64+
65+
void AttributePathExpandIteratorChecked::CheckOutputsIdentical(const char * msg)
66+
{
67+
ConcreteAttributePath dmPath;
68+
ConcreteAttributePath emPath;
69+
70+
bool dmResult = mDataModelIterator.Get(dmPath);
71+
bool emResult = mEmberIterator.Get(emPath);
72+
73+
if (dmResult == emResult)
74+
{
75+
// We check for:
76+
// - either failed result (in which case path should not matter)
77+
// - or exact match of paths on success
78+
//
79+
// NOTE: extra logic because mExpanded is NOT considered in operator== (ugly...)
80+
if ((dmResult == false) || ((dmPath == emPath) && (dmPath.mExpanded == emPath.mExpanded)))
81+
{
82+
// outputs are identical. All is good
83+
return;
84+
}
85+
}
86+
87+
ChipLogProgress(Test, "Different paths in DM vs EMBER (%d and %d) in %s", dmResult, emResult, msg);
88+
ChipLogProgress(Test, " DM PATH: 0x%X/" ChipLogFormatMEI "/" ChipLogFormatMEI " (%s)", dmPath.mEndpointId,
89+
ChipLogValueMEI(dmPath.mClusterId), ChipLogValueMEI(dmPath.mAttributeId),
90+
dmPath.mExpanded ? "EXPANDED" : "NOT expanded");
91+
ChipLogProgress(Test, " EMBER PATH: 0x%X/" ChipLogFormatMEI "/" ChipLogFormatMEI " (%s)", emPath.mEndpointId,
92+
ChipLogValueMEI(emPath.mClusterId), ChipLogValueMEI(emPath.mAttributeId),
93+
emPath.mExpanded ? "EXPANDED" : "NOT expanded");
94+
95+
chipDie();
96+
}
97+
98+
} // namespace app
99+
} // namespace chip
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright (c) 2024 Project CHIP Authors
3+
* All rights reserved.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
#pragma once
19+
20+
#include <app/AttributePathExpandIterator-DataModel.h>
21+
#include <app/AttributePathExpandIterator-Ember.h>
22+
23+
namespace chip {
24+
namespace app {
25+
26+
class AttributePathExpandIteratorChecked
27+
{
28+
public:
29+
AttributePathExpandIteratorChecked(InteractionModel::DataModel * dataModel,
30+
SingleLinkedListNode<AttributePathParams> * attributePath);
31+
32+
bool Next();
33+
bool Get(ConcreteAttributePath & aPath);
34+
void ResetCurrentCluster();
35+
void ResetTo(SingleLinkedListNode<AttributePathParams> * paths);
36+
37+
private:
38+
AttributePathExpandIteratorDataModel mDataModelIterator;
39+
AttributePathExpandIteratorEmber mEmberIterator;
40+
41+
void CheckOutputsIdentical(const char * msg);
42+
};
43+
44+
} // namespace app
45+
} // namespace chip

0 commit comments

Comments
 (0)