Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global Attribute ClusterRevision looks broken for clusters of bridged devices (#9823) #10412

Merged
merged 2 commits into from
Oct 18, 2021
Merged

Global Attribute ClusterRevision looks broken for clusters of bridged devices (#9823) #10412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb27a49
Return an error when trying to read/write internal stored attributes …
pjzander-signify Oct 1, 2021
3beebef
Change dynamic endpoints so that the cluster revision is read externally
pjzander-signify Oct 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions examples/bridge-app/esp32/main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,26 +75,26 @@ static uint8_t gDescriptorAttrStorage[DYNAMIC_ENDPOINT_COUNT][kDescriptorAttribu
// Declare On/Off cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(onOffAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* on/off */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Descriptor cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_DEVICE_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_SERVER_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CLIENT_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0) /* parts list */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Bridged Device Basic information cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(bridgedDeviceBasicAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_USER_LABEL_ATTRIBUTE_ID, CHAR_STRING, kUserLabelSize, 0), /* UserLabel */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* Reachable */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0003);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Fixed Label cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(fixedLabelAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0) /* label list */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Cluster List for Bridged Light endpoint
DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedLightClusters)
Expand All @@ -105,6 +105,14 @@ DECLARE_DYNAMIC_CLUSTER(ZCL_ON_OFF_CLUSTER_ID, onOffAttrs), DECLARE_DYNAMIC_CLUS
// Declare Bridged Light endpoint
DECLARE_DYNAMIC_ENDPOINT(bridgedLightEndpoint, bridgedLightClusters);

/* REVISION definitions:
*/

#define ZCL_DESCRIPTOR_CLUSTER_REVISION (1u)
pjzander-signify marked this conversation as resolved.
Show resolved Hide resolved
#define ZCL_BRIDGED_DEVICE_BASIC_CLUSTER_REVISION (1u)
#define ZCL_FIXED_LABEL_CLUSTER_REVISION (1u)
#define ZCL_ON_OFF_CLUSTER_REVISION (4u)

CHIP_ERROR AddDeviceEndpoint(Device * dev, EmberAfEndpointType * ep, uint16_t deviceType)
{
uint8_t index = 0;
Expand Down Expand Up @@ -191,6 +199,10 @@ EmberAfStatus HandleReadBridgedDeviceBasicAttribute(Device * dev, chip::Attribut
{
ToZclCharString(buffer, dev->GetName(), static_cast<uint8_t>(maxReadLength - 1));
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_BRIDGED_DEVICE_BASIC_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
Expand All @@ -205,30 +217,54 @@ EmberAfStatus HandleReadDescriptorAttribute(uint16_t endpointIndex, chip::Attrib
if ((maxReadLength <= kDescriptorAttributeArraySize) && (attributeId < kDescriptorAttributeCount))
{
memcpy(buffer, &gDescriptorAttrStorage[endpointIndex][attributeId][0], maxReadLength);
return EMBER_ZCL_STATUS_SUCCESS;
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_DESCRIPTOR_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}

return EMBER_ZCL_STATUS_FAILURE;
return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus HandleReadFixedLabelAttribute(Device * dev, EmberAfAttributeMetadata * am, uint8_t * buffer, uint16_t maxReadLength)
{
if ((am->attributeId == ZCL_LABEL_LIST_ATTRIBUTE_ID) && (maxReadLength <= kFixedLabelAttributeArraySize))
{
EncodeFixedLabel("room", dev->GetLocation(), buffer, maxReadLength, am);

return EMBER_ZCL_STATUS_SUCCESS;
}
else if ((am->attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_FIXED_LABEL_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}

return EMBER_ZCL_STATUS_FAILURE;
return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus HandleReadOnOffAttribute(Device * dev, chip::AttributeId attributeId, uint8_t * buffer, uint16_t maxReadLength)
{
ChipLogProgress(DeviceLayer, "HandleReadOnOffAttribute: attrId=%d, maxReadLength=%d", attributeId, maxReadLength);

ReturnErrorCodeIf((attributeId != ZCL_ON_OFF_ATTRIBUTE_ID) || (maxReadLength != 1), EMBER_ZCL_STATUS_FAILURE);
*buffer = dev->IsOn() ? 1 : 0;
if ((attributeId == ZCL_ON_OFF_ATTRIBUTE_ID) && (maxReadLength == 1))
{
*buffer = dev->IsOn() ? 1 : 0;
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_ON_OFF_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}

return EMBER_ZCL_STATUS_SUCCESS;
}

Expand Down
47 changes: 38 additions & 9 deletions examples/bridge-app/linux/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,26 +99,26 @@ static uint8_t gDescriptorAttrStorage[DYNAMIC_ENDPOINT_COUNT][kDescriptorAttribu
// Declare On/Off cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(onOffAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_ON_OFF_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* on/off */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Descriptor cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(descriptorAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_DEVICE_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* device list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_SERVER_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* server list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_CLIENT_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0), /* client list */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_PARTS_LIST_ATTRIBUTE_ID, ARRAY, kDescriptorAttributeArraySize, 0) /* parts list */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Bridged Device Basic information cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(bridgedDeviceBasicAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_USER_LABEL_ATTRIBUTE_ID, CHAR_STRING, kUserLabelSize, 0), /* UserLabel */
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_REACHABLE_ATTRIBUTE_ID, BOOLEAN, 1, 0) /* Reachable */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0003);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Fixed Label cluster attributes
DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(fixedLabelAttrs)
DECLARE_DYNAMIC_ATTRIBUTE(ZCL_LABEL_LIST_ATTRIBUTE_ID, ARRAY, kFixedLabelAttributeArraySize, 0) /* label list */
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(0x0001);
DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();

// Declare Cluster List for Bridged Light endpoint
DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedLightClusters)
Expand All @@ -129,6 +129,14 @@ DECLARE_DYNAMIC_CLUSTER(ZCL_ON_OFF_CLUSTER_ID, onOffAttrs), DECLARE_DYNAMIC_CLUS
// Declare Bridged Light endpoint
DECLARE_DYNAMIC_ENDPOINT(bridgedLightEndpoint, bridgedLightClusters);

// REVISION DEFINITIONS:
// =================================================================================

#define ZCL_DESCRIPTOR_CLUSTER_REVISION (1u)
#define ZCL_BRIDGED_DEVICE_BASIC_CLUSTER_REVISION (1u)
#define ZCL_FIXED_LABEL_CLUSTER_REVISION (1u)
#define ZCL_ON_OFF_CLUSTER_REVISION (4u)

// ---------------------------------------------------------------------------

int AddDeviceEndpoint(Device * dev, EmberAfEndpointType * ep, uint16_t deviceType)
Expand Down Expand Up @@ -258,6 +266,10 @@ EmberAfStatus HandleReadBridgedDeviceBasicAttribute(Device * dev, chip::Attribut
MakeZclCharString(zclString, dev->GetName());
buffer = zclString.data();
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_BRIDGED_DEVICE_BASIC_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
Expand All @@ -274,6 +286,10 @@ EmberAfStatus HandleReadOnOffAttribute(Device * dev, chip::AttributeId attribute
{
*buffer = dev->IsOn() ? 1 : 0;
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_ON_OFF_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
Expand Down Expand Up @@ -311,22 +327,35 @@ EmberAfStatus HandleReadDescriptorAttribute(uint16_t endpointIndex, chip::Attrib
if ((maxReadLength <= kDescriptorAttributeArraySize) && (attributeId < kDescriptorAttributeCount))
{
memcpy(buffer, &gDescriptorAttrStorage[endpointIndex][attributeId][0], maxReadLength);
return EMBER_ZCL_STATUS_SUCCESS;
}
else if ((attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_DESCRIPTOR_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}

return EMBER_ZCL_STATUS_FAILURE;
return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus HandleReadFixedLabelAttribute(Device * dev, EmberAfAttributeMetadata * am, uint8_t * buffer, uint16_t maxReadLength)
{
if ((am->attributeId == ZCL_LABEL_LIST_ATTRIBUTE_ID) && (maxReadLength <= kFixedLabelAttributeArraySize))
{
EncodeFixedLabel("room", dev->GetLocation(), buffer, maxReadLength, am);

return EMBER_ZCL_STATUS_SUCCESS;
}
else if ((am->attributeId == ZCL_CLUSTER_REVISION_SERVER_ATTRIBUTE_ID) && (maxReadLength == 2))
{
*buffer = (uint16_t) ZCL_FIXED_LABEL_CLUSTER_REVISION;
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}

return EMBER_ZCL_STATUS_FAILURE;
return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus HandleWriteDescriptorAttribute(uint16_t endpointIndex, EmberAfAttributeMetadata * am, uint8_t * buffer,
Expand Down
48 changes: 32 additions & 16 deletions src/app/util/attribute-storage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,16 +586,18 @@ bool emAfMatchAttribute(EmberAfCluster * cluster, EmberAfAttributeMetadata * am,
EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord, EmberAfAttributeMetadata ** metadata,
uint8_t * buffer, uint16_t readLength, bool write, int32_t index)
{
uint8_t i;
uint16_t attributeOffsetIndex = 0;

for (i = 0; i < emberAfEndpointCount(); i++)
for (uint8_t ep = 0; ep < emberAfEndpointCount(); ep++)
{
if (emAfEndpoints[i].endpoint == attRecord->endpoint)
// Is this a dynamic endpoint?
bool isDynamicEndpoint = (ep >= emberAfFixedEndpointCount());

if (emAfEndpoints[ep].endpoint == attRecord->endpoint)
{
EmberAfEndpointType * endpointType = emAfEndpoints[i].endpointType;
EmberAfEndpointType * endpointType = emAfEndpoints[ep].endpointType;
uint8_t clusterIndex;
if (!emberAfEndpointIndexIsEnabled(i))
if (!emberAfEndpointIndexIsEnabled(ep))
{
continue;
}
Expand Down Expand Up @@ -649,15 +651,29 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
}
}

return (am->mask & ATTRIBUTE_MASK_EXTERNAL_STORAGE
? (write) ? emberAfExternalAttributeWriteCallback(
attRecord->endpoint, attRecord->clusterId, am,
emAfGetManufacturerCodeForAttribute(cluster, am), buffer, index)
: emberAfExternalAttributeReadCallback(
attRecord->endpoint, attRecord->clusterId, am,
emAfGetManufacturerCodeForAttribute(cluster, am), buffer,
emberAfAttributeSize(am), index)
: typeSensitiveMemCopy(attRecord->clusterId, dst, src, am, write, readLength, index));
// Is the attribute externally stored?
if (am->mask & ATTRIBUTE_MASK_EXTERNAL_STORAGE)
{
return (write ? emberAfExternalAttributeWriteCallback(
attRecord->endpoint, attRecord->clusterId, am,
emAfGetManufacturerCodeForAttribute(cluster, am), buffer, index)
: emberAfExternalAttributeReadCallback(
attRecord->endpoint, attRecord->clusterId, am,
emAfGetManufacturerCodeForAttribute(cluster, am), buffer,
emberAfAttributeSize(am), index));
}
else
{
// Internal storage is only supported for fixed endpoints
if (!isDynamicEndpoint)
{
return typeSensitiveMemCopy(attRecord->clusterId, dst, src, am, write, readLength, index);
}
else
{
return EMBER_ZCL_STATUS_FAILURE;
}
}
}
}
else
Expand All @@ -679,9 +695,9 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
else
{ // Not the endpoint we are looking for
// Dynamic endpoints are external and don't factor into storage size
if (i < emberAfFixedEndpointCount())
if (!isDynamicEndpoint)
{
attributeOffsetIndex = static_cast<uint16_t>(attributeOffsetIndex + emAfEndpoints[i].endpointType->endpointSize);
attributeOffsetIndex = static_cast<uint16_t>(attributeOffsetIndex + emAfEndpoints[ep].endpointType->endpointSize);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/util/attribute-storage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@

#define DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(attrListName) EmberAfAttributeMetadata attrListName[] = {

#define DECLARE_DYNAMIC_ATTRIBUTE_LIST_END(clusterRevision) \
, { 0xFFFD, ZAP_TYPE(INT16U), 2, 0, ZAP_SIMPLE_DEFAULT(clusterRevision) } /* cluster revision */ \
#define DECLARE_DYNAMIC_ATTRIBUTE_LIST_END() \
, { 0xFFFD, ZAP_TYPE(INT16U), 2, ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE), ZAP_EMPTY_DEFAULT() } /* cluster revision */ \
}

#define DECLARE_DYNAMIC_ATTRIBUTE(attId, attType, attSizeBytes, attrMask) \
Expand Down