Skip to content

Commit

Permalink
[attribute storage] add automatic attribute storage for dynamic endpo…
Browse files Browse the repository at this point in the history 
…ints

Dynamically defined endpoints can now have their own block of storage
(to be provided by the caller of emberAfSetDynamicEndpoint()).

If no storage is set for a dynamic endpoint, it behaves exactly as before,
which is treating all attributes as "external", regardless of the respective
bit in the attribute's metadata.

With a storage provided, attributes behave the same way as static endpoint's
do, that is, only those marked "external" need to be handled programmatically
in the respective callbacks, other attributes' storage is automatic.
  • Loading branch information
@plan44
plan44 committed Aug 8, 2023
1 parent 367a0c6 commit 485fa6f
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 15 deletions.
10 changes: 9 additions & 1 deletion src/app/util/af-types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ typedef void (*EmberAfGenericClusterFunction)(void);
/**
* @brief Struct describing cluster
*/

typedef struct
{
/**
Expand Down Expand Up @@ -176,7 +177,7 @@ typedef struct
*/
uint8_t clusterCount;
/**
* Size of all non-external, non-singlet attribute in this endpoint type.
* Size of all non-external, non-singleton attributes in this endpoint type.
*/
uint16_t endpointSize;
} EmberAfEndpointType;
Expand Down Expand Up @@ -217,6 +218,13 @@ struct EmberAfDefinedEndpoint
*/
chip::DataVersion * dataVersions = nullptr;

/**
* Pointer to the memory block to be used for automatic attribute storage if
* this is a dynamic endpoint. If set, the memory block pointed at
* must have the sum of all endpointType->clusters[*].clustersize.
*/
uint8_t * dynamicAttributeStorage = nullptr;

/**
* Root endpoint id for composed device type.
*/
Expand Down
57 changes: 44 additions & 13 deletions src/app/util/attribute-storage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ uint16_t emberAfGetDynamicIndexFromEndpoint(EndpointId id)

