Skip to content

Commit

Permalink
Stop using non-namespaced attribute ids in core SDK code. (#24649)
Browse files Browse the repository at this point in the history
* Stop using non-namespaced attribute ids in core SDK code.

* Also remove now-unused attribute-id.h includes in core code.

* Fix clang-tidy else-after-return complaint.

* Apply suggestions from code review

Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>

* One more "target" addition.

* Fix restyle issue.

Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>
  • Loading branch information
2 people authored and pull[bot] committed Aug 23, 2023
1 parent 41c0cc7 commit 1711049
Show file tree
Hide file tree
Showing 16 changed files with 349 additions and 262 deletions.
20 changes: 14 additions & 6 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,31 +116,31 @@ jobs:
uses: gaurav-nelson/github-action-markdown-link-check@v1

# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for incorrect error use in VerifyOrExit
if: always()
run: |
git grep -n "VerifyOrExit(.*, [A-Za-z]*_ERROR" -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of PRI*8, which are not supported on some libcs.
if: always()
run: |
git grep -n "PRI.8" -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)third_party/lwip/repo/lwip/src/include/lwip/arch.h' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of PRI*16, which are not supported on some libcs.
if: always()
run: |
git grep -n "PRI.16" -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)third_party/lwip/repo/lwip/src/include/lwip/arch.h' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of %zu, which are not supported on some libcs.
if: always()
Expand All @@ -167,17 +167,25 @@ jobs:
scripts/tools/check_test_pics.py src/app/tests/suites/certification/ci-pics-values src/app/tests/suites/certification/PICS.yaml
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of 0x%u and the like, which lead to misleading output.
if: always()
run: |
git grep -n '0x%[0-9l.-]*[^0-9lxX".-]' -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we wasnt to exclude this file,
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of '"0x" PRIu*' and the like, which lead to misleading output.
if: always()
run: |
git grep -n '0x%[0-9-]*" *PRI[^xX]' -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match. And we want to exclude this file,
# to avoid our grep regexp matching itself.
- name: Check for use of legacy non-namespaced attribute ids.
if: always()
run: |
git grep -n '_ATTRIBUTE_ID' -- src ':(exclude)src/app/zap-templates/templates/app/attribute-id.zapt' && exit 1 || exit 0
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*/

#include <app-common/zap-generated/att-storage.h>
#include <app-common/zap-generated/attribute-id.h>
#include <app-common/zap-generated/attribute-type.h>
#include <app-common/zap-generated/attributes/Accessors.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app/AttributeAccessInterface.h>
Expand Down
2 changes: 0 additions & 2 deletions src/app/clusters/groups-server/groups-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
#include "groups-server.h"

#include <app-common/zap-generated/att-storage.h>
#include <app-common/zap-generated/attribute-id.h>
#include <app-common/zap-generated/attribute-type.h>
#include <app-common/zap-generated/attributes/Accessors.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/ids/Clusters.h>
Expand Down
30 changes: 19 additions & 11 deletions src/app/clusters/ias-zone-client/ias-zone-client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
*/

#include "ias-zone-client.h"
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app-common/zap-generated/ids/Commands.h>
#include <app/CommandHandler.h>
#include <app/util/af.h>

using namespace chip;
using namespace chip::app::Clusters::IasZone;
using namespace chip::app::Clusters::IasZone::Commands;

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -344,9 +346,11 @@ static EmberStatus sendCommand(EmberNodeId destAddress)

