Skip to content

Commit fce1492

Browse files
committed
According to the review comments:
1. Removed the hardcoaded size of the array to store endpointUniqueId 2. Changed the way endpointUniqueId is set. Using the add end point engine rather than a new setter. 3. Not using std::string instead changed to char array.
1 parent b3768d2 commit fce1492

File tree

5 files changed

+29
-35
lines changed

5 files changed

+29
-35
lines changed

scripts/tools/check_includes_config.py

-2
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,4 @@
193193

194194
# nrfconnect test runner
195195
'src/test_driver/nrfconnect/main/runner.cpp': {'vector'},
196-
197-
'src/app/util/af-types.h': {'string'},
198196
}

src/app/clusters/descriptor/descriptor.cpp

+5-8
Original file line numberDiff line numberDiff line change
@@ -212,16 +212,13 @@ CHIP_ERROR DescriptorAttrAccess::ReadDeviceAttribute(EndpointId endpoint, Attrib
212212
return err;
213213
}
214214

215-
CHIP_ERROR EncodeString(AttributeValueEncoder & encoder, const char * buf, size_t maxBufSize)
216-
{
217-
return encoder.Encode(chip::CharSpan(buf, strnlen(buf, maxBufSize)));
218-
}
219-
220215
CHIP_ERROR DescriptorAttrAccess::ReadEndpointUniqueId(EndpointId endpoint, AttributeValueEncoder & aEncoder)
221216
{
222-
char endpointUniqueId[32 + 1] = { 0 };
223-
GetEndpointUniqueIdForEndPoint(endpoint, endpointUniqueId);
224-
return EncodeString(aEncoder, endpointUniqueId, 32);
217+
char epUniqueId[Attributes::EndpointUniqueId::TypeInfo::MaxLength() + 1] = { 0 };
218+
chip::MutableCharSpan epUniqueIdSpan(epUniqueId);
219+
GetEndpointUniqueIdForEndPoint(endpoint, epUniqueIdSpan);
220+
221+
return aEncoder.Encode(epUniqueIdSpan);
225222
}
226223

227224
CHIP_ERROR DescriptorAttrAccess::ReadServerClusters(EndpointId endpoint, AttributeValueEncoder & aEncoder)

src/app/util/af-types.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
#include <stdbool.h> // For bool
2727
#include <stdint.h> // For various uint*_t types
28-
#include <string> // For string
2928

3029
#include <app/util/AttributesChangedListener.h>
3130
#include <app/util/MarkAttributeDirty.h>
@@ -238,7 +237,10 @@ struct EmberAfDefinedEndpoint
238237
*/
239238
chip::Span<const chip::app::Clusters::Descriptor::Structs::SemanticTagStruct::Type> tagList;
240239

241-
std::string endpointUniqueId;
240+
/**
241+
* Unique Id for this end point.
242+
*/
243+
char endpointUniqueId[chip::app::Clusters::Descriptor::Attributes::EndpointUniqueId::TypeInfo::MaxLength()] = {0};
242244
};
243245

