Skip to content

Commit 3debb6d

Browse files
andy31415andy31415
committed
Split out AttributeEncodeState and decouple things to use less friend classes
1 parent c278b9b commit 3debb6d

16 files changed

+109
-78
lines changed

‎src/app/AttributeValueEncoder.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
3232
{
3333
VerifyOrDie(mCurrentEncodingListIndex == kInvalidListIndex);
3434

35-
mEncodingInitialList = (mEncodeState.mCurrentEncodingListIndex == kInvalidListIndex);
35+
mEncodingInitialList = (mEncodeState.CurrentEncodingListIndex() == kInvalidListIndex);
3636
if (mEncodingInitialList)
3737
{
3838
// Clear mAllowPartialData flag here since this encode procedure is not atomic.
3939
// The most common error in this function is CHIP_ERROR_NO_MEMORY / CHIP_ERROR_BUFFER_TOO_SMALL, just revert and try
4040
// next time is ok.
41-
mEncodeState.mAllowPartialData = false;
41+
mEncodeState.SetAllowPartialData(false);
4242

4343
AttributeReportBuilder builder;
4444

@@ -59,7 +59,7 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
5959
ReturnErrorOnFailure(
6060
mAttributeReportIBsBuilder.GetWriter()->ReserveBuffer(kEndOfAttributeReportIBByteCount + kEndOfListByteCount));
6161

62-
mEncodeState.mCurrentEncodingListIndex = 0;
62+
mEncodeState.SetCurrentEncodingListIndex(0);
6363
}
6464
else
6565
{
@@ -72,7 +72,7 @@ CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
7272

7373
// After encoding the initial list start, the remaining items are atomically encoded into the buffer. Tell report engine to not
7474
// revert partial data.
75-
mEncodeState.mAllowPartialData = true;
75+
mEncodeState.SetAllowPartialData(true);
7676

7777
return CHIP_NO_ERROR;
7878
}
@@ -105,7 +105,7 @@ void AttributeValueEncoder::EnsureListEnded()
105105
// If we succeeded at encoding the whole list (i.e. the list is in fact
106106
// empty and we fit in the packet), mAllowPartialData will be ignored,
107107
// so it's safe to set it to false even if encoding succeeded.
108-
mEncodeState.mAllowPartialData = false;
108+
mEncodeState.SetAllowPartialData(false);
109109
}
110110
}
111111

‎src/app/AttributeValueEncoder.h

