Skip to content

Commit

Permalink
Address code review comments on zcl callbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
praveenCY committed Nov 3, 2021
1 parent 569e2e6 commit 85b75ce
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 33 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/examples-infineon.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ jobs:
timeout-minutes: 10
run: |
scripts/run_in_build_env.sh \
"scripts/build/build_examples.py --no-log-timestamps --target-glob 'infineon-p6-lock' build"
"scripts/build/build_examples.py --no-log-timestamps --target 'infineon-p6-lock' build"
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
p6 default lock-app \
out/infineon-p6-lock/chip-p6-lock-example.out
- name: Build all-clusters-app example
timeout-minutes: 10
run: |
scripts/run_in_build_env.sh \
"scripts/build/build_examples.py --no-log-timestamps --target-glob 'infineon-p6-all-clusters' build"
"scripts/build/build_examples.py --no-log-timestamps --target 'infineon-p6-all-clusters' build"
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
p6 default all-clusters-app \
out/infineon-p6-all-clusters/chip-p6-clusters-example.out
Expand Down
6 changes: 4 additions & 2 deletions examples/all-clusters-app/p6/src/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "qrcodegen.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-id.h>
#include <app/server/Dnssd.h>
#include <app/server/OnboardingCodesUtil.h>
Expand Down Expand Up @@ -62,6 +63,7 @@ StaticTask_t appTaskStruct;
using namespace chip::TLV;
using namespace ::chip::Credentials;
using namespace ::chip::DeviceLayer;
using namespace ::chip::app::Clusters;

AppTask AppTask::sAppTask;

