From 3f5773787e1d2a198930962cfba06cde87e6873e Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 27 Jul 2023 18:44:49 -0400
Subject: [PATCH] Improve encoding of lists where first item can't fit in
 packet.

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.
---
 src/app/AttributeAccessInterface.cpp  | 13 +++++++++++++
 src/app/AttributeAccessInterface.h    |  3 +++
 src/app/tests/TestReadInteraction.cpp |  4 ++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/app/AttributeAccessInterface.cpp b/src/app/AttributeAccessInterface.cpp
index f4fbec03b34ac4..3152bbf604ffd9 100644
--- a/src/app/AttributeAccessInterface.cpp
+++ b/src/app/AttributeAccessInterface.cpp
@@ -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
diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h
index 8b4ec3a0c91d47..c195dd4f8b66ef 100644
--- a/src/app/AttributeAccessInterface.h
+++ b/src/app/AttributeAccessInterface.h
@@ -292,6 +292,7 @@ class AttributeValueEncoder
 
         mCurrentEncodingListIndex++;
         mEncodeState.mCurrentEncodingListIndex++;
+        mEncodedAtLeastOneListItem = true;
         return CHIP_NO_ERROR;
     }
 
@@ -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;
 };
diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
index 0c490f86002f73..bc3c8a2f9e71cb 100644
--- a/src/app/tests/TestReadInteraction.cpp
+++ b/src/app/tests/TestReadInteraction.cpp
@@ -1309,8 +1309,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)