Skip to content

Commit 2ecbcc2

Browse files
andy31415andreilitvintcarmelveilleuxbzbarsky-applemkardous-silabs
authored
Switch DataModel::Provider to a span-based list iterator - flash and ram savings, simpler interface (project-chip#37033)
* Copied over the new AttributePathExpandIterator and will incrementally use it (so I can validate tests) * Rename AttributePathExpandIterator to legacy * Prepare for using new style iterators ... checking NOT YET enabled though * Enabled checks ... and unit tests fail, but this now can be debugged * Fix some of the underlying bugs: read handling logic assumes we are ok to undo * Unit tests pass now * Restyle * Use new iterator in IME * Update logic to use the new iterator on testRead * more updates * Restyle * Remove the legacy attribute path expand iterator * Update naming * Restyle * Remove extra argument for ReadHandler constructor * Restyle * Slight flash improvement * Fix up includes * Removed empty line * added comment on why state is a friend class * Comment updates * Restyle, add some comments and add extra checks on validity check only for expansion. This saves a tiny amount of flash (32 bytes) * Remove an include * Comment updates, renamed mLastOutputPath to mOutputPath * Fix one typo * Re-arrange members of ReadHandler to optimize for memory layout. This saves 8 bytes for struct. We still have a 20-byte padding which I am unsure how to get rid of * Restyle * Rename State to Position * One more rename * Remove redundant assigment ...we are at a net 0 txt increase now on qpg * Add more unit tests for non-obvious requirement that wildcard expansion checks path validity, however non-wildcard does not check it * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/ReadHandler.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/ReadHandler.cpp Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Use different values for the cluster ids for testing * One more state to position change * mExpanded is now set during output path returning. Removed 2 more sets to save another tinier amount of .text * Import metadatalist class and test * Remove some tests that seem redundant, keep only one * Start with generated commands, see if we can replace its usage... * Unit tests for GeneratedCommands pass * Start with an implementation of accepted commands (no testing yet) * More tests pass * Updated AcceptedCommands as well .. unit tests pass * Restyle * Fix namespaces * Slight refactor. Code is still very much ugly * A bit of refactor, code looks better and tests pass now * Code compile for semantic tag ... made std::optional support non-trivial destructors * Update test * Make chip::Optional be trivially destructible if the underlying type is. Previous implementation always had a destructor, so it was never trivially destructible. * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/ReadHandler.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/ReadHandler.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Use mCompletePosition * Another rename * Undo submodule update * Restyle * Remove extra char * Remove unused variable * Update comment text to not sound like graph parsing * Rename method to be more descriptive * Remove one more unused variable * Update peek attribute iterator to rollback and update code logic a bit. Hoping for cleaner code * Semantic tags conversion is done * Restyle * Migrate device types to the new format * update comment a bit * Add unused marker for chip errors used for logging only * Fix descriptor cluster * Fix microwave oven * Restyle * Fix intentional bugprone-use-after-move * Fix intentional bugprone-use-after-move * Restyle * Fix based on clang feedback * Move endpoints to the new style of iteration * Fix includes * Fix includes * Fix includes * make it standard that test Providers are for now CodegenDataModelProvider. Saves me some typing as I move things around * Restyle * Minor update in logic: do the endpoint selection when next is called * Allow startup to try to mark attributes dirty even if no provider exists yet * Fix typo * start implementing client clusters * Update ClientCluster logic * Restyle * Start defining the server cluster query * Actually use the new server cluster functionality * Fix an include * More include fixes * implement the get attributes and adapt unit tests * Restyle * more updates to cleanup code. I am a bit concerned about O(n^2) attribute access... * A rename and moved finder methods out of inline. Saves 650 bytes of flash * Update logic to centralize metadata list code with less templating * move the generic metadata list to detail, add an assert on trivial destruction * Fix namespace prefix * Save 70 bytes by using references in condensed for loops * Replace count-if because it seems to result in smaller code (46 bytes) * Another find_if replacement * Replaced some find_if...we are down to 92 bytes on qpg * Replaced one more find-if, saving another 88 bytes of flash * Removed algorithm includes: these are slow and would like compile to fail if used * Save more flash ... we should be at a net negative now * This seems to save even more * More savings by more encode overrides ... this is silly... * Fix typos * More explicit casting, removed 64-bit overrides * Added one more check for freeing memory .Still need to track a leak that darwin finds * Fix memory leak in assignment * Self-review: fix includes * Self-review: fix includes * Self-review: fix includes * Self-review: fix includes * Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/clusters/descriptor/descriptor.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/clusters/descriptor/descriptor.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/MetadataList.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix spelling for acquire * Update src/app/data-model-provider/MetadataList.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/MetadataList.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/MetadataList.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix is_trivially_destructible * Update src/app/data-model-provider/MetadataSearch.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * More fixes * Fix includes * Fix invalid check typo * Fix comment * Correct comment * Updated comment * Make logic between clusters/attributes/endpoints the same regarding nullopt and wildcards * Restyle * Update logic for IsDescentantof * Help compiler generate efficient code as we keep reusing the same pointer * clearer logic that we handle all cases * Fixes * Use calloc * Update comment * Fix include * Fix casting * Make cluster count functions from ember public API since they seem reusable * Also fix dynamic dispatch * update enumeration entry * another comment update * Undo submodule update * Rename metadata search to metadta lookup * Update metadata list methods to be all uppercase * Fix more renames * Fix invalid cast * Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/MetadataList.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Address comments * fix bug * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Mathieu Kardous <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/WriteHandler.cpp Co-authored-by: Mathieu Kardous <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Mathieu Kardous <84793247+mkardous-silabs@users.noreply.github.com> * Restyle and include update --------- Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Mathieu Kardous <84793247+mkardous-silabs@users.noreply.github.com>
1 parent f19f44a commit 2ecbcc2

38 files changed

+1920
-1925
lines changed

examples/common/pigweed/rpc_services/Attributes.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <app/InteractionModelEngine.h>
2828
#include <app/MessageDef/AttributeReportIBs.h>
2929
#include <app/data-model-provider/ActionReturnStatus.h>
30+
#include <app/data-model-provider/MetadataLookup.h>
3031
#include <app/data-model-provider/OperationTypes.h>
3132
#include <app/data-model-provider/Provider.h>
3233
#include <app/util/attribute-storage.h>
@@ -221,7 +222,9 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
221222
request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
222223
request.subjectDescriptor = &subjectDescriptor;
223224

224-
std::optional<app::DataModel::ClusterInfo> info = provider->GetServerClusterInfo(path);
225+
app::DataModel::ServerClusterFinder serverClusterFinder(provider);
226+
auto info = serverClusterFinder.Find(path);
227+
225228
if (!info.has_value())
226229
{
227230
return ::pw::Status::NotFound();

src/access/ProviderDeviceTypeResolver.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ class DynamicProviderDeviceTypeResolver : public chip::Access::AccessControl::De
3131

3232
bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override
3333
{
34-
app::DataModel::Provider * model = mModelGetter();
35-
for (auto type = model->FirstDeviceType(endpoint); type.has_value(); type = model->NextDeviceType(endpoint, *type))
34+
auto deviceTypes = mModelGetter()->DeviceTypes(endpoint);
35+
for (auto & type : deviceTypes.GetSpanValidForLifetime())
3636
{
37-
if (type->deviceTypeId == deviceType)
37+
if (type.deviceTypeId == deviceType)
3838
{
3939
return true;
4040
}

src/app/AttributePathExpandIterator.cpp

+132-41
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
#include <app/AttributePathExpandIterator.h>
1818

1919
#include <app/GlobalAttributes.h>
20+
#include <app/data-model-provider/MetadataLookup.h>
21+
#include <app/data-model-provider/MetadataTypes.h>
22+
#include <lib/core/DataModelTypes.h>
2023
#include <lib/support/CodeUtils.h>
2124

2225
#include <optional>
@@ -26,6 +29,10 @@ using namespace chip::app::DataModel;
2629
namespace chip {
2730
namespace app {
2831

32+
AttributePathExpandIterator::AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) :
33+
mDataModelProvider(dataModel), mPosition(position)
34+
{}
35+
2936
bool AttributePathExpandIterator::AdvanceOutputPath()
3037
{
3138
/// Output path invariants
@@ -113,34 +120,61 @@ bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
113120
break;
114121
}
115122

123+
DataModel::AttributeFinder finder(mDataModelProvider);
124+
116125
const ConcreteAttributePath attributePath(mPosition.mOutputPath.mEndpointId, mPosition.mOutputPath.mClusterId, attributeId);
117-
return mDataModelProvider->GetAttributeInfo(attributePath).has_value();
126+
return finder.Find(attributePath).has_value();
118127
}
119128

120129
std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
121130
{
122131
if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId)
123132
{
124-
if (mPosition.mAttributePath->mValue.HasWildcardAttributeId())
133+
// Attribute ID is tied to attribute index. If no attribute id is available yet
134+
// this means the index is invalid. Processing logic in output advance only resets
135+
// attribute ID to invalid when resetting iteration.
136+
mAttributeIndex = kInvalidIndex;
137+
}
138+
139+
if (mAttributeIndex == kInvalidIndex)
140+
{
141+
// start a new iteration of attributes on the current cluster path.
142+
mAttributes = mDataModelProvider->Attributes(mPosition.mOutputPath);
143+
144+
if (mPosition.mOutputPath.mAttributeId != kInvalidAttributeId)
125145
{
126-
AttributeEntry entry = mDataModelProvider->FirstAttribute(mPosition.mOutputPath);
127-
return entry.IsValid() //
128-
? entry.path.mAttributeId //
129-
: Clusters::Globals::Attributes::GeneratedCommandList::Id; //
146+
// Position on the correct attribute if we have a start point
147+
mAttributeIndex = 0;
148+
while ((mAttributeIndex < mAttributes.Size()) &&
149+
(mAttributes[mAttributeIndex].attributeId != mPosition.mOutputPath.mAttributeId))
150+
{
151+
mAttributeIndex++;
152+
}
130153
}
154+
}
131155

132-
// At this point, the attributeID is NOT a wildcard (i.e. it is fixed).
133-
//
134-
// For wildcard expansion, we validate that this is a valid attribute for the given
135-
// cluster on the given endpoint. If not a wildcard expansion, return it as-is.
136-
if (mPosition.mAttributePath->mValue.IsWildcardPath())
156+
if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId)
157+
{
158+
if (!mPosition.mAttributePath->mValue.HasWildcardAttributeId())
137159
{
138-
if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId))
160+
// The attributeID is NOT a wildcard (i.e. it is fixed).
161+
//
162+
// For wildcard expansion, we validate that this is a valid attribute for the given
163+
// cluster on the given endpoint. If not a wildcard expansion, return it as-is.
164+
if (mPosition.mAttributePath->mValue.IsWildcardPath())
139165
{
140-
return std::nullopt;
166+
if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId))
167+
{
168+
return std::nullopt;
169+
}
141170
}
171+
return mPosition.mAttributePath->mValue.mAttributeId;
142172
}
143-
return mPosition.mAttributePath->mValue.mAttributeId;
173+
mAttributeIndex = 0;
174+
}
175+
else
176+
{
177+
mAttributeIndex++;
144178
}
145179

