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.

Sign up for GitHub

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

[im] Fix wildcard path support per spec #11115

Merged
merged 3 commits into from
Nov 4, 2021
Merged

[im] Fix wildcard path support per spec #11115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91e9d9b
[im] Fix wildcard path support per spec
erjiaqing Oct 28, 2021
485ad7a
Fix
erjiaqing Nov 4, 2021
7434468
Move kInvalid* to private
erjiaqing Nov 4, 2021
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
127 changes: 49 additions & 78 deletions src/app/ClusterInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,103 +18,74 @@

#pragma once

#include <app/AttributePathParams.h>
#include <app/util/basic-types.h>
#include <assert.h>
#include <lib/core/Optional.h>

namespace chip {
namespace app {
static constexpr AttributeId kRootAttributeId = 0xFFFFFFFF;

/**
* ClusterInfo is the representation of an attribute path or an event path used by ReadHandler, ReadClient, WriteHandler,
* Report::Engine etc, it uses some invalid values for representing the wildcard values for its fields and contains a mpNext field
* so it can be used as a linked list.
*/
// TODO: The cluster info should be separated into AttributeInfo and EventInfo.
// Note: The change will happen after #11171 with a better linked list.
struct ClusterInfo
{
enum class Flags : uint8_t
{
kFieldIdValid = 0x01,
kListIndexValid = 0x02,
kEventIdValid = 0x03,
};

private:
// Endpoint Id is a uint16 number, and should between 0 and 0xFFFE
static constexpr EndpointId kInvalidEndpointId = 0xFFFF;
// The ClusterId, AttributeId and EventId are MEIs,
// 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI
static constexpr ClusterId kInvalidClusterId = 0xFFFF'FFFF;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
static constexpr AttributeId kInvalidAttributeId = 0xFFFF'FFFF;
static constexpr EventId kInvalidEventId = 0xFFFF'FFFF;
// ListIndex is a uint16 number, thus 0xFFFF is not a valid list index.
static constexpr ListIndex kInvalidListIndex = 0xFFFF;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved

public:
bool IsAttributePathSupersetOf(const ClusterInfo & other) const
{
if ((other.mEndpointId != mEndpointId) || (other.mClusterId != mClusterId))
{
return false;
}

Optional<AttributeId> myFieldId =
mFlags.Has(Flags::kFieldIdValid) ? Optional<AttributeId>::Value(mFieldId) : Optional<AttributeId>::Missing();

Optional<AttributeId> otherFieldId = other.mFlags.Has(Flags::kFieldIdValid) ? Optional<AttributeId>::Value(other.mFieldId)
: Optional<AttributeId>::Missing();

Optional<ListIndex> myListIndex =
mFlags.Has(Flags::kListIndexValid) ? Optional<ListIndex>::Value(mListIndex) : Optional<ListIndex>::Missing();
VerifyOrReturnError(HasWildcardEndpointId() || mEndpointId == other.mEndpointId, false);
VerifyOrReturnError(HasWildcardClusterId() || mClusterId == other.mClusterId, false);
VerifyOrReturnError(HasWildcardAttributeId() || mFieldId == other.mFieldId, false);
VerifyOrReturnError(HasWildcardListIndex() || mListIndex == other.mListIndex, false);

Optional<ListIndex> otherListIndex = other.mFlags.Has(Flags::kListIndexValid) ? Optional<ListIndex>::Value(other.mListIndex)
: Optional<ListIndex>::Missing();

// If list index exists, field index must exist
// Field 0xFFFFFFF (any) & listindex set is invalid
assert(!(myListIndex.HasValue() && !myFieldId.HasValue()));
assert(!(otherListIndex.HasValue() && !otherFieldId.HasValue()));
assert(!(myFieldId == Optional<AttributeId>::Value(kRootAttributeId) && myListIndex.HasValue()));
assert(!(otherFieldId == Optional<AttributeId>::Value(kRootAttributeId) && otherListIndex.HasValue()));

if (myFieldId == Optional<AttributeId>::Value(kRootAttributeId))
{
return true;
}

if (myFieldId != otherFieldId)
{
return false;
}
return true;
}

// We only support top layer for attribute representation, either FieldId or FieldId + ListIndex
// Combination: if myFieldId == otherFieldId, ListIndex cannot exist without FieldId
// 1. myListIndex and otherListIndex both missing or both exactly the same, then current is superset of other
// 2. myListIndex is missing, no matter if otherListIndex is missing or not, then current is superset of other
if (myListIndex == otherListIndex)
{
// either both missing or both exactly the same
return true;
}
bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); }

if (!myListIndex.HasValue())
{
// difference is ok only if myListIndex is missing
return true;
}
/**
* Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not
* be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not
* wildcard.
*/
bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); }