Expand Down Expand Up @@ -218,9 +220,9 @@ void AppTask::DispatchEvent(AppEvent * aEvent)
void AppTask::OnOffUpdateClusterState(void)
{
uint8_t newValue = sLightLED.Get();

// write the new on/off value
EmberAfStatus status = emberAfWriteAttribute(2, ZCL_ON_OFF_CLUSTER_ID, ZCL_ON_OFF_ATTRIBUTE_ID, CLUSTER_MASK_SERVER, &newValue,
ZCL_BOOLEAN_ATTRIBUTE_TYPE);
EmberAfStatus status = OnOff::Attributes::OnOff::Set(2, newValue);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
P6_LOG("ERR: updating on/off %x", status);
Expand Down
24 changes: 13 additions & 11 deletions examples/all-clusters-app/p6/src/ClusterManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "AppConfig.h"
#include "LEDWidget.h"
#include <app-common/zap-generated/attribute-id.h>
#include <app-common/zap-generated/attributes/Accessors.h>
#include <app-common/zap-generated/cluster-id.h>
#include <app/Command.h>
#include <app/server/Dnssd.h>
Expand All @@ -49,7 +50,8 @@ ClusterManager ClusterManager::sCluster;

void ClusterManager::OnOnOffPostAttributeChangeCallback(EndpointId endpointId, AttributeId attributeId, uint8_t * value)
{
VerifyOrExit(attributeId == ZCL_ON_OFF_ATTRIBUTE_ID, P6_LOG("Unhandled Attribute ID: '0x%04x", attributeId));
VerifyOrExit(attributeId == ZCL_ON_OFF_ATTRIBUTE_ID,
P6_LOG("Unhandled Attribute ID: '" ChipLogFormatMEI "'", ChipLogValueMEI(attributeId)));
VerifyOrExit(endpointId == ENDPOINT_FIRST_IDX || endpointId == ENDPOINT_SECOND_IDX,
P6_LOG("Unexpected EndPoint ID: `0x%02x'", endpointId));

Expand All @@ -68,7 +70,8 @@ void ClusterManager::OnLevelControlAttributeChangeCallback(EndpointId endpointId
bool onOffState = mEndpointOnOffState[endpointId - 1];
uint8_t brightness = onOffState ? *value : 0;

VerifyOrExit(attributeId == ZCL_CURRENT_LEVEL_ATTRIBUTE_ID, P6_LOG("Unhandled Attribute ID: '0x%04x", attributeId));
VerifyOrExit(attributeId == ZCL_CURRENT_LEVEL_ATTRIBUTE_ID,
P6_LOG("Unhandled Attribute ID: '" ChipLogFormatMEI "'", ChipLogValueMEI(attributeId)));
VerifyOrExit(endpointId == ENDPOINT_FIRST_IDX || endpointId == ENDPOINT_SECOND_IDX,
P6_LOG("Unexpected EndPoint ID: `0x%02x'", endpointId));

Expand All @@ -80,9 +83,9 @@ void ClusterManager::OnLevelControlAttributeChangeCallback(EndpointId endpointId

void ClusterManager::OnColorControlAttributeChangeCallback(EndpointId endpointId, AttributeId attributeId, uint8_t * value)
{
VerifyOrExit(attributeId == ZCL_COLOR_CONTROL_CURRENT_HUE_ATTRIBUTE_ID ||
attributeId == ZCL_COLOR_CONTROL_CURRENT_SATURATION_ATTRIBUTE_ID,
P6_LOG("Unhandled AttributeId ID: '0x%04x", attributeId));
VerifyOrExit(attributeId == ColorControl::Attributes::CurrentHue::Id ||
attributeId == ColorControl::Attributes::CurrentSaturation::Id,
P6_LOG("Unhandled Attribute ID: '" ChipLogFormatMEI "'", ChipLogValueMEI(attributeId)));
VerifyOrExit(endpointId == ENDPOINT_FIRST_IDX || endpointId == ENDPOINT_SECOND_IDX,
P6_LOG("Unexpected EndPoint ID: `0x%02x'", endpointId));
if (endpointId == 1)
Expand All @@ -91,12 +94,11 @@ void ClusterManager::OnColorControlAttributeChangeCallback(EndpointId endpointId
/* If the Current Attribute is ZCL_COLOR_CONTROL_CURRENT_HUE_ATTRIBUTE_ID, read the saturation value and
* set the color on Cluster LED using both Saturation and Hue.
*/
if (attributeId == ZCL_COLOR_CONTROL_CURRENT_HUE_ATTRIBUTE_ID)
if (attributeId == ColorControl::Attributes::CurrentHue::Id)
{
hue = *value;
/* Read Current Saturation value when Attribute change callback for HUE Attribute */
emberAfReadServerAttribute(endpointId, ZCL_COLOR_CONTROL_CLUSTER_ID, ZCL_COLOR_CONTROL_CURRENT_SATURATION_ATTRIBUTE_ID,
&saturation, sizeof(uint8_t));
ColorControl::Attributes::CurrentSaturation::Get(endpointId, &saturation);
}
else
{
Expand All @@ -105,8 +107,7 @@ void ClusterManager::OnColorControlAttributeChangeCallback(EndpointId endpointId
*/
saturation = *value;
/* Read Current Hue value when Attribute change callback for SATURATION Attribute */
emberAfReadServerAttribute(endpointId, ZCL_COLOR_CONTROL_CLUSTER_ID, ZCL_COLOR_CONTROL_CURRENT_HUE_ATTRIBUTE_ID, &hue,
sizeof(uint8_t));
ColorControl::Attributes::CurrentHue::Get(endpointId, &hue);
}
/* Set RGB Color on Cluster Indication LED */
sClusterLED.SetColor(hue, saturation);
Expand Down Expand Up @@ -135,7 +136,8 @@ void IdentifyTimerHandler(Layer * systemLayer, void * appState)

void ClusterManager::OnIdentifyPostAttributeChangeCallback(EndpointId endpointId, AttributeId attributeId, uint8_t * value)
{
VerifyOrExit(attributeId == ZCL_IDENTIFY_TIME_ATTRIBUTE_ID, P6_LOG("Unhandled Attribute ID: '0x%04x", attributeId));
VerifyOrExit(attributeId == Identify::Attributes::IdentifyTime::Id,
P6_LOG("Unhandled Attribute ID: '" ChipLogFormatMEI "'", ChipLogValueMEI(attributeId)));
VerifyOrExit(endpointId == ENDPOINT_FIRST_IDX, P6_LOG("Unexpected EndPoint ID: `0x%02x'", endpointId));

/* IdentifyTime Attribute Spec mentions "flashing a light with a period of 0.5 seconds" */
Expand Down
16 changes: 9 additions & 7 deletions examples/all-clusters-app/p6/src/ZclCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,29 @@ void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath &
EndpointId endpoint = attributePath.mEndpointId;
ClusterId clusterId = attributePath.mClusterId;
AttributeId attributeId = attributePath.mAttributeId;
P6_LOG("emberAfPostAttributeChangeCallback - Cluster ID: '0x%04x', EndPoint ID: '0x%02x', Attribute ID: '0x%04x'", clusterId,
endpoint, attributeId);
P6_LOG("MatterPostAttributeChangeCallback - Cluster ID: " ChipLogFormatMEI
", EndPoint ID: '0x%02x', Attribute ID: " ChipLogFormatMEI,
ChipLogValueMEI(clusterId), endpoint, ChipLogValueMEI(attributeId));

switch (clusterId)
{
case ZCL_ON_OFF_CLUSTER_ID:
case OnOff::Id:
ClusterMgr().OnOnOffPostAttributeChangeCallback(endpoint, attributeId, value);
break;

case ZCL_IDENTIFY_CLUSTER_ID:
case Identify::Id:
ClusterMgr().OnIdentifyPostAttributeChangeCallback(endpoint, attributeId, value);
break;

case ZCL_LEVEL_CONTROL_CLUSTER_ID:
case LevelControl::Id:
ClusterMgr().OnLevelControlAttributeChangeCallback(endpoint, attributeId, value);
break;

case ZCL_COLOR_CONTROL_CLUSTER_ID:
case ColorControl::Id:
ClusterMgr().OnColorControlAttributeChangeCallback(endpoint, attributeId, value);
break;
default:
P6_LOG("Unhandled cluster ID: %d", clusterId);
P6_LOG("Unhandled cluster ID: " ChipLogFormatMEI, ChipLogValueMEI(clusterId));
break;
}
}
Expand Down
10 changes: 5 additions & 5 deletions examples/platform/p6/LEDWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class LEDWidget

private:
uint64_t mLastChangeTimeMS = 0;
uint32_t mBlinkOnTimeMS = 0;
uint32_t mBlinkOffTimeMS = 0;
int mLedNum = 0;
bool mState = 0;
uint8_t mbrightness = 0;
uint32_t mBlinkOnTimeMS = 0;
uint32_t mBlinkOffTimeMS = 0;
int mLedNum = 0;
bool mState = 0;
uint8_t mbrightness = 0;
uint16_t mHue;
uint8_t mSaturation;
cyhal_pwm_t pwm_red;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(
state.currentPosition = getCurrentPosition(endpoint);
state.targetPosition = percentOpen;
state.delayMs = calculateDelayMs(endpoint, state.targetPosition, &state.increasing);
emberAfBarrierControlClusterPrintln("Scheduling barrier move from %d to %d with %dms delay", state.currentPosition,
state.targetPosition, (unsigned int) state.delayMs);
emberAfBarrierControlClusterPrintln("Scheduling barrier move from %d to %d with %" PRIu32 "ms delay", state.currentPosition,
state.targetPosition, state.delayMs);
emberAfScheduleServerTick(endpoint, BarrierControl::Id, state.delayMs);

if (state.currentPosition < state.targetPosition)
Expand Down
4 changes: 2 additions & 2 deletions src/app/clusters/door-lock-server/door-lock-server-core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ void emAfPluginDoorLockServerWriteAttributes(const EmAfPluginDoorLockServerAttri
(uint8_t *) &data[i].value, ZCL_INT16U_ATTRIBUTE_TYPE);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfDoorLockClusterPrintln("Failed to write %s attribute 0x%2X: 0x%X", description, (unsigned int) data[i].id,
status);
emberAfDoorLockClusterPrintln("Failed to write %s attribute " ChipLogFormatMEI ": 0x%X", description,
ChipLogValueMEI(data[i].id), status);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/util/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ static bool dispatchZclMessage(EmberAfClusterCommand * cmd)
else if ((cmd->type == EMBER_INCOMING_MULTICAST || cmd->type == EMBER_INCOMING_MULTICAST_LOOPBACK) &&
!emberAfGroupsClusterEndpointInGroupCallback(cmd->apsFrame->destinationEndpoint, cmd->apsFrame->groupId))
{
emberAfDebugPrint("Drop cluster 0x%2x command 0x%x", (unsigned int) cmd->apsFrame->clusterId,
(unsigned int) cmd->commandId);
emberAfDebugPrint("Drop cluster " ChipLogFormatMEI " command " ChipLogFormatMEI, ChipLogValueMEI(cmd->apsFrame->clusterId),
ChipLogValueMEI(cmd->commandId));
emberAfDebugPrint(" for endpoint 0x%x due to wrong %s: ", cmd->apsFrame->destinationEndpoint, "group");
emberAfDebugPrintln("0x%02x", cmd->apsFrame->groupId);
return false;
Expand Down

0 comments on commit 85b75ce

Please sign in to comment.