static void setCieAddress(EmberNodeId destAddress)
{
#error "This needs to be fixed to handle 4-byte attribute ids"

uint8_t writeAttributes[] = {
EMBER_LOW_BYTE(ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID),
EMBER_HIGH_BYTE(ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID),
EMBER_LOW_BYTE(Attributes::IasCieAddress::Id),
EMBER_HIGH_BYTE(Attributes::IasCieAddress::Id),
ZCL_IEEE_ADDRESS_ATTRIBUTE_TYPE,
0,
0,
Expand Down Expand Up @@ -430,12 +434,14 @@ void emberAfPluginIasZoneClientZdoMessageReceivedCallback(EmberNodeId emberNodeI

void readIasZoneServerAttributes(EmberNodeId nodeId)
{
#error "This needs to be fixed to handle 4-byte attribute ids"

uint8_t iasZoneAttributeIds[] = {
EMBER_LOW_BYTE(ZCL_ZONE_STATE_ATTRIBUTE_ID), EMBER_HIGH_BYTE(ZCL_ZONE_STATE_ATTRIBUTE_ID),
EMBER_LOW_BYTE(Attributes::ZoneState::Id), EMBER_HIGH_BYTE(Attributes::ZoneState::Id),

EMBER_LOW_BYTE(ZCL_ZONE_TYPE_ATTRIBUTE_ID), EMBER_HIGH_BYTE(ZCL_ZONE_TYPE_ATTRIBUTE_ID),
EMBER_LOW_BYTE(Attributes::ZoneType::Id), EMBER_HIGH_BYTE(Attributes::ZoneType::Id),

EMBER_LOW_BYTE(ZCL_ZONE_STATUS_ATTRIBUTE_ID), EMBER_HIGH_BYTE(ZCL_ZONE_STATUS_ATTRIBUTE_ID),
EMBER_LOW_BYTE(Attributes::ZoneStatus::Id), EMBER_HIGH_BYTE(Attributes::ZoneStatus::Id),
};
emberAfFillExternalBuffer((ZCL_GLOBAL_COMMAND | ZCL_FRAME_CONTROL_CLIENT_TO_SERVER), app::Clusters::IasZone::Id,
ZCL_READ_ATTRIBUTES_COMMAND_ID, "b", iasZoneAttributeIds, sizeof(iasZoneAttributeIds));
Expand All @@ -447,9 +453,11 @@ void readIasZoneServerAttributes(EmberNodeId nodeId)

void readIasZoneServerCieAddress(EmberNodeId nodeId)
{
#error "This needs to be fixed to handle 4-byte attribute ids"

uint8_t iasZoneAttributeIds[] = {
EMBER_LOW_BYTE(ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID),
EMBER_HIGH_BYTE(ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID),
EMBER_LOW_BYTE(Attributes::IasCieAddress::Id),
EMBER_HIGH_BYTE(Attributes::IasCieAddress::Id),
};
emberAfFillExternalBuffer((ZCL_GLOBAL_COMMAND | ZCL_FRAME_CONTROL_CLIENT_TO_SERVER), app::Clusters::IasZone::Id,
ZCL_READ_ATTRIBUTES_COMMAND_ID, "b", iasZoneAttributeIds, sizeof(iasZoneAttributeIds));
Expand Down Expand Up @@ -495,7 +503,7 @@ void emberAfPluginIasZoneClientReadAttributesResponseCallback(ClusterId clusterI
i++; // skip the type of the attribute. We already know what it should be.
switch (attributeId)
{
case ZCL_ZONE_STATUS_ATTRIBUTE_ID:
case Attributes::ZoneStatus::Id:
if ((i + 2) > bufLen)
{
// Too short, dump the message.
Expand All @@ -505,7 +513,7 @@ void emberAfPluginIasZoneClientReadAttributesResponseCallback(ClusterId clusterI
setServerZoneStatus(currentIndex, zoneStatus);
i += 2;
break;
case ZCL_ZONE_TYPE_ATTRIBUTE_ID:
case Attributes::ZoneType::Id:
if ((i + 2) > bufLen)
{
// Too short, dump the message.
Expand All @@ -515,7 +523,7 @@ void emberAfPluginIasZoneClientReadAttributesResponseCallback(ClusterId clusterI
setServerZoneType(currentIndex, zoneType);
i += 2;
break;
case ZCL_ZONE_STATE_ATTRIBUTE_ID:
case Attributes::ZoneState::Id:
if ((i + 1) > bufLen)
{
// Too short, dump the message
Expand All @@ -525,7 +533,7 @@ void emberAfPluginIasZoneClientReadAttributesResponseCallback(ClusterId clusterI
setServerZoneState(currentIndex, zoneState);
i++;
break;
case ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID: {
case Attributes::IasCieAddress::Id: {
uint8_t myIeee[EUI64_SIZE];
emberAfGetEui64(myIeee);
if ((i + 8) > bufLen)
Expand Down
25 changes: 11 additions & 14 deletions src/app/clusters/ias-zone-server/ias-zone-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "ias-zone-server.h"
#include <app-common/zap-generated/att-storage.h>
#include <app-common/zap-generated/attribute-id.h>
#include <app-common/zap-generated/attribute-type.h>
#include <app-common/zap-generated/callback.h>
#include <app-common/zap-generated/cluster-objects.h>
Expand Down Expand Up @@ -197,7 +196,7 @@ MatterIasZoneClusterServerPreAttributeChangedCallback(const app::ConcreteAttribu

// If this is not a CIE Address write, the CIE address has already been
// written, or the IAS Zone server is already enrolled, do nothing.
if (attributePath.mAttributeId != ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID || emberAfCurrentCommand() == nullptr)
if (attributePath.mAttributeId != Attributes::IasCieAddress::Id || emberAfCurrentCommand() == nullptr)
{
return Protocols::InteractionModel::Status::Success;
}
Expand Down Expand Up @@ -238,7 +237,7 @@ MatterIasZoneClusterServerPreAttributeChangedCallback(const app::ConcreteAttribu
}

zeroAddress = true;
emberAfReadServerAttribute(endpoint, IasZone::Id, ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID, (uint8_t *) ieeeAddress, 8);
emberAfReadServerAttribute(endpoint, IasZone::Id, Attributes::IasCieAddress::Id, (uint8_t *) ieeeAddress, 8);
for (i = 0; i < 8; i++)
{
if (ieeeAddress[i] != 0)
Expand Down Expand Up @@ -298,7 +297,7 @@ bool emberAfIasZoneClusterAmIEnrolled(EndpointId endpoint)
{
EmberAfIasZoneState zoneState = EMBER_ZCL_IAS_ZONE_STATE_NOT_ENROLLED; // Clear this out completely.
EmberAfStatus status;
status = emberAfReadServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_STATE_ATTRIBUTE_ID, (unsigned char *) &zoneState,
status = emberAfReadServerAttribute(endpoint, IasZone::Id, Attributes::ZoneState::Id, (unsigned char *) &zoneState,
1); // uint8_t size

return (status == EMBER_ZCL_STATUS_SUCCESS && zoneState == EMBER_ZCL_IAS_ZONE_STATE_ENROLLED);
Expand All @@ -308,8 +307,7 @@ static void updateEnrollState(EndpointId endpoint, bool enrolled)
{
EmberAfIasZoneState zoneState = (enrolled ? EMBER_ZCL_IAS_ZONE_STATE_ENROLLED : EMBER_ZCL_IAS_ZONE_STATE_NOT_ENROLLED);

emberAfWriteServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_STATE_ATTRIBUTE_ID, (uint8_t *) &zoneState,
ZCL_INT8U_ATTRIBUTE_TYPE);
emberAfWriteServerAttribute(endpoint, IasZone::Id, Attributes::ZoneState::Id, (uint8_t *) &zoneState, ZCL_INT8U_ATTRIBUTE_TYPE);
emberAfIasZoneClusterPrintln("IAS Zone Server State: %pEnrolled", (enrolled ? "" : "NOT "));
}

Expand All @@ -324,7 +322,7 @@ bool emberAfIasZoneClusterZoneEnrollResponseCallback(app::CommandHandler * comma
EmberAfStatus status;

endpoint = emberAfCurrentEndpoint();
status = emberAfReadServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_ID_ATTRIBUTE_ID, &epZoneId, sizeof(uint8_t));
status = emberAfReadServerAttribute(endpoint, IasZone::Id, Attributes::ZoneId::Id, &epZoneId, sizeof(uint8_t));
if (status == EMBER_ZCL_STATUS_SUCCESS)
{
if (enrollResponseCode == EMBER_ZCL_IAS_ENROLL_RESPONSE_CODE_SUCCESS)
Expand Down Expand Up @@ -380,7 +378,7 @@ EmberStatus emberAfPluginIasZoneServerUpdateZoneStatus(EndpointId endpoint, uint
#endif
EmberStatus sendStatus = EMBER_SUCCESS;

emberAfWriteServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_STATUS_ATTRIBUTE_ID, (uint8_t *) &newStatus,
emberAfWriteServerAttribute(endpoint, IasZone::Id, Attributes::ZoneStatus::Id, (uint8_t *) &newStatus,
ZCL_INT16U_ATTRIBUTE_TYPE);

if (enrollmentMethod == EMBER_ZCL_IAS_ZONE_ENROLLMENT_MODE_TRIP_TO_PAIR)
Expand Down Expand Up @@ -523,7 +521,7 @@ void emberAfIasZoneClusterServerInitCallback(EndpointId endpoint)
#endif

zoneType = (EmberAfIasZoneType) EMBER_AF_PLUGIN_IAS_ZONE_SERVER_ZONE_TYPE;
emberAfWriteAttribute(endpoint, IasZone::Id, ZCL_ZONE_TYPE_ATTRIBUTE_ID, (uint8_t *) &zoneType, ZCL_INT16U_ATTRIBUTE_TYPE);
emberAfWriteAttribute(endpoint, IasZone::Id, Attributes::ZoneType::Id, (uint8_t *) &zoneType, ZCL_INT16U_ATTRIBUTE_TYPE);

emberAfPluginIasZoneServerUpdateZoneStatus(endpoint,
0, // status: All alarms cleared
Expand All @@ -538,7 +536,7 @@ void emberAfIasZoneClusterServerTickCallback(EndpointId endpoint)
uint8_t emberAfPluginIasZoneServerGetZoneId(EndpointId endpoint)
{
uint8_t zoneId = UNDEFINED_ZONE_ID;
emberAfReadServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_ID_ATTRIBUTE_ID, &zoneId,
emberAfReadServerAttribute(endpoint, IasZone::Id, Attributes::ZoneId::Id, &zoneId,
emberAfGetDataSize(ZCL_INT8U_ATTRIBUTE_TYPE));
return zoneId;
}
Expand Down Expand Up @@ -566,19 +564,18 @@ static bool areZoneServerAttributesNonVolatile(EndpointId endpoint)
static void setZoneId(EndpointId endpoint, uint8_t zoneId)
{
emberAfIasZoneClusterPrintln("IAS Zone Server Zone ID: 0x%X", zoneId);
emberAfWriteServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_ID_ATTRIBUTE_ID, &zoneId, ZCL_INT8U_ATTRIBUTE_TYPE);
emberAfWriteServerAttribute(endpoint, IasZone::Id, Attributes::ZoneId::Id, &zoneId, ZCL_INT8U_ATTRIBUTE_TYPE);
}

static void unenrollSecurityDevice(EndpointId endpoint)
{
uint8_t ieeeAddress[] = { 0, 0, 0, 0, 0, 0, 0, 0 };
uint16_t zoneType = EMBER_AF_PLUGIN_IAS_ZONE_SERVER_ZONE_TYPE;

emberAfWriteServerAttribute(endpoint, IasZone::Id, ZCL_IAS_CIE_ADDRESS_ATTRIBUTE_ID, (uint8_t *) ieeeAddress,
emberAfWriteServerAttribute(endpoint, IasZone::Id, Attributes::IasCieAddress::Id, (uint8_t *) ieeeAddress,
ZCL_NODE_ID_ATTRIBUTE_TYPE);

emberAfWriteServerAttribute(endpoint, IasZone::Id, ZCL_ZONE_TYPE_ATTRIBUTE_ID, (uint8_t *) &zoneType,
ZCL_INT16U_ATTRIBUTE_TYPE);
emberAfWriteServerAttribute(endpoint, IasZone::Id, Attributes::ZoneType::Id, (uint8_t *) &zoneType, ZCL_INT16U_ATTRIBUTE_TYPE);

setZoneId(endpoint, UNDEFINED_ZONE_ID);
// Restore the enrollment method back to its default value.
Expand Down
2 changes: 0 additions & 2 deletions src/app/clusters/identify-server/identify-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

#include "identify-server.h"

#include <app-common/zap-generated/attribute-id.h>
#include <app-common/zap-generated/attribute-type.h>
#include <app-common/zap-generated/attributes/Accessors.h>
#include <app-common/zap-generated/callback.h>
#include <app-common/zap-generated/cluster-objects.h>
Expand Down
1 change: 0 additions & 1 deletion src/app/clusters/mode-select-server/mode-select-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include <app-common/zap-generated/af-structs.h>
#include <app-common/zap-generated/att-storage.h>
#include <app-common/zap-generated/attribute-type.h>
#include <app-common/zap-generated/attributes/Accessors.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/ids/Attributes.h>
Expand Down
Loading

0 comments on commit 1711049

Please sign in to comment.