146180
// Advance the existing attribute id if it can be advanced.
@@ -165,10 +199,9 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
165199
return std::nullopt;
166200
}
167201

168-
AttributeEntry entry = mDataModelProvider->NextAttribute(mPosition.mOutputPath);
169-
if (entry.IsValid())
202+
if (mAttributeIndex < mAttributes.Size())
170203
{
171-
return entry.path.mAttributeId;
204+
return mAttributes[mAttributeIndex].attributeId;
172205
}
173206

174207
// Finished the data model, start with global attributes
@@ -178,55 +211,113 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
178211

179212
std::optional<ClusterId> AttributePathExpandIterator::NextClusterId()
180213
{
181-
182214
if (mPosition.mOutputPath.mClusterId == kInvalidClusterId)
183215
{
184-
if (mPosition.mAttributePath->mValue.HasWildcardClusterId())
216+
// Cluster ID is tied to cluster index. If no cluster id available yet
217+
// this means index is invalid. Processing logic in output advance only resets
218+
// cluster ID to invalid when resetting iteration.
219+
mClusterIndex = kInvalidIndex;
220+
}
221+
222+
if (mClusterIndex == kInvalidIndex)
223+
{
224+
// start a new iteration on the current endpoint
225+
mClusters = mDataModelProvider->ServerClusters(mPosition.mOutputPath.mEndpointId);
226+
227+
if (mPosition.mOutputPath.mClusterId != kInvalidClusterId)
185228
{
186-
ClusterEntry entry = mDataModelProvider->FirstServerCluster(mPosition.mOutputPath.mEndpointId);
187-
return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
229+
// Position on the correct cluster if we have a start point
230+
mClusterIndex = 0;
231+
while ((mClusterIndex < mClusters.Size()) && (mClusters[mClusterIndex].clusterId != mPosition.mOutputPath.mClusterId))
232+
{
233+
mClusterIndex++;
234+
}
188235
}
236+
}
189237

190-
// At this point, the clusterID is NOT a wildcard (i.e. is fixed).
191-
//
192-
// For wildcard expansion, we validate that this is a valid cluster for the endpoint.
193-
// If non-wildcard expansion, we return as-is.
194-
if (mPosition.mAttributePath->mValue.IsWildcardPath())
238+
if (mPosition.mOutputPath.mClusterId == kInvalidClusterId)
239+
{
240+
241+
if (!mPosition.mAttributePath->mValue.HasWildcardClusterId())
195242
{
196-
const ConcreteClusterPath clusterPath(mPosition.mOutputPath.mEndpointId, mPosition.mAttributePath->mValue.mClusterId);
197-
if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value())
243+
// The clusterID is NOT a wildcard (i.e. is fixed).
244+
//
245+
// For wildcard expansion, we validate that this is a valid cluster for the endpoint.
246+
// If non-wildcard expansion, we return as-is.
247+
if (mPosition.mAttributePath->mValue.IsWildcardPath())
198248
{
199-
return std::nullopt;
249+
const ClusterId clusterId = mPosition.mAttributePath->mValue.mClusterId;
250+
251+
auto span = mClusters.GetSpanValidForLifetime();
252+
253+
bool found = false;
254+
for (auto & entry : span)
255+
{
256+
if (entry.clusterId == clusterId)
257+
{
258+
found = true;
259+
break;
260+
}
261+
}
262+
263+
if (!found)
264+
{
265+
return std::nullopt;
266+
}
200267
}
201-
}
202268

203-
return mPosition.mAttributePath->mValue.mClusterId;
269+
return mPosition.mAttributePath->mValue.mClusterId;
270+
}
271+
mClusterIndex = 0;
272+
}
273+
else
274+
{
275+
mClusterIndex++;
204276
}
205277

