Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yunhanw-google
yunhanw-google committed Nov 13, 2021
1 parent 9dd537d commit c1547ee
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 195 deletions.
3 changes: 1 addition & 2 deletions src/app/ClusterInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ struct ClusterInfo
private:
// Allow AttributePathParams access these constants.
friend struct AttributePathParams;
friend struct ConcreteEventPath;

// The ClusterId, AttributeId and EventId are MEIs,
// 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI
Expand Down Expand Up @@ -66,8 +67,6 @@ struct ClusterInfo
*/
bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); }

bool IsValidEventPath() const { return !HasWildcardEventId(); }

inline bool HasWildcardNodeId() const { return mNodeId == kUndefinedNodeId; }
inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; }
inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; }
Expand Down
5 changes: 5 additions & 0 deletions src/app/ConcreteEventPath.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#pragma once

#include <app/ClusterInfo.h>
#include <app/util/basic-types.h>

namespace chip {
Expand Down Expand Up @@ -50,6 +51,10 @@ struct ConcreteEventPath
return mEndpointId == other.mEndpointId && mClusterId == other.mClusterId && mEventId == other.mEventId;
}

bool IsValidEventPath() const { return !HasWildcardEventId(); }

inline bool HasWildcardEventId() const { return mEventId == ClusterInfo::kInvalidEventId; }

EndpointId mEndpointId = 0;
ClusterId mClusterId = 0;
EventId mEventId = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/app/DeviceControllerInteractionModelDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class DeviceControllerInteractionModelDelegate : public chip::app::ReadClient::C
void OnDone(app::WriteClient * apWriteClient) override {}

void OnEventData(const app::ReadClient * apReadClient, const app::EventHeader & aEventHeader, TLV::TLVReader * apData,
const app::StatusIB & aStatus) override
const app::StatusIB * apStatus) override
{}

void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, TLV::TLVReader * apData,
Expand Down
1 change: 0 additions & 1 deletion src/app/EventHeader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ struct EventHeader
EventNumber mEventNumber = 0;
PriorityLevel mPriorityLevel = PriorityLevel::Invalid;
Timestamp mTimestamp;
Timestamp mDeltaTimestamp;
};
} // namespace app
} // namespace chip
4 changes: 4 additions & 0 deletions src/app/EventLoggingTypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ struct Timestamp
return timestamp;
}

bool IsValid() { return mType != Type::kInvalid; }
bool IsSystem() { return mType == Type::kSystem; }
bool IsEpoch() { return mType == Type::kEpoch; }

Type mType = Type::kInvalid;
uint64_t mValue = 0;
};
Expand Down
46 changes: 2 additions & 44 deletions src/app/EventManagement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,6 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even
EventReportIB::Builder eventReportBuilder;
EventDataIB::Builder eventDataIBBuilder;
EventPathIB::Builder eventPathBuilder;
EventStatusIB::Builder eventStatusIBBuilder;
StatusIB::Builder statusIBBuilder;
StatusIB status;
uint64_t deltatime = 0;

