Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update AttributePathExpandIterator to be able to use the IM/DM DataModel split #34288

Merged
merged 28 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
272ac40
Ember invoke implementation with unit tests inside DataModel
andy31415 Jul 6, 2024
95243b7
Support ember/datamodel/dual(checked) attribute path expand iterators
andy31415 Jul 6, 2024
c801b4c
Code review comments
andy31415 Jul 9, 2024
1e72d43
Merge branch 'imdm/5-ember-invoke-clean' into imdm/6-start-using-iter…
andy31415 Jul 9, 2024
9935d05
Merge branch 'master' into imdm/5-ember-invoke-clean
andy31415 Jul 10, 2024
2263249
Merge branch 'imdm/5-ember-invoke-clean' into imdm/6-start-using-iter…
andy31415 Jul 10, 2024
6a135cd
Fix interactionmodel::status compilation in icd-management-server whe…
andy31415 Jul 10, 2024
3a13e4a
Do not include CodegeDataModelInstance if we do not use the codegen d…
andy31415 Jul 10, 2024
abf2143
Fix linter
andreilitvin Jul 11, 2024
9339ac3
Fix java controller linkage.
andreilitvin Jul 11, 2024
492362c
Fix Amebad compilation: OUT is defined in Ameba
andreilitvin Jul 11, 2024
8b5b471
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 11, 2024
9972019
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 11, 2024
15136da
Update log formatting
andreilitvin Jul 17, 2024
0702f68
Review comment: use GlobalAttributesNotInMetadata directly
andreilitvin Jul 17, 2024
4e1bf41
Remove debug bits
andreilitvin Jul 17, 2024
17124ff
Code review update: we just need to match next logic in the ember cod…
andreilitvin Jul 17, 2024
0985403
Restyle
andreilitvin Jul 17, 2024
29f9cb0
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 17, 2024
d451f5a
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 17, 2024
a72c584
Do not check for differences if result is false ... this is what peop…
andreilitvin Jul 18, 2024
25d3294
Fix typo
andreilitvin Jul 18, 2024
381602b
Restyle
andreilitvin Jul 18, 2024
1e7cfbd
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 18, 2024
108ef01
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 18, 2024
cb42ae6
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 18, 2024
c3fa7d4
Merge branch 'master' into imdm/6-start-using-iterator
andreilitvin Jul 18, 2024
0c4134b
Update src/app/AttributePathExpandIterator-DataModel.cpp
andy31415 Jul 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions src/app/AttributePathExpandIterator-Checked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/

#include "lib/support/logging/TextOnlyLogging.h"
#include <app/AttributePathExpandIterator-Checked.h>

