Skip to content

Commit

Permalink
Add IM read event e2e test and fix the found bug (#8338)
Browse files Browse the repository at this point in the history
Summary of Changes:
-- When generating report, we read event from debug priority to critical
priority, we should read event from cirtical priority to debug priority.
We add test to check the initial read events are from critical priority
via providing critical and info event. Without this fix, the test would
fail.
-- During test, we see current report engine fail to handle event-only
read request where it inserts empty attribute list, we rollback the
empty eventlist if no attribute is there, without this fix, the test
would fail
-- Fix event number calculation and event number test.
  • Loading branch information
yunhanw-google authored and pull[bot] committed Sep 14, 2021
1 parent 330f243 commit 3262632
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 92 deletions.
1 change: 1 addition & 0 deletions src/app/EventLoggingTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ struct EventLoadOutContext
Timestamp mPreviousSystemTime;
Timestamp mCurrentSystemTime;
EventNumber mCurrentEventNumber = 0;
size_t mEventCount = 0;
Timestamp mCurrentUTCTime;
ClusterInfo * mpInterestedEventPaths = nullptr;
bool mFirst = true;
Expand Down
5 changes: 3 additions & 2 deletions src/app/EventManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,13 +690,14 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD

loadOutContext->mPreviousSystemTime.mValue = loadOutContext->mCurrentSystemTime.mValue;
loadOutContext->mFirst = false;
loadOutContext->mEventCount++;
}

return err;
}

CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo * apClusterInfolist, PriorityLevel aPriority,
EventNumber & aEventNumber)
EventNumber & aEventNumber, size_t & aEventCount)
{
// TODO: Add particular set of event Paths in FetchEventsSince so that we can filter the interested paths
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -729,7 +730,7 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *

exit:
aEventNumber = context.mCurrentEventNumber;

aEventCount += context.mEventCount;
return err;
}

Expand Down
3 changes: 2 additions & 1 deletion src/app/EventManagement.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ class EventManagement
* completion, the event number of the last event
* fetched.
*
* @param[out] aEventCount The number of fetched event
* @retval #CHIP_END_OF_TLV The function has reached the end of the
* available log entries at the specified
* priority level
Expand All @@ -395,7 +396,7 @@ class EventManagement
*
*/
CHIP_ERROR FetchEventsSince(chip::TLV::TLVWriter & aWriter, ClusterInfo * apClusterInfolist, PriorityLevel aPriority,
EventNumber & aEventNumber);
EventNumber & aEventNumber, size_t & aEventCount);