VerifyOrExit(apContext->mCurrentEventNumber >= apContext->mStartingEventNumber,
Expand All @@ -318,26 +315,6 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even
eventReportBuilder.Init(&(apContext->mWriter));
// TODO: Update IsUrgent, issue 11386
// TODO: Update statusIB, issue 11388
eventStatusIBBuilder = eventReportBuilder.CreateEventStatus();
eventPathBuilder = eventStatusIBBuilder.CreatePath();
err = eventStatusIBBuilder.GetError();
SuccessOrExit(err);
eventPathBuilder.Node(apOptions->mpEventSchema->mNodeId)
.Endpoint(apOptions->mpEventSchema->mEndpointId)
.Cluster(apOptions->mpEventSchema->mClusterId)
.Event(apOptions->mpEventSchema->mEventId)
.IsUrgent(false)
.EndOfEventPathIB();
err = eventPathBuilder.GetError();
SuccessOrExit(err);
statusIBBuilder = eventStatusIBBuilder.CreateErrorStatus();
err = eventStatusIBBuilder.GetError();
SuccessOrExit(err);
statusIBBuilder.EncodeStatusIB(status);
eventStatusIBBuilder.EndOfEventStatusIB();
err = statusIBBuilder.GetError();
SuccessOrExit(err);

eventDataIBBuilder = eventReportBuilder.CreateEventData();
eventPathBuilder = eventDataIBBuilder.CreatePath();
err = eventDataIBBuilder.GetError();
Expand Down Expand Up @@ -621,23 +598,6 @@ CHIP_ERROR EventManagement::CopyEvent(const TLVReader & aReader, TLVWriter & aWr
ReturnErrorOnFailure(reader.EnterContainer(containerType));
ReturnErrorOnFailure(aWriter.StartContainer(AnonymousTag, kTLVType_Structure, containerType));

ReturnErrorOnFailure(reader.Next());
ReturnErrorOnFailure(reader.EnterContainer(containerType1));
ReturnErrorOnFailure(aWriter.StartContainer(TLV::ContextTag(to_underlying(EventReportIB::Tag::kEventStatus)),
kTLVType_Structure, containerType1));
ReturnErrorOnFailure(reader.Next());
do
{
ReturnErrorOnFailure(aWriter.CopyElement(reader));
} while (CHIP_NO_ERROR == (err = reader.Next()));
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
ReturnErrorOnFailure(err);
ReturnErrorOnFailure(reader.ExitContainer(containerType1));
ReturnErrorOnFailure(aWriter.EndContainer(containerType1));

ReturnErrorOnFailure(reader.Next());
ReturnErrorOnFailure(reader.EnterContainer(containerType1));
ReturnErrorOnFailure(
Expand Down Expand Up @@ -685,8 +645,7 @@ CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDep
innerReader.Init(aReader);
ReturnErrorOnFailure(innerReader.EnterContainer(tlvType));
ReturnErrorOnFailure(innerReader.Next());
// Skip EventStatus Element
ReturnErrorOnFailure(innerReader.Next());

ReturnErrorOnFailure(innerReader.EnterContainer(tlvType1));
err = TLV::Utilities::Iterate(innerReader, FetchEventParameters, &event, false /*recurse*/);
if (event.mFieldsToRead != kRequiredEventField)
Expand Down Expand Up @@ -836,8 +795,7 @@ CHIP_ERROR EventManagement::EvictEvent(CHIPCircularTLVBuffer & apBuffer, void *
TLVType containerType1;
ReturnErrorOnFailure(aReader.EnterContainer(containerType));
ReturnErrorOnFailure(aReader.Next());
// Skip EventStatus
ReturnErrorOnFailure(aReader.Next());

ReturnErrorOnFailure(aReader.EnterContainer(containerType1));
EventEnvelopeContext context;
constexpr bool recurse = false;
Expand Down
97 changes: 97 additions & 0 deletions src/app/MessageDef/EventDataIB.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,103 @@ CHIP_ERROR EventDataIB::Parser::GetData(TLV::TLVReader * const apReader) const
return mReader.FindElementWithTag(TLV::ContextTag(to_underlying(Tag::kData)), *apReader);
}

CHIP_ERROR EventDataIB::Parser::ProcessEventPath(EventPathIB::Parser & aEventPath, ConcreteEventPath & aConcreteEventPath)
{
// The ReportData must contain a concrete event path
CHIP_ERROR err = aEventPath.GetEndpoint(&(aConcreteEventPath.mEndpointId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH);

err = aEventPath.GetCluster(&(aConcreteEventPath.mClusterId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH);

err = aEventPath.GetEvent(&(aConcreteEventPath.mEventId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH);

VerifyOrReturnError(aConcreteEventPath.IsValidEventPath(), CHIP_ERROR_IM_MALFORMED_EVENT_PATH);
return CHIP_NO_ERROR;
}

CHIP_ERROR EventDataIB::Parser::ProcessEventTimestamp(EventHeader & aEventHeader)
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint64_t timeStampVal = 0;
bool hasSystemTimestamp = false;
bool hasEpochTimestamp = false;
bool hasDeltaSystemTimestamp = false;
bool hasDeltaEpochTimestamp = false;
err = GetDeltaSystemTimestamp(&timeStampVal);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
else if (err == CHIP_NO_ERROR)
{
VerifyOrReturnError(aEventHeader.mTimestamp.IsSystem(), CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT);
aEventHeader.mTimestamp.mValue += timeStampVal;
hasDeltaSystemTimestamp = true;
}
ReturnErrorOnFailure(err);

err = GetDeltaEpochTimestamp(&timeStampVal);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
else if (err == CHIP_NO_ERROR)
{
aEventHeader.mTimestamp.mValue += timeStampVal;
hasDeltaEpochTimestamp = true;
}
ReturnErrorOnFailure(err);

err = GetSystemTimestamp(&timeStampVal);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
else if (err == CHIP_NO_ERROR)
{
aEventHeader.mTimestamp.mType = Timestamp::Type::kSystem;
aEventHeader.mTimestamp.mValue = timeStampVal;
hasSystemTimestamp = true;
}
ReturnErrorOnFailure(err);

err = GetEpochTimestamp(&timeStampVal);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
else if (err == CHIP_NO_ERROR)
{
aEventHeader.mTimestamp.mType = Timestamp::Type::kEpoch;
aEventHeader.mTimestamp.mValue = timeStampVal;
hasEpochTimestamp = true;
}

if ((hasSystemTimestamp && !hasEpochTimestamp && !hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) ||
(!hasSystemTimestamp && hasEpochTimestamp && !hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) ||
(!hasSystemTimestamp && !hasEpochTimestamp && hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) ||
(!hasSystemTimestamp && !hasEpochTimestamp && !hasDeltaSystemTimestamp && hasDeltaEpochTimestamp))
{
return CHIP_NO_ERROR;
}
return CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT;
}

CHIP_ERROR EventDataIB::Parser::DecodeEventHeader(EventHeader & aEventHeader)
{
uint8_t priorityLevel = 0;
EventPathIB::Parser path;
ReturnErrorOnFailure(GetPath(&path));
ReturnErrorOnFailure(ProcessEventPath(path, aEventHeader.mPath));
ReturnErrorOnFailure(GetEventNumber(&(aEventHeader.mEventNumber)));
ReturnErrorOnFailure(GetPriority(&priorityLevel));
aEventHeader.mPriorityLevel = static_cast<PriorityLevel>(priorityLevel);
ReturnErrorOnFailure(ProcessEventTimestamp(aEventHeader));
return CHIP_NO_ERROR;
}

EventPathIB::Builder & EventDataIB::Builder::CreatePath()
{
// skip if error has already been set
Expand Down
6 changes: 6 additions & 0 deletions src/app/MessageDef/EventDataIB.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "StructBuilder.h"
#include "StructParser.h"
#include <app/AppBuildConfig.h>
#include <app/EventHeader.h>
#include <app/EventLoggingTypes.h>
#include <app/util/basic-types.h>
#include <lib/core/CHIPCore.h>
Expand Down Expand Up @@ -156,9 +157,14 @@ class Parser : public StructParser
*/
CHIP_ERROR GetData(TLV::TLVReader * const apReader) const;

CHIP_ERROR DecodeEventHeader(EventHeader & aEventHeader);

protected:
// A recursively callable function to parse a data element and pretty-print it.
CHIP_ERROR ParseData(TLV::TLVReader & aReader, int aDepth) const;

CHIP_ERROR ProcessEventPath(EventPathIB::Parser & aEventPath, ConcreteEventPath & aConcreteEventPath);
CHIP_ERROR ProcessEventTimestamp(EventHeader & aEventHeader);
};

class Builder : public StructBuilder
Expand Down
16 changes: 14 additions & 2 deletions src/app/MessageDef/EventReportIB.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,21 @@ CHIP_ERROR EventReportIB::Parser::CheckSchemaValidity() const

if (CHIP_END_OF_TLV == err)
{
const int RequiredFields = (1 << to_underlying(Tag::kEventData)) | (1 << to_underlying(Tag::kEventStatus));
// check for at most field:
const int CheckDataField = 1 << to_underlying(Tag::kEventData);
const int CheckStatusField = (1 << to_underlying(Tag::kEventStatus));

if ((TagPresenceMask & RequiredFields) == RequiredFields)
if ((TagPresenceMask & CheckDataField) == CheckDataField && (TagPresenceMask & CheckStatusField) == CheckStatusField)
{
// kEventData and kEventStatus both exist
err = CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB;
}
else if ((TagPresenceMask & CheckDataField) != CheckDataField && (TagPresenceMask & CheckStatusField) != CheckStatusField)
{
// kEventData and kErrorStatus not exist
err = CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB;
}
else
{
err = CHIP_NO_ERROR;
}
Expand Down
Loading

0 comments on commit c1547ee

Please sign in to comment.