+56-33
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,60 @@
2828
namespace chip {
2929
namespace app {
3030

31+
/**
32+
* Maintains the internal state of list encoding
33+
*/
34+
class AttributeEncodeState
35+
{
36+
public:
37+
AttributeEncodeState() = default;
38+
39+
bool AllowPartialData() const { return mAllowPartialData; }
40+
ListIndex CurrentEncodingListIndex() const { return mCurrentEncodingListIndex; }
41+
42+
AttributeEncodeState & SetAllowPartialData(bool allow)
43+
{
44+
mAllowPartialData = allow;
45+
return *this;
46+
}
47+
48+
AttributeEncodeState & SetCurrentEncodingListIndex(ListIndex idx)
49+
{
50+
mCurrentEncodingListIndex = idx;
51+
return *this;
52+
}
53+
54+
void Reset()
55+
{
56+
mCurrentEncodingListIndex = kInvalidListIndex;
57+
mAllowPartialData = false;
58+
}
59+
60+
private:
61+
/**
62+
* If set to kInvalidListIndex, indicates that we have not encoded any data for the list yet and
63+
* need to start by encoding an empty list before we start encoding any list items.
64+
*
65+
* When set to a valid ListIndex value, indicates the index of the next list item that needs to be
66+
* encoded (i.e. the count of items encoded so far).
67+
*/
68+
ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
69+
70+
/**
71+
* When an attempt to encode an attribute returns an error, the buffer may contain tailing dirty data
72+
* (since the put was aborted). The report engine normally rolls back the buffer to right before encoding
73+
* of the attribute started on errors.
74+
*
75+
* When chunking a list, EncodeListItem will atomically encode list items, ensuring that the
76+
* state of the buffer is valid to send (i.e. contains no trailing garbage), and return an error
77+
* if the list doesn't entirely fit. In this situation, mAllowPartialData is set to communicate to the
78+
* report engine that it should not roll back the list items.
79+
*
80+
* TODO: There might be a better name for this variable.
81+
*/
82+
bool mAllowPartialData = false;
83+
};
84+
3185
/**
3286
* The AttributeValueEncoder is a helper class for filling report payloads into AttributeReportIBs.
3387
* The attribute value encoder can be initialized with a AttributeEncodeState for saving and recovering its state between encode
@@ -68,37 +122,6 @@ class AttributeValueEncoder
68122
AttributeValueEncoder & mAttributeValueEncoder;
69123
};
70124

71-
class AttributeEncodeState
72-
{
73-
public:
74-
AttributeEncodeState() : mAllowPartialData(false), mCurrentEncodingListIndex(kInvalidListIndex) {}
75-
bool AllowPartialData() const { return mAllowPartialData; }
76-
77-
private:
78-
friend class AttributeValueEncoder;
79-
/**
80-
* When an attempt to encode an attribute returns an error, the buffer may contain tailing dirty data
81-
* (since the put was aborted). The report engine normally rolls back the buffer to right before encoding
82-
* of the attribute started on errors.
83-
*
84-
* When chunking a list, EncodeListItem will atomically encode list items, ensuring that the
85-
* state of the buffer is valid to send (i.e. contains no trailing garbage), and return an error
86-
* if the list doesn't entirely fit. In this situation, mAllowPartialData is set to communicate to the
87-
* report engine that it should not roll back the list items.
88-
*
89-
* TODO: There might be a better name for this variable.
90-
*/
91-
bool mAllowPartialData = false;
92-
/**
93-
* If set to kInvalidListIndex, indicates that we have not encoded any data for the list yet and
94-
* need to start by encoding an empty list before we start encoding any list items.
95-
*
96-
* When set to a valid ListIndex value, indicates the index of the next list item that needs to be
97-
* encoded (i.e. the count of items encoded so far).
98-
*/
99-
ListIndex mCurrentEncodingListIndex = kInvalidListIndex;
100-
};
101-
102125
AttributeValueEncoder(AttributeReportIBs::Builder & aAttributeReportIBsBuilder, Access::SubjectDescriptor subjectDescriptor,
103126
const ConcreteAttributePath & aPath, DataVersion aDataVersion, bool aIsFabricFiltered = false,
104127
const AttributeEncodeState & aState = AttributeEncodeState()) :
@@ -194,7 +217,7 @@ class AttributeValueEncoder
194217
{
195218
// EncodeListItem must be called after EnsureListStarted(), thus mCurrentEncodingListIndex and
196219
// mEncodeState.mCurrentEncodingListIndex are not invalid values.
197-
if (mCurrentEncodingListIndex < mEncodeState.mCurrentEncodingListIndex)
220+
if (mCurrentEncodingListIndex < mEncodeState.CurrentEncodingListIndex())
198221
{
199222
// We have encoded this element in previous chunks, skip it.
200223
mCurrentEncodingListIndex++;
@@ -225,7 +248,7 @@ class AttributeValueEncoder
225248
}
226249

227250
mCurrentEncodingListIndex++;
228-
mEncodeState.mCurrentEncodingListIndex++;
251+
mEncodeState.SetCurrentEncodingListIndex(mCurrentEncodingListIndex);
229252
mEncodedAtLeastOneListItem = true;
230253
return CHIP_NO_ERROR;
231254
}

‎src/app/ReadHandler.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -841,7 +841,7 @@ void ReadHandler::PersistSubscription()
841841
void ReadHandler::ResetPathIterator()
842842
{
843843
mAttributePathExpandIterator = AttributePathExpandIterator(mpAttributePathList);
844-
mAttributeEncoderState = AttributeValueEncoder::AttributeEncodeState();
844+
mAttributeEncoderState.Reset();
845845
}
846846

847847
void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeChanged)
@@ -870,7 +870,7 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha
870870
// our iterator to point back to the beginning of that cluster. This ensures that the receiver will get a coherent view of
871871
// the state of the cluster as present on the server
872872
mAttributePathExpandIterator.ResetCurrentCluster();
873-
mAttributeEncoderState = AttributeValueEncoder::AttributeEncodeState();
873+
mAttributeEncoderState.Reset();
874874
}
875875

876876
// ReportScheduler will take care of verifying the reportability of the handler and schedule the run

‎src/app/ReadHandler.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,8 @@ class ReadHandler : public Messaging::ExchangeDelegate
412412
/// or after the min interval is reached if it has not yet been reached.
413413
void ForceDirtyState();
414414

415-
const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; }
416-
void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; }
415+
const AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; }
416+
void SetAttributeEncodeState(const AttributeEncodeState & aState) { mAttributeEncoderState = aState; }
417417
uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; }
418418

419419
// Returns the number of interested paths, including wildcard and concrete paths.
@@ -562,7 +562,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
562562

563563
// The detailed encoding state for a single attribute, used by list chunking feature.
564564
// The size of AttributeEncoderState is 2 bytes for now.
565-
AttributeValueEncoder::AttributeEncodeState mAttributeEncoderState;
565+
AttributeEncodeState mAttributeEncoderState;
566566

