Skip to content

Commit

Permalink
Improve encoding of lists where first item can't fit in packet.
Browse files Browse the repository at this point in the history
When the first item of a list can't fit in the current packet, it's better to
just send the packet without the list at all, then encode as many items as we
can in the packet that follows before we start doing IB-per-item.

Otherwise (before this change) we encode an empty list IB, then end up doing
IB-per-item for the whole list.
  • Loading branch information
bzbarsky-apple committed Jul 27, 2023
1 parent 0da4af6 commit 351dc16
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
13 changes: 13 additions & 0 deletions src/app/AttributeAccessInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ void AttributeValueEncoder::EnsureListEnded()

AttributeReportBuilder builder;
VerifyOrDie(builder.FinishAttribute(mAttributeReportIBsBuilder) == CHIP_NO_ERROR);

if (!mEncodedAtLeastOneListItem)
{
// If we have not managed to encode any list items, we don't actually
// want to output the single "empty list" IB that will then be followed
// by one-IB-per-item in the next packet. Just have the reporting
// engine roll back our entire attribute and put us in the next packet.
//
// If we succeeded at encoding the whole list (i.e. the list is in fact
// empty and we fit in the packet), mAllowPartialData will be ignored,
// so it's safe to set it to false even if encoding succeeded.
mEncodeState.mAllowPartialData = false;
}
}

} // namespace app
Expand Down
3 changes: 3 additions & 0 deletions src/app/AttributeAccessInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ class AttributeValueEncoder

mCurrentEncodingListIndex++;
mEncodeState.mCurrentEncodingListIndex++;
mEncodedAtLeastOneListItem = true;
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -340,6 +341,8 @@ class AttributeValueEncoder
// started chunking it yet, so we're encoding a single attribute report IB
// for the whole list, not one per item.
bool mEncodingInitialList = false;
// mEncodedAtLeastOneListItem becomes true once we successfully encode a list item.
bool mEncodedAtLeastOneListItem = false;
AttributeEncodeState mEncodeState;
ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
};
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1295,8 +1295,8 @@ void TestReadInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void
!aPath.IsListItemOperation())
{
mGotStartOfSecondReport = true;
// Wait for an actual data chunk.
return;
// We always have data chunks, so go ahead to mark things
// dirty as needed.
}

if (!mGotStartOfSecondReport)
Expand Down

0 comments on commit 351dc16

Please sign in to comment.