namespace chip {
Expand Down Expand Up @@ -69,19 +70,26 @@ void AttributePathExpandIteratorChecked::CheckOutputsIdentical(const char * msg)
bool dmResult = mDataModelIterator.Get(dmPath);
bool emResult = mEmberIterator.Get(emPath);

// NOTE: extra logic because mExpanded is NOT considered in operator== (ugly...)
if ((dmResult == emResult) && (dmPath == emPath) && (dmPath.mExpanded == emPath.mExpanded))
if (dmResult == emResult)
{
// outputs are identical. All is good
return;
// We check for:
// - either failed result (in which case path should not matter)
// - or exact match of paths on success
//
// NOTE: extra logic because mExpanded is NOT considered in operator== (ugly...)
if ((dmResult == false) || ((dmPath == emPath) && (dmPath.mExpanded == emPath.mExpanded)))
{
// outputs are identical. All is good
return;
}
}

ChipLogProgress(Test, "Different paths in DM vs EMBER (%d and %d) in %s", dmResult, emResult, msg);
ChipLogProgress(Test, " DM PATH: 0x%X/0x%X/0x%X (%s)", static_cast<int>(dmPath.mEndpointId),
static_cast<int>(dmPath.mClusterId), static_cast<int>(dmPath.mAttributeId),
ChipLogProgress(Test, " DM PATH: 0x%X/" ChipLogFormatMEI "/" ChipLogFormatMEI " (%s)", dmPath.mEndpointId,
ChipLogValueMEI(dmPath.mClusterId), ChipLogValueMEI(dmPath.mAttributeId),
dmPath.mExpanded ? "EXPANDED" : "NOT expanded");
ChipLogProgress(Test, " EMBER PATH: 0x%X/0x%X/0x%X (%s)", static_cast<int>(emPath.mEndpointId),
static_cast<int>(emPath.mClusterId), static_cast<int>(emPath.mAttributeId),
ChipLogProgress(Test, " EMBER PATH: 0x%X/" ChipLogFormatMEI "/" ChipLogFormatMEI " (%s)", emPath.mEndpointId,
ChipLogValueMEI(emPath.mClusterId), ChipLogValueMEI(emPath.mAttributeId),
emPath.mExpanded ? "EXPANDED" : "NOT expanded");

chipDie();
Expand Down
41 changes: 20 additions & 21 deletions src/app/AttributePathExpandIterator-DataModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "lib/support/CodeUtils.h"
#include <app/AttributePathExpandIterator-DataModel.h>
#include <app/GlobalAttributes.h>

Expand Down Expand Up @@ -80,19 +81,21 @@ std::optional<AttributeId> AttributePathExpandIteratorDataModel::NextAttributeId
// advance the existing attribute id if it can be advanced
VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardAttributeId(), std::nullopt);

// NOTE: this switch matches the order in GlobalAttributesNotInMetadata so that order-dependent
// checks match
switch (mOutputPath.mAttributeId)
// Ensure (including ordering) that GlobalAttributesNotInMetadata is reported as needed
for (unsigned i = 0; i < ArraySize(GlobalAttributesNotInMetadata); i++)
{
case Clusters::Globals::Attributes::GeneratedCommandList::Id:
return Clusters::Globals::Attributes::AcceptedCommandList::Id;
case Clusters::Globals::Attributes::AcceptedCommandList::Id:
#if CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE
return Clusters::Globals::Attributes::EventList::Id;
case Clusters::Globals::Attributes::EventList::Id:
#endif // CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE
return Clusters::Globals::Attributes::AttributeList::Id;
case Clusters::Globals::Attributes::AttributeList::Id:
if (GlobalAttributesNotInMetadata[i] != mOutputPath.mAttributeId)
{
continue;
}

unsigned nextAttributeIndex = i + 1;
if (nextAttributeIndex < ArraySize(GlobalAttributesNotInMetadata))
{
return GlobalAttributesNotInMetadata[nextAttributeIndex];
}

// reached the end of global attributes
return std::nullopt;
}

Expand All @@ -101,7 +104,10 @@ std::optional<AttributeId> AttributePathExpandIteratorDataModel::NextAttributeId
{
return entry.path.mAttributeId;
}
return Clusters::Globals::Attributes::GeneratedCommandList::Id;

// Finished the data model, start with global attributes
static_assert(ArraySize(GlobalAttributesNotInMetadata) > 0);
return GlobalAttributesNotInMetadata[0];
}

std::optional<ClusterId> AttributePathExpandIteratorDataModel::NextClusterId()
Expand Down Expand Up @@ -152,18 +158,11 @@ std::optional<ClusterId> AttributePathExpandIteratorDataModel::NextEndpointId()

void AttributePathExpandIteratorDataModel::ResetCurrentCluster()
{
ChipLogError(Test, "Reset current cluster data model");

// If this is a null iterator, or the attribute id of current cluster info is not a wildcard attribute id, then this function
// will do nothing, since we won't be expanding the wildcard attribute ids under a cluster.
VerifyOrReturn(mpAttributePath != nullptr && mpAttributePath->mValue.HasWildcardAttributeId());

// Reset to the first attribute of the current cluster.
// It is expected that the output path is already positioned on a proper cluster
//
AttributeEntry entry = mDataModel->FirstAttribute(mOutputPath);
VerifyOrDie(entry.IsValid()); // TODO: is this sane? do we require validity here?

// Proceed path expansion to ask for the first attribute of the current cluster
mOutputPath.mAttributeId = kInvalidAttributeId;
mOutputPath.mExpanded = true; // we know this is a wildcard attribute
Next();
Expand Down
Loading