567567
// Current Handler state
568568
HandlerState mState = HandlerState::Idle;

‎src/app/dynamic_server/DynamicDispatcher.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ Status DetermineAttributeStatus(const ConcreteAttributePath & aPath, bool aIsWri
123123

124124
CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
125125
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
126-
AttributeValueEncoder::AttributeEncodeState * aEncoderState)
126+
AttributeEncodeState * aEncoderState)
127127
{
128128
Status status = DetermineAttributeStatus(aPath, /* aIsWrite = */ false);
129129
return aAttributeReports.EncodeAttributeStatus(aPath, StatusIB(status));

‎src/app/reporting/Engine.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ bool Engine::IsClusterDataVersionMatch(const SingleLinkedListNode<DataVersionFil
8282
CHIP_ERROR
8383
Engine::RetrieveClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
8484
AttributeReportIBs::Builder & aAttributeReportIBs, const ConcreteReadAttributePath & aPath,
85-
AttributeValueEncoder::AttributeEncodeState * aEncoderState)
85+
AttributeEncodeState * aEncoderState)
8686
{
8787
ChipLogDetail(DataManagement, "<RE:Run> Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", aPath.mClusterId,
8888
aPath.mAttributeId);
@@ -199,7 +199,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
199199
attributeReportIBs.Checkpoint(attributeBackup);
200200
ConcreteReadAttributePath pathForRetrieval(readPath);
201201
// Load the saved state from previous encoding session for chunking of one single attribute (list chunking).
202-
AttributeValueEncoder::AttributeEncodeState encodeState = apReadHandler->GetAttributeEncodeState();
202+
AttributeEncodeState encodeState = apReadHandler->GetAttributeEncodeState();
203203
err = RetrieveClusterData(apReadHandler->GetSubjectDescriptor(), apReadHandler->IsFabricFiltered(), attributeReportIBs,
204204
pathForRetrieval, &encodeState);
205205
if (err != CHIP_NO_ERROR)
@@ -226,7 +226,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
226226
// We met a error during writing reports, one common case is we are running out of buffer, rollback the
227227
// attributeReportIB to avoid any partial data.
228228
attributeReportIBs.Rollback(attributeBackup);
229-
apReadHandler->SetAttributeEncodeState(AttributeValueEncoder::AttributeEncodeState());
229+
apReadHandler->SetAttributeEncodeState(AttributeEncodeState());
230230

231231
if (!IsOutOfWriterSpaceError(err))
232232
{
@@ -256,7 +256,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
256256
}
257257
SuccessOrExit(err);
258258
// Successfully encoded the attribute, clear the internal state.
259-
apReadHandler->SetAttributeEncodeState(AttributeValueEncoder::AttributeEncodeState());
259+
apReadHandler->SetAttributeEncodeState(AttributeEncodeState());
260260
}
261261
// We just visited all paths interested by this read handler and did not abort in the middle of iteration, there are no more
262262
// chunks for this report.

‎src/app/reporting/Engine.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,7 @@ class Engine
170170
bool aBufferIsUsed, bool * apHasMoreChunks, bool * apHasEncodedData);
171171
CHIP_ERROR RetrieveClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
172172
AttributeReportIBs::Builder & aAttributeReportIBs,
173-
const ConcreteReadAttributePath & aClusterInfo,
174-
AttributeValueEncoder::AttributeEncodeState * apEncoderState);
173+
const ConcreteReadAttributePath & aClusterInfo, AttributeEncodeState * apEncoderState);
175174
CHIP_ERROR CheckAccessDeniedEventPaths(TLV::TLVWriter & aWriter, bool & aHasEncodedData, ReadHandler * apReadHandler);
176175

177176
// If version match, it means don't send, if version mismatch, it means send.