206278
VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardClusterId(), std::nullopt);
279+
VerifyOrReturnValue(mClusterIndex < mClusters.Size(), std::nullopt);
207280

208-
ClusterEntry entry = mDataModelProvider->NextServerCluster(mPosition.mOutputPath);
209-
return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
281+
return mClusters[mClusterIndex].clusterId;
210282
}
211283

212-
std::optional<ClusterId> AttributePathExpandIterator::NextEndpointId()
284+
std::optional<EndpointId> AttributePathExpandIterator::NextEndpointId()
213285
{
286+
if (mEndpointIndex == kInvalidIndex)
287+
{
288+
// index is missing, have to start a new iteration
289+
mEndpoints = mDataModelProvider->Endpoints();
290+
291+
if (mPosition.mOutputPath.mEndpointId != kInvalidEndpointId)
292+
{
293+
// Position on the correct endpoint if we have a start point
294+
mEndpointIndex = 0;
295+
while ((mEndpointIndex < mEndpoints.Size()) && (mEndpoints[mEndpointIndex].id != mPosition.mOutputPath.mEndpointId))
296+
{
297+
mEndpointIndex++;
298+
}
299+
}
300+
}
301+
214302
if (mPosition.mOutputPath.mEndpointId == kInvalidEndpointId)
215303
{
216-
if (mPosition.mAttributePath->mValue.HasWildcardEndpointId())
304+
if (!mPosition.mAttributePath->mValue.HasWildcardEndpointId())
217305
{
218-
EndpointEntry ep = mDataModelProvider->FirstEndpoint();
219-
return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
306+
return mPosition.mAttributePath->mValue.mEndpointId;
220307
}
221308

222-
return mPosition.mAttributePath->mValue.mEndpointId;
309+
// start from the beginning
310+
mEndpointIndex = 0;
311+
}
312+
else
313+
{
314+
mEndpointIndex++;
223315
}
224316

225-
// Expand endpoints only if it is a wildcard on the endpoint specifically.
226317
VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardEndpointId(), std::nullopt);
318+
VerifyOrReturnValue(mEndpointIndex < mEndpoints.Size(), std::nullopt);
227319