244246
/**

src/app/util/attribute-storage.cpp

+15-19
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ uint16_t emberAfGetDynamicIndexFromEndpoint(EndpointId id)
265265

266266
CHIP_ERROR emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const EmberAfEndpointType * ep,
267267
const Span<DataVersion> & dataVersionStorage, Span<const EmberAfDeviceType> deviceTypeList,
268-
EndpointId parentEndpointId)
268+
chip::CharSpan endpointUniqueId, EndpointId parentEndpointId)
269269
{
270270
auto realIndex = index + FIXED_ENDPOINT_COUNT;
271271

@@ -317,10 +317,14 @@ CHIP_ERROR emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const EmberA
317317
}
318318
}
319319
}
320-
emAfEndpoints[index].endpoint = id;
321-
emAfEndpoints[index].deviceTypeList = deviceTypeList;
322-
emAfEndpoints[index].endpointType = ep;
323-
emAfEndpoints[index].dataVersions = dataVersionStorage.data();
320+
emAfEndpoints[index].endpoint = id;
321+
emAfEndpoints[index].deviceTypeList = deviceTypeList;
322+
emAfEndpoints[index].endpointType = ep;
323+
emAfEndpoints[index].dataVersions = dataVersionStorage.data();
324+
memcpy((void *)(emAfEndpoints[index].endpointUniqueId), endpointUniqueId.data(),
325+
endpointUniqueId.size() > Clusters::Descriptor::Attributes::EndpointUniqueId::TypeInfo::MaxLength()?
326+
Clusters::Descriptor::Attributes::EndpointUniqueId::TypeInfo::MaxLength() : endpointUniqueId.size());
327+
324328
// Start the endpoint off as disabled.
325329
emAfEndpoints[index].bitmask.Clear(EmberAfEndpointOptions::isEnabled);
326330
emAfEndpoints[index].parentEndpointId = parentEndpointId;
@@ -1080,15 +1084,19 @@ CHIP_ERROR GetSemanticTagForEndpointAtIndex(EndpointId endpoint, size_t index,
10801084
return CHIP_NO_ERROR;
10811085
}
10821086

1083-
CHIP_ERROR GetEndpointUniqueIdForEndPoint(EndpointId endpoint, char * buf)
1087+
CHIP_ERROR GetEndpointUniqueIdForEndPoint(EndpointId endpoint, chip::MutableCharSpan & epUniqueIdMutSpan)
10841088
{
10851089
uint16_t endpointIndex = emberAfIndexFromEndpoint(endpoint);
10861090

10871091
if (endpointIndex == 0xFFFF)
10881092
{
10891093
return CHIP_ERROR_NOT_FOUND;
10901094
}
1091-
strcpy(buf, emAfEndpoints[endpointIndex].endpointUniqueId.c_str());
1095+
chip::CharSpan epUniqueIdSpan(emAfEndpoints[endpointIndex].endpointUniqueId,
1096+
strnlen(emAfEndpoints[endpointIndex].endpointUniqueId,
1097+
Clusters::Descriptor::Attributes::EndpointUniqueId::TypeInfo::MaxLength()));
1098+
chip::CopyCharSpanToMutableCharSpan(epUniqueIdSpan, epUniqueIdMutSpan);
1099+
10921100
return CHIP_NO_ERROR;
10931101
}
10941102

@@ -1116,18 +1124,6 @@ CHIP_ERROR SetTagList(EndpointId endpoint, Span<const Clusters::Descriptor::Stru
11161124
return CHIP_NO_ERROR;
11171125
}
11181126

1119-
CHIP_ERROR SetEndPointUniqueId(chip::EndpointId endpoint, std::string endpointUniqueId)
1120-
{
1121-
uint16_t endpointIndex = emberAfIndexFromEndpoint(endpoint);
1122-
if (endpointIndex == 0xFFFF)
1123-
{
1124-
return CHIP_ERROR_INVALID_ARGUMENT;
1125-
}
1126-
1127-
emAfEndpoints[endpointIndex].endpointUniqueId = endpointUniqueId;
1128-
return CHIP_NO_ERROR;
1129-
}
1130-
11311127
// Returns the cluster of Nth server or client cluster,
11321128
// depending on server toggle.
11331129
const EmberAfCluster * emberAfGetNthCluster(EndpointId endpoint, uint8_t n, bool server)

src/app/util/attribute-storage.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,7 @@ CHIP_ERROR GetSemanticTagForEndpointAtIndex(chip::EndpointId endpoint, size_t in
208208
CHIP_ERROR SetTagList(chip::EndpointId endpoint,
209209
chip::Span<const chip::app::Clusters::Descriptor::Structs::SemanticTagStruct::Type> tagList);
210210

211-
CHIP_ERROR GetEndpointUniqueIdForEndPoint(chip::EndpointId endpoint, char * buf);
212-
213-
CHIP_ERROR SetEndPointUniqueId(chip::EndpointId endpoint, std::string endpointUniqueId);
211+
CHIP_ERROR GetEndpointUniqueIdForEndPoint(chip::EndpointId endpoint, chip::MutableCharSpan & epUniqueIdMutSpan);
214212

215213
// Returns number of clusters put into the passed cluster list
216214
// for the given endpoint and client/server polarity
@@ -252,6 +250,8 @@ void emberAfEndpointConfigure();
252250
// An optional device type list can be passed in as well. If provided, the memory
253251
// backing the list needs to remain allocated until this dynamic endpoint is cleared.
254252
//
253+
// An optional endpointUniqueId can be passed.
254+
//
255255
// An optional parent endpoint id should be passed for child endpoints of composed device.
256256
//
257257
// Returns CHIP_NO_ERROR No error.
@@ -262,7 +262,8 @@ void emberAfEndpointConfigure();
262262
CHIP_ERROR emberAfSetDynamicEndpoint(uint16_t index, chip::EndpointId id, const EmberAfEndpointType * ep,
263263
const chip::Span<chip::DataVersion> & dataVersionStorage,
264264
chip::Span<const EmberAfDeviceType> deviceTypeList = {},
265-
chip::EndpointId parentEndpointId = chip::kInvalidEndpointId);
265+
chip::CharSpan endpointUniqueId ={},
266+
chip::EndpointId parentEndpointId = chip::kInvalidEndpointId);
266267
chip::EndpointId emberAfClearDynamicEndpoint(uint16_t index);
267268
uint16_t emberAfGetDynamicIndexFromEndpoint(chip::EndpointId id);
268269
/**

0 commit comments

Comments
 (0)