‎src/app/tests/TestAttributeValueEncoder.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ template <size_t N>
5555
struct LimitedTestSetup
5656
{
5757
LimitedTestSetup(nlTestSuite * aSuite, const FabricIndex aFabricIndex = kUndefinedFabricIndex,
58-
const AttributeValueEncoder::AttributeEncodeState & aState = AttributeValueEncoder::AttributeEncodeState()) :
58+
const AttributeEncodeState & aState = AttributeEncodeState()) :
5959
encoder(builder, aFabricIndex, ConcreteAttributePath(kRandomEndpointId, kRandomClusterId, kRandomAttributeId),
6060
kRandomDataVersion, aFabricIndex != kUndefinedFabricIndex, aState)
6161
{
@@ -275,7 +275,7 @@ void TestEncodeFabricScoped(nlTestSuite * aSuite, void * aContext)
275275

276276
void TestEncodeListChunking(nlTestSuite * aSuite, void * aContext)
277277
{
278-
AttributeValueEncoder::AttributeEncodeState state;
278+
AttributeEncodeState state;
279279

280280
bool list[] = { true, false, false, true, true, false };
281281
auto listEncoder = [&list](const auto & encoder) -> CHIP_ERROR {
@@ -400,7 +400,7 @@ void TestEncodeListChunking(nlTestSuite * aSuite, void * aContext)
400400

401401
void TestEncodeListChunking2(nlTestSuite * aSuite, void * aContext)
402402
{
403-
AttributeValueEncoder::AttributeEncodeState state;
403+
AttributeEncodeState state;
404404

405405
bool list[] = { true, false, false, true, true, false };
406406
auto listEncoder = [&list](const auto & encoder) -> CHIP_ERROR {

‎src/app/tests/TestReadInteraction.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ namespace app {
303303

304304
CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
305305
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
306-
AttributeValueEncoder::AttributeEncodeState * apEncoderState)
306+
AttributeEncodeState * apEncoderState)
307307
{
308308
if (aPath.mClusterId >= Test::kMockEndpointMin)
309309
{

‎src/app/tests/integration/chip_im_initiator.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip
631631

632632
CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
633633
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
634-
AttributeValueEncoder::AttributeEncodeState * apEncoderState)
634+
AttributeEncodeState * apEncoderState)
635635
{
636636
AttributeReportIB::Builder & attributeReport = aAttributeReports.CreateAttributeReport();
637637
ReturnErrorOnFailure(aAttributeReports.GetError());

‎src/app/tests/integration/chip_im_responder.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPat
121121

122122
CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
123123
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
124-
AttributeValueEncoder::AttributeEncodeState * apEncoderState)
124+
AttributeEncodeState * apEncoderState)
125125
{
126126
ReturnErrorOnFailure(AttributeValueEncoder(aAttributeReports, 0, aPath, 0).Encode(kTestFieldValue1));
127127
return CHIP_NO_ERROR;

‎src/app/util/ember-compatibility-functions.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -427,12 +427,11 @@ CHIP_ERROR GlobalAttributeReader::EncodeCommandList(const ConcreteClusterPath &
427427
// aTriedEncode outparam is set to whether the AttributeAccessInterface tried to encode a value.
428428
CHIP_ERROR ReadViaAccessInterface(SubjectDescriptor subjectDescriptor, bool aIsFabricFiltered,
429429
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
430-
AttributeValueEncoder::AttributeEncodeState * aEncoderState,
431-
AttributeAccessInterface * aAccessInterface, bool * aTriedEncode)
430+
AttributeEncodeState * aEncoderState, AttributeAccessInterface * aAccessInterface,
431+
bool * aTriedEncode)
432432
{
433-
AttributeValueEncoder::AttributeEncodeState state =
434-
(aEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *aEncoderState);
435-
DataVersion version = 0;
433+
AttributeEncodeState state = (aEncoderState == nullptr ? AttributeEncodeState() : *aEncoderState);
434+
DataVersion version = 0;
436435
ReturnErrorOnFailure(ReadClusterDataVersion(aPath, version));
437436
AttributeValueEncoder valueEncoder(aAttributeReports, subjectDescriptor, aPath, version, aIsFabricFiltered, state);
438437
CHIP_ERROR err = aAccessInterface->Read(aPath, valueEncoder);
@@ -523,7 +522,7 @@ bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
523522

524523
CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
525524
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
526-
AttributeValueEncoder::AttributeEncodeState * apEncoderState)
525+
AttributeEncodeState * apEncoderState)
527526
{
528527
ChipLogDetail(DataManagement,
529528
"Reading attribute: Cluster=" ChipLogFormatMEI " Endpoint=%x AttributeId=" ChipLogFormatMEI " (expanded=%d)",

‎src/app/util/ember-compatibility-functions.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath);
6060
*/
6161
CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
6262
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
63-
AttributeValueEncoder::AttributeEncodeState * apEncoderState);
63+
AttributeEncodeState * apEncoderState);
6464

6565
/**
6666
* Returns the metadata of the attribute for the given path.

‎src/app/util/mock/Functions.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ namespace Test {
3535

3636
CHIP_ERROR ReadSingleMockClusterData(FabricIndex aAccessingFabricIndex, const app::ConcreteAttributePath & aPath,
3737
app::AttributeReportIBs::Builder & aAttributeReports,
38-
app::AttributeValueEncoder::AttributeEncodeState * apEncoderState);
38+
app::AttributeEncodeState * apEncoderState);
3939

4040
/// Increase the current value for `GetVersion`
4141
void BumpVersion();

0 commit comments

Comments
 (0)