/**
* @brief
Expand Down
9 changes: 5 additions & 4 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,15 +303,16 @@ bool ReadHandler::CheckEventClean(EventManagement & aEventManager)
{
if (mCurrentPriority == PriorityLevel::Invalid)
{
// Upload is not in middle, previous mLastScheduledEventNumber is not valid, Check for new events, and set a checkpoint
for (size_t index = 0; index < ArraySize(mSelfProcessedEvents); index++)
// Upload is not in middle, previous mLastScheduledEventNumber is not valid, Check for new events from Critical high
// priority to Debug low priority, and set a checkpoint when there is dirty events
for (int index = ArraySize(mSelfProcessedEvents) - 1; index >= 0; index--)
{
EventNumber lastEventNumber = aEventManager.GetLastEventNumber(static_cast<PriorityLevel>(index));
if ((lastEventNumber != 0) && (lastEventNumber >= mSelfProcessedEvents[index]))
{
// We have more events. snapshot last event IDs
aEventManager.SetScheduledEventEndpoint(&(mLastScheduledEventNumber[0]));
// initialize the next priority level to transfer
// initialize the next dirty priority level to transfer
MoveToNextScheduledDirtyPriority();
return false;
}
Expand All @@ -329,7 +330,7 @@ bool ReadHandler::CheckEventClean(EventManagement & aEventManager)

void ReadHandler::MoveToNextScheduledDirtyPriority()
{
for (uint8_t i = 0; i < ArraySize(mSelfProcessedEvents); i++)
for (int i = ArraySize(mSelfProcessedEvents) - 1; i >= 0; i--)
{
if ((mLastScheduledEventNumber[i] != 0) && mSelfProcessedEvents[i] <= mLastScheduledEventNumber[i])
{
Expand Down
3 changes: 2 additions & 1 deletion src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ class ReadHandler
// sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again.
bool CheckEventClean(EventManagement & aEventManager);

// Move to the next dirty priority where last schedule event number is larger than current self vended event number
// Move to the next dirty priority from critical high priority to debug low priority, where last schedule event number
// is larger than current self vended event number
void MoveToNextScheduledDirtyPriority();

private:
Expand Down
39 changes: 21 additions & 18 deletions src/app/reporting/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,15 @@ Engine::RetrieveClusterData(AttributeDataElement::Builder & aAttributeDataElemen
return err;
}

CHIP_ERROR Engine::BuildSingleReportDataAttributeDataList(ReportData::Builder & reportDataBuilder, ReadHandler * apReadHandler)
CHIP_ERROR Engine::BuildSingleReportDataAttributeDataList(ReportData::Builder & aReportDataBuilder, ReadHandler * apReadHandler)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ClusterInfo * clusterInfo = apReadHandler->GetAttributeClusterInfolist();
AttributeDataList::Builder attributeDataList = reportDataBuilder.CreateAttributeDataListBuilder();
SuccessOrExit(err = reportDataBuilder.GetError());
CHIP_ERROR err = CHIP_NO_ERROR;
ClusterInfo * clusterInfo = apReadHandler->GetAttributeClusterInfolist();
bool attributeClean = true;
TLV::TLVWriter backup;
aReportDataBuilder.Checkpoint(backup);
AttributeDataList::Builder attributeDataList = aReportDataBuilder.CreateAttributeDataListBuilder();
SuccessOrExit(err = aReportDataBuilder.GetError());
// TODO: Need to handle multiple chunk of message
while (clusterInfo != nullptr)
{
Expand All @@ -101,6 +104,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeDataList(ReportData::Builder &
err = RetrieveClusterData(attributeDataElementBuilder, *clusterInfo);
VerifyOrExit(err == CHIP_NO_ERROR,
ChipLogError(DataManagement, "<RE:Run> Error retrieving data from cluster, aborting"));
attributeClean = false;
}

clusterInfo = clusterInfo->mpNext;
Expand All @@ -109,14 +113,18 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeDataList(ReportData::Builder &
err = attributeDataList.GetError();

exit:
if (attributeClean || err != CHIP_NO_ERROR)
{
aReportDataBuilder.Rollback(backup);
}
ChipLogFunctError(err);
return err;
}

CHIP_ERROR Engine::BuildSingleReportDataEventList(ReportData::Builder & aReportDataBuilder, ReadHandler * apReadHandler)
{
CHIP_ERROR err = CHIP_NO_ERROR;
EventNumber eventCount = 0;
CHIP_ERROR err = CHIP_NO_ERROR;
size_t eventCount = 0;
TLV::TLVWriter backup;
bool eventClean = true;
EventNumber initialEvents[kNumPriorityLevel];
Expand All @@ -125,10 +133,9 @@ CHIP_ERROR Engine::BuildSingleReportDataEventList(ReportData::Builder & aReportD
EventManagement & eventManager = EventManagement::GetInstance();
EventList::Builder eventList;

VerifyOrExit(clusterInfoList != nullptr, );

aReportDataBuilder.Checkpoint(backup);

VerifyOrExit(clusterInfoList != nullptr, );
VerifyOrExit(apReadHandler != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);

eventList = aReportDataBuilder.CreateEventDataListBuilder();
Expand All @@ -153,15 +160,14 @@ CHIP_ERROR Engine::BuildSingleReportDataEventList(ReportData::Builder & aReportD
// proceed only if there are new events.
if (eventClean)
{
aReportDataBuilder.Rollback(backup);
ExitNow(); // Read clean, move along
}

while (apReadHandler->GetCurrentPriority() != PriorityLevel::Invalid)
{
uint8_t priorityIndex = static_cast<uint8_t>(apReadHandler->GetCurrentPriority());
err = eventManager.FetchEventsSince(*(eventList.GetWriter()), clusterInfoList, apReadHandler->GetCurrentPriority(),
eventNumberList[priorityIndex]);
eventNumberList[priorityIndex], eventCount);

if ((err == CHIP_END_OF_TLV) || (err == CHIP_ERROR_TLV_UNDERRUN) || (err == CHIP_NO_ERROR))
{
Expand All @@ -174,8 +180,6 @@ CHIP_ERROR Engine::BuildSingleReportDataEventList(ReportData::Builder & aReportD
}
else if ((err == CHIP_ERROR_BUFFER_TOO_SMALL) || (err == CHIP_ERROR_NO_MEMORY))
{
eventCount = CountEvents(apReadHandler, initialEvents);

// when first cluster event is too big to fit in the packet, ignore that cluster event.
if (eventCount == 0)
{
Expand Down Expand Up @@ -210,15 +214,14 @@ CHIP_ERROR Engine::BuildSingleReportDataEventList(ReportData::Builder & aReportD
eventList.EndOfEventList();
SuccessOrExit(err = eventList.GetError());

eventCount = CountEvents(apReadHandler, initialEvents);
ChipLogDetail(DataManagement, "Fetched 0x" ChipLogFormatX64 " events", ChipLogValueX64(eventCount));
ChipLogDetail(DataManagement, "Fetched %zu events", eventCount);

exit:
if (err != CHIP_NO_ERROR)
if (err != CHIP_NO_ERROR || eventCount == 0 || eventClean)
{
ChipLogError(DataManagement, "Error retrieving events, err = %" CHIP_ERROR_FORMAT, ChipError::FormatError(err));
aReportDataBuilder.Rollback(backup);
}

ChipLogFunctError(err);
return err;
}

Expand Down
9 changes: 5 additions & 4 deletions src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,18 @@ static void CheckLogReadOut(nlTestSuite * apSuite, chip::app::EventManagement &
CHIP_ERROR err;
chip::TLV::TLVReader reader;
chip::TLV::TLVWriter writer;
size_t eventCount = 0;
uint8_t backingStore[1024];
size_t elementCount;
size_t totalNumElements;
writer.Init(backingStore, 1024);
err = alogMgmt.FetchEventsSince(writer, clusterInfo, priority, startingEventNumber);
err = alogMgmt.FetchEventsSince(writer, clusterInfo, priority, startingEventNumber, eventCount);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR || err == CHIP_END_OF_TLV);

reader.Init(backingStore, writer.GetLengthWritten());

err = chip::TLV::Utilities::Count(reader, elementCount, false);
err = chip::TLV::Utilities::Count(reader, totalNumElements, false);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(apSuite, elementCount == expectedNumEvents);
NL_TEST_ASSERT(apSuite, totalNumElements == expectedNumEvents && totalNumElements == eventCount);
reader.Init(backingStore, writer.GetLengthWritten());
chip::TLV::Debug::Dump(reader, SimpleDumpWriter);
}
Expand Down
Loading

0 comments on commit 3262632

Please sign in to comment.