Skip to content

Commit

Permalink
Make sure we're not using non-type-safe attribute writes. (#25057)
Browse files Browse the repository at this point in the history
Gets rid of the emberAfWriteServerAttribute define, and adds a lint to restrict
emberAfWriteAttribute use to the implementation of the type-safe writes, plus
some pigweed RPC code.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 7, 2023
1 parent e1a5829 commit 1067975
Show file tree
Hide file tree
Showing 6 changed files with 1,076 additions and 1,141 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,13 @@ jobs:
if: always()
run: |
git grep -n 'emberAfReadAttribute' -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)src/app/util/af.h' ':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' ':(exclude)src/app/zap-templates/templates/app/attributes/Accessors-src.zapt' ':(exclude)src/app/util/attribute-table.cpp' && 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, as well as excluding the files
# that implement the type-safe accessors, attribute writing from the wire, and some
# Pigweed RPC code that seems hard to update.
- name: Check for use of 'emberAfWriteAttribute' instead of the type-safe setters
if: always()
run: |
git grep -n 'emberAfWriteAttribute' -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)src/app/util/af.h' ':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' ':(exclude)src/app/zap-templates/templates/app/attributes/Accessors-src.zapt' ':(exclude)src/app/util/attribute-table.cpp' ':(exclude)examples/common/pigweed/rpc_services/Attributes.h' ':(exclude)src/app/util/attribute-table.h' ':(exclude)src/app/util/ember-compatibility-functions.cpp' && exit 1 || exit 0
2 changes: 1 addition & 1 deletion examples/contact-sensor-app/telink/src/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ void AppTask::UpdateClusterStateInternal(intptr_t arg)
{
uint8_t newValue = ContactSensorMgr().IsContactClosed();

ChipLogProgress(NotSpecified, "emberAfWriteAttribute : %d", newValue);
ChipLogProgress(NotSpecified, "StateValue::Set : %d", newValue);

// write the new boolean state value
EmberAfStatus status = app::Clusters::BooleanState::Attributes::StateValue::Set(1, newValue);
Expand Down
16 changes: 12 additions & 4 deletions examples/lock-app/nxp/k32w/k32w0/main/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -758,12 +758,20 @@ void AppTask::UpdateClusterState(void)
}
void AppTask::UpdateClusterStateInternal(intptr_t arg)
{
uint8_t newValue = !BoltLockMgr().IsUnlocked();
using namespace chip::app::Clusters::DoorLock;

DlLockState newValue;
if (BoltLockMgr().IsUnlocked())
{
newValue = DlLockState::kUnlocked;
}
else
{
newValue = DlLockState::kLocked;
}

// write the new door lock state
EmberAfStatus status =
emberAfWriteAttribute(1, chip::app::Clusters::DoorLock::Id, chip::app::Clusters::DoorLock::Attributes::LockState::Id,
(uint8_t *) &newValue, ZCL_ENUM8_ATTRIBUTE_TYPE);
EmberAfStatus status = Attributes::LockState::Set(1, newValue);

if (status != EMBER_ZCL_STATUS_SUCCESS)
{
Expand Down
5 changes: 0 additions & 5 deletions src/app/util/af.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,6 @@ bool emberAfContainsClient(chip::EndpointId endpoint, chip::ClusterId clusterId)
EmberAfStatus emberAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
uint8_t * dataPtr, EmberAfAttributeType dataType);

// For now, just define emberAfWriteServerAttribute to emberAfWriteAttribute, to
// minimize code churn.
// TODO: Remove this define.
#define emberAfWriteServerAttribute emberAfWriteAttribute

/**
* @brief Read the attribute value, performing all the checks.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ EmberAfStatus Set(chip::EndpointId endpoint, {{zapTypeToEncodableClusterObjectTy
uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
emberAfCopyInt{{#if (isShortString type)}}8{{else}}16{{/if}}u(zclString, 0, static_cast<{{>lengthType}}>(value.size()));
memcpy(&zclString[{{>sizingBytes}}], value.data(), value.size());
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
return emberAfWriteAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{else}}
using Traits = NumericAttributeTraits<{{accessorTraitType type}}>;
if (!Traits::CanRepresentValue(/* isNullable = */ {{isNullable}}, value))
Expand All @@ -102,7 +102,7 @@ EmberAfStatus Set(chip::EndpointId endpoint, {{zapTypeToEncodableClusterObjectTy
Traits::StorageType storageValue;
Traits::WorkingToStorage(value, storageValue);
uint8_t * writable = Traits::ToAttributeStoreRepresentation(storageValue);
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, writable, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
return emberAfWriteAttribute(endpoint, {{>clusterId}}, Id, writable, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{/if}}
}

Expand All @@ -111,13 +111,13 @@ EmberAfStatus SetNull(chip::EndpointId endpoint)
{
{{#if (isString type)}}
uint8_t zclString[{{>sizingBytes}}] = { {{#if (isShortString type)}}0xFF{{else}}0xFF, 0xFF{{/if}} };
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
return emberAfWriteAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{else}}
using Traits = NumericAttributeTraits<{{accessorTraitType type}}>;
Traits::StorageType value;
Traits::SetNull(value);
uint8_t * writable = Traits::ToAttributeStoreRepresentation(value);
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, writable, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
return emberAfWriteAttribute(endpoint, {{>clusterId}}, Id, writable, ZCL_{{typeAsDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{/if}}
}

Expand Down
Loading

0 comments on commit 1067975

Please sign in to comment.