228-
EndpointEntry ep = mDataModelProvider->NextEndpoint(mPosition.mOutputPath.mEndpointId);
229-
return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
320+
return mEndpoints[mEndpointIndex].id;
230321
}
231322

232323
} // namespace app

src/app/AttributePathExpandIterator.h

+18-4
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@
1919

2020
#include <app/AttributePathParams.h>
2121
#include <app/ConcreteAttributePath.h>
22+
#include <app/data-model-provider/MetadataList.h>
23+
#include <app/data-model-provider/MetadataTypes.h>
2224
#include <app/data-model-provider/Provider.h>
2325
#include <lib/core/DataModelTypes.h>
2426
#include <lib/support/LinkedList.h>
27+
#include <lib/support/Span.h>
28+
29+
#include <limits>
2530

2631
namespace chip {
2732
namespace app {
@@ -96,9 +101,7 @@ class AttributePathExpandIterator
96101
ConcreteAttributePath mOutputPath;
97102
};
98103

99-
AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) :
100-
mDataModelProvider(dataModel), mPosition(position)
101-
{}
104+
AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position);
102105

103106
// This class may not be copied. A new one should be created when needed and they
104107
// should not overlap.
@@ -113,9 +116,20 @@ class AttributePathExpandIterator
113116
bool Next(ConcreteAttributePath & path);
114117

