Skip to content

Commit

Permalink
[IM] Fix verious corner cases for event chunking
Browse files Browse the repository at this point in the history
  • Loading branch information
@erjiaqing
erjiaqing committed Mar 17, 2022
1 parent d80d593 commit c8f5707
Show file tree
Hide file tree
Showing 6 changed files with 368 additions and 37 deletions.
11 changes: 10 additions & 1 deletion src/app/EventManagement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,16 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *
}

exit:
aEventMin = context.mCurrentEventNumber + 1;
if (err == CHIP_ERROR_BUFFER_TOO_SMALL || err == CHIP_ERROR_NO_MEMORY)
{
// We failed to fetch the current event because the buffer is too small, we will start from this one the next time.
aEventMin = context.mCurrentEventNumber;
}
else
{
// For all other cases, continue from the next event.
aEventMin = context.mCurrentEventNumber + 1;
}
aEventCount += context.mEventCount;
return err;
}
Expand Down
36 changes: 15 additions & 21 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,8 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
CHIP_ERROR err = CHIP_NO_ERROR;
ReportDataMessage::Parser report;

bool isEventReportsPresent = false;
bool isAttributeReportIBsPresent = false;
bool suppressResponse = true;
uint64_t subscriptionId = 0;
bool suppressResponse = true;
uint64_t subscriptionId = 0;
EventReportIBs::Parser eventReportIBs;
AttributeReportIBs::Parser attributeReportIBs;
System::PacketBufferTLVReader reader;
Expand Down Expand Up @@ -474,33 +472,28 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
}
SuccessOrExit(err);

err = report.GetEventReports(&eventReportIBs);
isEventReportsPresent = (err == CHIP_NO_ERROR);
err = report.GetEventReports(&eventReportIBs);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);

if (isEventReportsPresent)
else if (err == CHIP_NO_ERROR)
{
chip::TLV::TLVReader EventReportsReader;
eventReportIBs.GetReader(&EventReportsReader);
err = ProcessEventReportIBs(EventReportsReader);
SuccessOrExit(err);
}
SuccessOrExit(err);

err = report.GetAttributeReportIBs(&attributeReportIBs);
isAttributeReportIBsPresent = (err == CHIP_NO_ERROR);
err = report.GetAttributeReportIBs(&attributeReportIBs);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);

if (isAttributeReportIBsPresent)
else if (err == CHIP_NO_ERROR)
{
TLV::TLVReader attributeReportIBsReader;
mIsAttributeReportIBsPresent = true;
attributeReportIBs.GetReader(&attributeReportIBsReader);

if (mIsInitialReport)
Expand All @@ -510,13 +503,14 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
}

err = ProcessAttributeReportIBs(attributeReportIBsReader);
SuccessOrExit(err);
}
SuccessOrExit(err);

if (!mPendingMoreChunks)
{
mpCallback.OnReportEnd();
mIsInitialReport = true;
}
if (mIsAttributeReportIBsPresent && !mPendingMoreChunks)
{
mpCallback.OnReportEnd();
mIsInitialReport = true;
mIsAttributeReportIBsPresent = false;
}

SuccessOrExit(err = report.ExitContainer());
Expand Down
3 changes: 2 additions & 1 deletion src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ class ReadClient : public Messaging::ExchangeDelegate
FabricIndex mFabricIndex = kUndefinedFabricIndex;
InteractionType mInteractionType = InteractionType::Read;
Timestamp mEventTimestamp;
EventNumber mEventMin = 0;
EventNumber mEventMin = 0;
bool mIsAttributeReportIBsPresent = false;

ReadClient * mpNext = nullptr;
InteractionModelEngine * mpImEngine = nullptr;
Expand Down
29 changes: 21 additions & 8 deletions src/app/reporting/Engine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
if (!attributeDataWritten && err == CHIP_NO_ERROR)
{
aReportDataBuilder.Rollback(backup);
aReportDataBuilder.ResetError();
}

// hasMoreChunks + no data encoded is a flag that we have encountered some trouble when processing the attribute.
Expand All @@ -261,7 +262,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
}

CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder & aReportDataBuilder, ReadHandler * apReadHandler,
bool * apHasMoreChunks, bool * apHasEncodedData)
bool aBufferIsUsed, bool * apHasMoreChunks, bool * apHasEncodedData)
{
CHIP_ERROR err = CHIP_NO_ERROR;
size_t eventCount = 0;
Expand Down Expand Up @@ -289,8 +290,11 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder
}