return false;
}
inline bool HasWildcardNodeId() const { return mNodeId == kUndefinedNodeId; }
inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; }
inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; }
inline bool HasWildcardAttributeId() const { return mFieldId == kInvalidAttributeId; }
inline bool HasWildcardListIndex() const { return mListIndex == kInvalidListIndex; }
inline bool HasWildcardEventId() const { return mEventId == kInvalidEventId; }

ClusterInfo() {}
NodeId mNodeId = 0;
ClusterId mClusterId = 0;
ListIndex mListIndex = 0;
AttributeId mFieldId = 0;
EndpointId mEndpointId = 0;
BitFlags<Flags> mFlags;
ClusterInfo * mpNext = nullptr;
EventId mEventId = 0;
/* For better structure alignment
* Above ordering is by bit-size to ensure least amount of memory alignment padding.
* Changing order to something more natural (e.g. clusterid before nodeid) will result
/*
* For better structure alignment
* Below ordering is by bit-size to ensure least amount of memory alignment padding.
* Changing order to something more natural (e.g. endpoint id before cluster id) will result
* in extra memory alignment padding.
* uint64 mNodeId
* uint16_t mClusterId
* uint16_t mListIndex
* uint8_t FieldId
* uint8_t EndpointId
* uint8_t mDirty
* uint8_t mType
* uint32_t mpNext
* uint16_t EventId
* padding 2 bytes
*/
NodeId mNodeId = kUndefinedNodeId; // uint64
ClusterInfo * mpNext = nullptr; // pointer width (32/64 bits)
ClusterId mClusterId = kInvalidClusterId; // uint32
AttributeId mFieldId = kInvalidAttributeId; // uint32
EventId mEventId = kInvalidEventId; // uint32
ListIndex mListIndex = kInvalidListIndex; // uint16
EndpointId mEndpointId = kInvalidEndpointId; // uint16
};
} // namespace app
} // namespace chip
1 change: 1 addition & 0 deletions src/app/EventManagement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ static bool IsInterestedEventPaths(EventLoadOutContext * eventLoadOutContext, co
}
while (interestedEventPaths != nullptr)
{
// TODO: Support wildcard event path
if (interestedEventPaths->mNodeId == event.mNodeId && interestedEventPaths->mEndpointId == event.mEndpointId &&
interestedEventPaths->mClusterId == event.mClusterId && interestedEventPaths->mEventId == event.mEventId)
{
Expand Down
2 changes: 0 additions & 2 deletions src/app/InteractionModelEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,6 @@ void InteractionModelEngine::ReleaseClusterInfoList(ClusterInfo *& aClusterInfo)
{
lastClusterInfo = lastClusterInfo->mpNext;
}
lastClusterInfo->mFlags.ClearAll();
lastClusterInfo->mpNext = mpNextAvailableClusterInfo;
mpNextAvailableClusterInfo = aClusterInfo;
aClusterInfo = nullptr;
Expand Down Expand Up @@ -502,7 +501,6 @@ bool InteractionModelEngine::MergeOverlappedAttributePath(ClusterInfo * apAttrib
{
runner->mListIndex = aAttributePath.mListIndex;
runner->mFieldId = aAttributePath.mFieldId;
runner->mFlags = aAttributePath.mFlags;
return true;
}
runner = runner->mpNext;
Expand Down
15 changes: 10 additions & 5 deletions src/app/MessageDef/AttributePath.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR CheckSchemaValidity() const;
#endif
/**
* @brief Get a TLVReader for the NodeId. Next() must be called before accessing them.
* @brief Get a TLVReader for the NodeId. Next() must be called before accessing them. If the node id field does not exist, the
* nodeid will not be touched.
*
* @param [in] apNodeId A pointer to apNodeId
*
Expand All @@ -85,7 +86,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetNodeId(chip::NodeId * const apNodeId) const;

/**
* @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them.
* @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them. If the endpoint id field does not
* exist, the value will not be touched.
*
* @param [in] apEndpointId A pointer to apEndpointId
*
Expand All @@ -96,7 +98,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetEndpointId(chip::EndpointId * const apEndpointId) const;

/**
* @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them.
* @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them. If the cluster id field does not
* exist, the value will not be touched.
*
* @param [in] apClusterId A pointer to apClusterId
*
Expand All @@ -107,7 +110,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetClusterId(chip::ClusterId * const apClusterId) const;

/**
* @brief Get a TLVReader for the FieldId. Next() must be called before accessing them.
* @brief Get a TLVReader for the FieldId. Next() must be called before accessing them. If the field id field does not exist,
* the value will not be touched.
*
* @param [in] apFieldId A pointer to apFieldId
*
Expand All @@ -118,7 +122,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetFieldId(chip::AttributeId * const apFieldId) const;

/**
* @brief Get a TLVReader for the ListIndex. Next() must be called before accessing them.
* @brief Get a TLVReader for the ListIndex. Next() must be called before accessing them. If the list index field does not
* exist, the value will not be touched.
*
* @param [in] apListIndex A pointer to apListIndex
*
Expand Down
18 changes: 16 additions & 2 deletions src/app/MessageDef/Parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,37 @@ class Parser
chip::TLV::TLVType mOuterContainerType;
Parser();

/**
* Gets a unsigned integer value with the given tag, the value is not touched when the tag is not found in the TLV.
*
* @return #CHIP_NO_ERROR on success
* #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
* #CHIP_END_OF_TLV if there is no such element
*/
template <typename T>
CHIP_ERROR GetUnsignedInteger(const uint8_t aContextTag, T * const apLValue) const
{
return GetSimpleValue(aContextTag, chip::TLV::kTLVType_UnsignedInteger, apLValue);
};

/**
* Gets a scalar value with the given tag, the value is not touched when the tag is not found in the TLV.
*
* @return #CHIP_NO_ERROR on success
* #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
* #CHIP_END_OF_TLV if there is no such element
*/
template <typename T>
CHIP_ERROR GetSimpleValue(const uint8_t aContextTag, const chip::TLV::TLVType aTLVType, T * const apLValue) const
{
CHIP_ERROR err = CHIP_NO_ERROR;
chip::TLV::TLVReader reader;

*apLValue = 0;

err = mReader.FindElementWithTag(chip::TLV::ContextTag(aContextTag), reader);
SuccessOrExit(err);

*apLValue = 0;

VerifyOrExit(aTLVType == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);

err = reader.Get(*apLValue);
Expand Down
38 changes: 18 additions & 20 deletions src/app/ReadClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,39 +473,37 @@ CHIP_ERROR ReadClient::ProcessAttributeDataList(TLV::TLVReader & aAttributeDataL
SuccessOrExit(err);

err = element.GetAttributePath(&attributePathParser);
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

// We are using the feature that the parser won't touch the value if the field does not exist, since all fields in the
// cluster info will be invalid / wildcard, it is safe ignore CHIP_END_OF_TLV directly.

err = attributePathParser.GetNodeId(&(clusterInfo.mNodeId));
SuccessOrExit(err);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
}
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

// The ReportData must contain a concrete attribute path

err = attributePathParser.GetEndpointId(&(clusterInfo.mEndpointId));
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetClusterId(&(clusterInfo.mClusterId));
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetFieldId(&(clusterInfo.mFieldId));
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
if (CHIP_NO_ERROR == err)
{
clusterInfo.mFlags.Set(ClusterInfo::Flags::kFieldIdValid);
}
else if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetListIndex(&(clusterInfo.mListIndex));
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
if (CHIP_NO_ERROR == err)
{
VerifyOrExit(clusterInfo.mFlags.Has(ClusterInfo::Flags::kFieldIdValid), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
clusterInfo.mFlags.Set(ClusterInfo::Flags::kListIndexValid);
}
else if (CHIP_END_OF_TLV == err)
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
}
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

VerifyOrExit(clusterInfo.IsValidAttributePath(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = element.GetData(&dataReader);
if (err == CHIP_END_OF_TLV)
Expand Down
32 changes: 19 additions & 13 deletions src/app/ReadHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,28 +335,38 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathList::Parser & aAt
AttributePath::Parser path;
err = path.Init(reader);
SuccessOrExit(err);
err = path.GetNodeId(&(clusterInfo.mNodeId));
SuccessOrExit(err);
// TODO: Support wildcard paths here
// TODO: MEIs (ClusterId and AttributeId) have a invalid pattern instead of a single invalid value, need to add separate
// functions for checking if we have received valid values.
err = path.GetEndpointId(&(clusterInfo.mEndpointId));
if (err == CHIP_NO_ERROR)
{
VerifyOrExit(!clusterInfo.HasWildcardEndpointId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
}
SuccessOrExit(err);
err = path.GetClusterId(&(clusterInfo.mClusterId));
if (err == CHIP_NO_ERROR)
{
VerifyOrExit(!clusterInfo.HasWildcardClusterId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
}

SuccessOrExit(err);
err = path.GetFieldId(&(clusterInfo.mFieldId));
if (CHIP_NO_ERROR == err)
if (CHIP_END_OF_TLV == err)
{
clusterInfo.mFlags.Set(ClusterInfo::Flags::kFieldIdValid);
err = CHIP_NO_ERROR;
erjiaqing marked this conversation as resolved.
Show resolved Hide resolved
}
else if (CHIP_END_OF_TLV == err)
else if (err == CHIP_NO_ERROR)
{
err = CHIP_NO_ERROR;
VerifyOrExit(!clusterInfo.HasWildcardAttributeId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
}
SuccessOrExit(err);

err = path.GetListIndex(&(clusterInfo.mListIndex));
if (CHIP_NO_ERROR == err)
{
VerifyOrExit(clusterInfo.mFlags.Has(ClusterInfo::Flags::kFieldIdValid), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
clusterInfo.mFlags.Set(ClusterInfo::Flags::kListIndexValid);
VerifyOrExit(!clusterInfo.HasWildcardAttributeId() && !clusterInfo.HasWildcardListIndex(),
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
}
else if (CHIP_END_OF_TLV == err)
{
Expand Down Expand Up @@ -398,11 +408,7 @@ CHIP_ERROR ReadHandler::ProcessEventPaths(EventPaths::Parser & aEventPathsParser
err = path.GetCluster(&(clusterInfo.mClusterId));
SuccessOrExit(err);
err = path.GetEvent(&(clusterInfo.mEventId));
if (CHIP_NO_ERROR == err)
{
clusterInfo.mFlags.Set(ClusterInfo::Flags::kEventIdValid);
}
else if (CHIP_END_OF_TLV == err)
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
}
Expand Down
Loading