EmberAfStatus emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const EmberAfEndpointType * ep,
const chip::Span<chip::DataVersion> & dataVersionStorage,
chip::Span<const EmberAfDeviceType> deviceTypeList, EndpointId parentEndpointId)
chip::Span<const EmberAfDeviceType> deviceTypeList, EndpointId parentEndpointId,
uint8_t * dynamicAttributeStorage)
{
auto realIndex = index + FIXED_ENDPOINT_COUNT;

Expand Down Expand Up @@ -275,6 +276,7 @@ EmberAfStatus emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const Emb
// Start the endpoint off as disabled.
emAfEndpoints[index].bitmask.Clear(EmberAfEndpointOptions::isEnabled);
emAfEndpoints[index].parentEndpointId = parentEndpointId;
emAfEndpoints[index].dynamicAttributeStorage = dynamicAttributeStorage;

emberAfSetDynamicEndpointCount(MAX_ENDPOINT_COUNT - FIXED_ENDPOINT_COUNT);

Expand Down Expand Up @@ -373,8 +375,8 @@ EmberAfStatus emAfClusterPreAttributeChangedCallback(const app::ConcreteAttribut
EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
// Casting and calling a function pointer on the same line results in ignoring the return
// of the call on gcc-arm-none-eabi-9-2019-q4-major
EmberAfClusterPreAttributeChangedCallback f = (EmberAfClusterPreAttributeChangedCallback)(
emberAfFindClusterFunction(cluster, CLUSTER_MASK_PRE_ATTRIBUTE_CHANGED_FUNCTION));
EmberAfClusterPreAttributeChangedCallback f = (EmberAfClusterPreAttributeChangedCallback) (emberAfFindClusterFunction(
cluster, CLUSTER_MASK_PRE_ATTRIBUTE_CHANGED_FUNCTION));
if (f != nullptr)
{
status = f(attributePath, attributeType, size, value);
Expand Down Expand Up @@ -571,21 +573,40 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
{
assertChipStackLockedByCurrentThread();

uint16_t attributeOffsetIndex = 0;
// offset relative to the storage block, which is:
// - for static endpoints: attributeData[] global
// - for dynamic endpoints: dynamicAttributeStorage (unless nullPtr, then all attributes must be external)
uint16_t attributeStorageOffset = 0;

for (uint16_t ep = 0; ep < emberAfEndpointCount(); ep++)
{
// Is this a dynamic endpoint?
bool isDynamicEndpoint = (ep >= emberAfFixedEndpointCount());

if (emAfEndpoints[ep].endpoint == attRecord->endpoint)
{
{ // Got the endpoint
const EmberAfEndpointType * endpointType = emAfEndpoints[ep].endpointType;
uint8_t clusterIndex;
if (!emberAfEndpointIndexIsEnabled(ep))
{
// TODO: I think this is wrong, and should be a break
// It does not harm because usually no other endpointindex will contain
// an endpoint with the same ID, but it would cause catastrophic mess in
// attribute data because attributeOffsetIndex does not get incremented -
// endpoint enabling/disabling is dynamic, so enabled/disabled state
// MUST NOT change the data layout!
continue;
}

bool hasDynamicAttributeStorage = emAfEndpoints[ep].dynamicAttributeStorage != nullptr;
if (hasDynamicAttributeStorage)
{
// endpoint storage is not in the static global `attributeData`, but offset
// from the per-endpoint dynamicAttributeStorage pointer.
// Endpoint processing starts here, so reset the offset.
attributeStorageOffset = 0;
}

for (clusterIndex = 0; clusterIndex < endpointType->clusterCount; clusterIndex++)
{
const EmberAfCluster * cluster = &(endpointType->cluster[clusterIndex]);
Expand All @@ -604,9 +625,17 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
}

{
// attribute storage can be
// - singleton: statically allocated in singletonAttributeData global
// - static endpoint: statically allocated in attributeData global
// - dynamic endpoint with dynamic storage: in memory block provided at endpoint instantiation
uint8_t * attributeLocation =
(am->mask & ATTRIBUTE_MASK_SINGLETON ? singletonAttributeLocation(am)
: attributeData + attributeOffsetIndex);
(am->mask & ATTRIBUTE_MASK_SINGLETON
? singletonAttributeLocation(am)
: (hasDynamicAttributeStorage ? emAfEndpoints[ep].dynamicAttributeStorage
: attributeData) + attributeStorageOffset);
;

uint8_t *src, *dst;
if (write)
{
Expand All @@ -622,6 +651,7 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
{
if (buffer == nullptr)
{
// only getting metadata
return EMBER_ZCL_STATUS_SUCCESS;
}

Expand All @@ -644,7 +674,8 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
}

// Internal storage is only supported for fixed endpoints
if (!isDynamicEndpoint)
// and dynamic ones with dynamicAttributeStorage assigned.
if (!isDynamicEndpoint || hasDynamicAttributeStorage)
{
return typeSensitiveMemCopy(attRecord->clusterId, dst, src, am, write, readLength);
}
Expand All @@ -657,7 +688,7 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
// Increase the index if attribute is not externally stored
if (!(am->mask & ATTRIBUTE_MASK_EXTERNAL_STORAGE) && !(am->mask & ATTRIBUTE_MASK_SINGLETON))
{
attributeOffsetIndex = static_cast<uint16_t>(attributeOffsetIndex + emberAfAttributeSize(am));
attributeStorageOffset = static_cast<uint16_t>(attributeStorageOffset + emberAfAttributeSize(am));
}
}
}
Expand All @@ -667,18 +698,18 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
}

// Not the cluster we are looking for
attributeOffsetIndex = static_cast<uint16_t>(attributeOffsetIndex + cluster->clusterSize);
attributeStorageOffset = static_cast<uint16_t>(attributeStorageOffset + cluster->clusterSize);
}

// Cluster is not in the endpoint.
return EMBER_ZCL_STATUS_UNSUPPORTED_CLUSTER;
}

// Not the endpoint we are looking for
// Dynamic endpoints are external and don't factor into storage size
// Dynamic endpoints are external and don't factor into statically allocated global storage size
if (!isDynamicEndpoint)
{
attributeOffsetIndex = static_cast<uint16_t>(attributeOffsetIndex + emAfEndpoints[ep].endpointType->endpointSize);
attributeStorageOffset = static_cast<uint16_t>(attributeStorageOffset + emAfEndpoints[ep].endpointType->endpointSize);
}
}
return EMBER_ZCL_STATUS_UNSUPPORTED_ENDPOINT; // Sorry, endpoint was not found.
Expand Down Expand Up @@ -1183,7 +1214,7 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
uint8_t attrData[ATTRIBUTE_LARGEST];
auto * attrStorage = ignoreStorage ? nullptr : app::GetAttributePersistenceProvider();
// Don't check whether we actually have an attrStorage here, because it's OK
// to have one if none of our attributes have NVM storage.
// to have none if none of our attributes have NVM storage.

for (ep = 0; ep < epCount; ep++)
{
Expand Down
6 changes: 5 additions & 1 deletion src/app/util/attribute-storage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,14 @@ CHIP_ERROR emberAfSetDeviceTypeList(chip::EndpointId endpoint, chip::Span<const
//
// An optional parent endpoint id should be passed for child endpoints of composed device.
//
// An optional dynamicAttributeStorage can be passed to allow automatic attribute storage.
// This must point to a memory block of ep->endpointSize bytes size.
//
EmberAfStatus emberAfSetDynamicEndpoint(uint16_t index, chip::EndpointId id, const EmberAfEndpointType * ep,
const chip::Span<chip::DataVersion> & dataVersionStorage,
chip::Span<const EmberAfDeviceType> deviceTypeList = {},
chip::EndpointId parentEndpointId = chip::kInvalidEndpointId);
chip::EndpointId parentEndpointId = chip::kInvalidEndpointId,
uint8_t * dynamicAttributeStorage = nullptr);
chip::EndpointId emberAfClearDynamicEndpoint(uint16_t index);
uint16_t emberAfGetDynamicIndexFromEndpoint(chip::EndpointId id);

Expand Down

0 comments on commit 485fa6f

Please sign in to comment.