{
EventReportIBs::Builder & eventReportIBs = aReportDataBuilder.CreateEventReports();
const uint32_t kReservedSizeEndOfReportIBs = 1;
EventReportIBs::Builder & eventReportIBs = aReportDataBuilder.CreateEventReports();
SuccessOrExit(err = aReportDataBuilder.GetError());
VerifyOrExit(eventReportIBs.GetWriter() != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
SuccessOrExit(err = eventReportIBs.GetWriter()->ReserveBuffer(kReservedSizeEndOfReportIBs));
err = eventManager.FetchEventsSince(*(eventReportIBs.GetWriter()), clusterInfoList, eventMin, eventCount,
apReadHandler->GetSubjectDescriptor());

Expand All @@ -302,9 +306,9 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder
else if ((err == CHIP_ERROR_BUFFER_TOO_SMALL) || (err == CHIP_ERROR_NO_MEMORY))
{
// when first cluster event is too big to fit in the packet, ignore that cluster event.
if (eventCount == 0)
// However, we may have encoded some attributes befure, we don't skip it in that case.
if (eventCount == 0 && !aBufferIsUsed)
{
eventMin++;
ChipLogDetail(DataManagement, "<RE:Run> first cluster event is too big so that it fails to fit in the packet!");
err = CHIP_NO_ERROR;
}
Expand All @@ -327,6 +331,7 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder
ExitNow();
}

SuccessOrExit(err = eventReportIBs.GetWriter()->UnreserveBuffer(kReservedSizeEndOfReportIBs));
eventReportIBs.EndOfEventReports();
SuccessOrExit(err = eventReportIBs.GetError());
}
Expand All @@ -338,9 +343,13 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder
*apHasEncodedData = !(eventCount == 0 || eventClean);
}

if (err == CHIP_NO_ERROR && (eventCount == 0 || eventClean))
// Maybe encoding the attributes have already used up all space.
if ((err == CHIP_NO_ERROR || err == CHIP_ERROR_NO_MEMORY || err == CHIP_ERROR_BUFFER_TOO_SMALL) &&
(eventCount == 0 || eventClean))
{
aReportDataBuilder.Rollback(backup);
aReportDataBuilder.ResetError();
err = CHIP_NO_ERROR;
}

// hasMoreChunks + no data encoded is a flag that we have encountered some trouble when processing the attribute.
Expand Down Expand Up @@ -373,6 +382,9 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
// Reserved size for the end of report message, which is an end-of-container (i.e 1 byte for the control tag).
const uint32_t kReservedSizeForEndOfReportMessage = 1;

// Reserved size for an empty EventReportIBs, so we can at least check if there are any events need to be reported.
const uint32_t kReservedSizeForEventReportIBs = 3; // type, tag, end of container

VerifyOrExit(apReadHandler != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(!bufHandle.IsNull(), err = CHIP_ERROR_NO_MEMORY);

Expand Down Expand Up @@ -404,7 +416,7 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
}

SuccessOrExit(err = reportDataWriter.ReserveBuffer(kReservedSizeForMoreChunksFlag + kReservedSizeForIMRevision +
kReservedSizeForEndOfReportMessage));
kReservedSizeForEndOfReportMessage + kReservedSizeForEventReportIBs));

{
bool hasMoreChunksForAttributes = false;
Expand All @@ -415,8 +427,9 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
err = BuildSingleReportDataAttributeReportIBs(reportDataBuilder, apReadHandler, &hasMoreChunksForAttributes,
&hasEncodedAttributes);
SuccessOrExit(err);

err = BuildSingleReportDataEventReports(reportDataBuilder, apReadHandler, &hasMoreChunksForEvents, &hasEncodedEvents);
SuccessOrExit(err = reportDataWriter.UnreserveBuffer(kReservedSizeForEventReportIBs));
err = BuildSingleReportDataEventReports(reportDataBuilder, apReadHandler, hasEncodedAttributes, &hasMoreChunksForEvents,
&hasEncodedEvents);
SuccessOrExit(err);

hasMoreChunks = hasMoreChunksForAttributes || hasMoreChunksForEvents;
Expand Down
2 changes: 1 addition & 1 deletion src/app/reporting/Engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class Engine
CHIP_ERROR BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Builder & reportDataBuilder, ReadHandler * apReadHandler,
bool * apHasMoreChunks, bool * apHasEncodedData);
CHIP_ERROR BuildSingleReportDataEventReports(ReportDataMessage::Builder & reportDataBuilder, ReadHandler * apReadHandler,
bool * apHasMoreChunks, bool * apHasEncodedData);
bool aBufferIsUsed, bool * apHasMoreChunks, bool * apHasEncodedData);
CHIP_ERROR RetrieveClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
AttributeReportIBs::Builder & aAttributeReportIBs,
const ConcreteReadAttributePath & aClusterInfo,
Expand Down
Loading

0 comments on commit c8f5707

Please sign in to comment.