From d5c6eb6cf695f302e5890fd5a0d492c6de13fa7d Mon Sep 17 00:00:00 2001 From: eve-cxrp <80681009+eve-cxrp@users.noreply.github.com> Date: Tue, 28 Sep 2021 16:22:21 +0200 Subject: [PATCH] Fix Identify cluster callback handling (#9988) * fix idenitfy cluster callback handling The current identify cluster could only handle one identify request without an error. After that it would get stuck and would not perform another identify start callback. * make compiler & CI happy * refactor activate logic in identify-server --- .../identify-server/identify-server.cpp | 151 +++++++++--------- 1 file changed, 76 insertions(+), 75 deletions(-) diff --git a/src/app/clusters/identify-server/identify-server.cpp b/src/app/clusters/identify-server/identify-server.cpp index 33b6e6d250ee9c..5cb582799c3cb8 100644 --- a/src/app/clusters/identify-server/identify-server.cpp +++ b/src/app/clusters/identify-server/identify-server.cpp @@ -100,72 +100,6 @@ static inline void unreg(Identify * inst) } } -static void scheduleIdentifyTick(EndpointId endpoint) -{ - Identify * identify = inst(endpoint); - uint16_t identifyTime; - - if (identify == nullptr) - { - return; - } - - if (EMBER_ZCL_STATUS_SUCCESS == Clusters::Identify::Attributes::GetIdentifyTime(endpoint, &identifyTime)) - { - /* effect identifier changed during identify */ - if (identify->mTargetEffectIdentifier != identify->mCurrentEffectIdentifier) - { - identify->mCurrentEffectIdentifier = identify->mTargetEffectIdentifier; - - /* finish identify process */ - if (EMBER_ZCL_IDENTIFY_EFFECT_IDENTIFIER_FINISH_EFFECT == identify->mCurrentEffectIdentifier && identifyTime > 0) - { - (void) chip::DeviceLayer::SystemLayer().StartTimer(MILLISECOND_TICKS_PER_SECOND, onIdentifyClusterTick, identify); - return; - } - /* stop identify process */ - else if (EMBER_ZCL_IDENTIFY_EFFECT_IDENTIFIER_STOP_EFFECT == identify->mCurrentEffectIdentifier && identifyTime > 0) - { - Clusters::Identify::Attributes::SetIdentifyTime(endpoint, 0); - - if (nullptr != identify->mOnIdentifyStop) - identify->mOnIdentifyStop(identify); - } - /* change from e.g. Breathe to Blink during identify */ - else - { - /* cancel identify */ - Clusters::Identify::Attributes::SetIdentifyTime(endpoint, 0); - if (nullptr != identify->mOnIdentifyStop) - identify->mOnIdentifyStop(identify); - - /* trigger effect identifier callback */ - if (nullptr != identify->mOnEffectIdentifier) - identify->mOnEffectIdentifier(identify); - } - } - else if (identifyTime > 0) - { - /* we only start if both callbacks are set */ - if (nullptr != identify->mOnIdentifyStart && nullptr != identify->mOnIdentifyStop && false == identify->mActive) - { - identify->mActive = true; - identify->mOnIdentifyStart(identify); - } - - (void) chip::DeviceLayer::SystemLayer().StartTimer(MILLISECOND_TICKS_PER_SECOND, onIdentifyClusterTick, identify); - return; - } - else - { - if (nullptr != identify->mOnIdentifyStop) - identify->mOnIdentifyStop(identify); - } - } - - (void) chip::DeviceLayer::SystemLayer().CancelTimer(onIdentifyClusterTick, identify); -} - void emberAfIdentifyClusterServerInitCallback(EndpointId endpoint) { (void) endpoint; @@ -183,24 +117,91 @@ static void onIdentifyClusterTick(chip::System::Layer * systemLayer, void * appS if (EMBER_ZCL_STATUS_SUCCESS == Clusters::Identify::Attributes::GetIdentifyTime(endpoint, &identifyTime) && 0 != identifyTime) { + identifyTime = static_cast(identifyTime == 0 ? 0 : identifyTime - 1); // This tick writes the new attribute, which will trigger the Attribute - // Changed callback below, which in turn will schedule or cancel the tick. - // Because of this, the tick does not have to be scheduled here. - (void) Clusters::Identify::Attributes::SetIdentifyTime(endpoint, - static_cast(identifyTime == 0 ? 0 : identifyTime - 1)); - } - else - { - identify->mActive = false; + // Changed callback. + (void) Clusters::Identify::Attributes::SetIdentifyTime(endpoint, identifyTime); } } } +static inline void identify_activate(Identify * identify) +{ + if (nullptr != identify->mOnIdentifyStart && nullptr != identify->mOnIdentifyStop && false == identify->mActive) + { + identify->mActive = true; + identify->mOnIdentifyStart(identify); + } +} + +static inline void identify_deactivate(Identify * identify) +{ + if (nullptr != identify->mOnIdentifyStop) + { + identify->mActive = false; + identify->mOnIdentifyStop(identify); + } +} + void emberAfIdentifyClusterServerAttributeChangedCallback(EndpointId endpoint, AttributeId attributeId) { if (attributeId == Clusters::Identify::Attributes::Ids::IdentifyTime) { - scheduleIdentifyTick(endpoint); + Identify * identify = inst(endpoint); + uint16_t identifyTime; + + if (identify == nullptr) + { + return; + } + + if (EMBER_ZCL_STATUS_SUCCESS == Clusters::Identify::Attributes::GetIdentifyTime(endpoint, &identifyTime)) + { + /* effect identifier changed during identify */ + if (identify->mTargetEffectIdentifier != identify->mCurrentEffectIdentifier) + { + identify->mCurrentEffectIdentifier = identify->mTargetEffectIdentifier; + + /* finish identify process */ + if (EMBER_ZCL_IDENTIFY_EFFECT_IDENTIFIER_FINISH_EFFECT == identify->mCurrentEffectIdentifier && identifyTime > 0) + { + (void) chip::DeviceLayer::SystemLayer().StartTimer(MILLISECOND_TICKS_PER_SECOND, onIdentifyClusterTick, + identify); + return; + } + /* stop identify process */ + else if (EMBER_ZCL_IDENTIFY_EFFECT_IDENTIFIER_STOP_EFFECT == identify->mCurrentEffectIdentifier && identifyTime > 0) + { + Clusters::Identify::Attributes::SetIdentifyTime(endpoint, 0); + identify_deactivate(identify); + } + /* change from e.g. Breathe to Blink during identify */ + else + { + /* cancel identify */ + Clusters::Identify::Attributes::SetIdentifyTime(endpoint, 0); + identify_deactivate(identify); + + /* trigger effect identifier callback */ + if (nullptr != identify->mOnEffectIdentifier) + identify->mOnEffectIdentifier(identify); + } + } + else if (identifyTime > 0) + { + /* we only start if both callbacks are set */ + identify_activate(identify); + + (void) chip::DeviceLayer::SystemLayer().StartTimer(MILLISECOND_TICKS_PER_SECOND, onIdentifyClusterTick, identify); + return; + } + else + { + identify_deactivate(identify); + } + } + + (void) chip::DeviceLayer::SystemLayer().CancelTimer(onIdentifyClusterTick, identify); } }