115118
private:
119+
static constexpr size_t kInvalidIndex = std::numeric_limits<size_t>::max();
120+
116121
DataModel::Provider * mDataModelProvider;
117122
Position & mPosition;
118123

124+
DataModel::MetadataList<DataModel::EndpointEntry> mEndpoints; // all endpoints
125+
size_t mEndpointIndex = kInvalidIndex;
126+
127+
DataModel::MetadataList<DataModel::ServerClusterEntry> mClusters; // all clusters ON THE CURRENT endpoint
128+
size_t mClusterIndex = kInvalidIndex;
129+
130+
DataModel::MetadataList<DataModel::AttributeEntry> mAttributes; // all attributes ON THE CURRENT cluster
131+
size_t mAttributeIndex = kInvalidIndex;
132+
119133
/// Move to the next endpoint/cluster/attribute triplet that is valid given
120134
/// the current mOutputPath and mpAttributePath.
121135
///
@@ -140,7 +154,7 @@ class AttributePathExpandIterator
140154
/// Will start from the beginning if current mOutputPath.mEndpointId is kInvalidEndpointId
141155
///
142156
/// Respects path expansion/values in mpAttributePath
143-
std::optional<ClusterId> NextEndpointId();
157+
std::optional<EndpointId> NextEndpointId();
144158

145159
/// Checks if the given attributeId is valid for the current mOutputPath(endpoint/cluster)
146160
///

src/app/AttributeValueEncoder.h

+28
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,34 @@ class AttributeValueEncoder
6464
return mAttributeValueEncoder.EncodeListItem(mCheckpoint, aArg);
6565
}
6666

67+
// overrides that save flash: no need to care about the extra const
68+
// Without this, we have a usage of:
69+
// chip::ChipError chip::app::AttributeValueEncoder::EncodeListItem<unsigned long const&>
70+
// Overall we tend to have very similar code expand (from nm):
71+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char>(unsigned char&&)
72+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char&>(unsigned char&)
73+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char const&>(unsigned char const&)
74+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short const&>(unsigned short const&)
75+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long&>(unsigned long&)
76+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short&>(unsigned short&)
77+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long long&>(unsigned long long&)
78+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short>(unsigned short&&)
79+
// chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long long>(unsigned long long&&)
80+
// that we try to reduce
81+
//
82+
// TODO:
83+
// - we should figure where the extra const override is used
84+
// - we should try to avoid having such footguns. This list template-explosion seems
85+
// dangerous for flash.
86+
//
87+
// This relies on TLV numbers always being encoded as 64-bit value
88+
inline CHIP_ERROR Encode(uint32_t const & aArg) const { return Encode<uint64_t>(aArg); }
89+
inline CHIP_ERROR Encode(uint32_t & aArg) const { return Encode<uint64_t>(aArg); }
90+
inline CHIP_ERROR Encode(uint16_t const & aArg) const { return Encode<uint64_t>(aArg); }
91+
inline CHIP_ERROR Encode(uint16_t & aArg) const { return Encode<uint64_t>(aArg); }
92+
inline CHIP_ERROR Encode(uint8_t const & aArg) const { return Encode<uint64_t>(aArg); }
93+
inline CHIP_ERROR Encode(uint8_t & aArg) const { return Encode<uint64_t>(aArg); }
94+
6795
private:
6896
AttributeValueEncoder & mAttributeValueEncoder;
6997
// Avoid calling the TLVWriter constructor for every instantiation of

0 commit comments

Comments
 (0)