diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp index c176d6f6b17bbb..03b3d59d58ea98 100644 --- a/src/app/clusters/level-control/level-control.cpp +++ b/src/app/clusters/level-control/level-control.cpp @@ -102,6 +102,7 @@ struct EmberAfLevelControlState CallbackScheduleState callbackSchedule; QuieterReportingAttribute quietCurrentLevel{ DataModel::NullNullable }; QuieterReportingAttribute quietRemainingTime{ DataModel::MakeNullable(0) }; + bool commandIsOnOff; // tracks transitions triggered by the on/off cluster }; static EmberAfLevelControlState stateTable[kLevelControlStateTableSize]; @@ -110,7 +111,7 @@ static EmberAfLevelControlState * getState(EndpointId endpoint); static Status moveToLevelHandler(EndpointId endpoint, CommandId commandId, uint8_t level, DataModel::Nullable transitionTimeDs, chip::Optional> optionsMask, - chip::Optional> optionsOverride, uint16_t storedLevel); + chip::Optional> optionsOverride, uint16_t storedLevel, bool commandIsOnOff); static void moveHandler(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, MoveModeEnum moveMode, DataModel::Nullable rate, chip::Optional> optionsMask, chip::Optional> optionsOverride); @@ -499,6 +500,11 @@ void emberAfLevelControlClusterServerTickCallback(EndpointId endpoint) } } + // "clear" the stored command. + state->commandId = Commands::Stop::Id; + // since the transition is complete, clear the tracking of commands from the OnOff cluster. + state->commandIsOnOff = false; + state->callbackSchedule.runTime = System::Clock::Milliseconds32(0); writeRemainingTime(endpoint, 0); } @@ -695,7 +701,7 @@ Status MoveToLevel(EndpointId endpointId, const Commands::MoveToLevel::Decodable return moveToLevelHandler(endpointId, Commands::MoveToLevel::Id, level, transitionTime, Optional>(optionsMask), Optional>(optionsOverride), - INVALID_STORED_LEVEL); // Don't revert to the stored level + INVALID_STORED_LEVEL, false); // Don't revert to the stored level } #ifdef MATTER_DM_PLUGIN_SCENES_MANAGEMENT @@ -734,7 +740,7 @@ bool emberAfLevelControlClusterMoveToLevelWithOnOffCallback(CommandHandler * com Status status = moveToLevelHandler(commandPath.mEndpointId, Commands::MoveToLevelWithOnOff::Id, level, transitionTime, Optional>(optionsMask), Optional>(optionsOverride), - INVALID_STORED_LEVEL); // Don't revert to the stored level + INVALID_STORED_LEVEL, false); // Don't revert to the stored level commandObj->AddStatus(commandPath, status); @@ -870,7 +876,7 @@ bool emberAfLevelControlClusterStopWithOnOffCallback(CommandHandler * commandObj static Status moveToLevelHandler(EndpointId endpoint, CommandId commandId, uint8_t level, DataModel::Nullable transitionTimeDs, chip::Optional> optionsMask, - chip::Optional> optionsOverride, uint16_t storedLevel) + chip::Optional> optionsOverride, uint16_t storedLevel, bool commandIsOnOff) { EmberAfLevelControlState * state = getState(endpoint); DataModel::Nullable currentLevel; @@ -907,7 +913,8 @@ static Status moveToLevelHandler(EndpointId endpoint, CommandId commandId, uint8 return Status::Failure; } - state->commandId = commandId; + state->commandId = commandId; + state->commandIsOnOff = commandIsOnOff; // Move To Level commands cause the device to move from its current level to // the specified level at the specified rate. @@ -1097,6 +1104,9 @@ static void moveHandler(CommandHandler * commandObj, const ConcreteCommandPath & // Cancel any currently active command before fiddling with the state. cancelEndpointTimerCallback(endpoint); + // the moveHandler is only called for commands that are triggered by this cluster and therefore + // the current command is _not_ coming from the OnOff cluster. + state->commandIsOnOff = false; state->eventDurationMs = eventDuration; status = Attributes::CurrentLevel::Get(endpoint, currentLevel); @@ -1337,6 +1347,7 @@ static void stopHandler(CommandHandler * commandObj, const ConcreteCommandPath & cancelEndpointTimerCallback(endpoint); SetCurrentLevelQuietReport(endpoint, state, state->quietCurrentLevel.value(), true /*isEndOfTransition*/); writeRemainingTime(endpoint, 0); + state->commandIsOnOff = false; send_default_response: commandObj->AddStatus(commandPath, status); @@ -1425,22 +1436,55 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new transitionTime.SetNull(); #endif // IGNORE_LEVEL_CONTROL_CLUSTER_ON_OFF_TRANSITION_TIME + // Check if we're already performing an on/off transition. + if (state->commandIsOnOff) + { + if (!newValue && (state->moveToLevel == minimumLevelAllowedForTheDevice)) + { + // new state should be 'OFF' and we're already performing the 'OFF' transition + return; + } + if (newValue && (state->moveToLevel != minimumLevelAllowedForTheDevice)) + { + // new state should be 'ON' and we're already performing the 'ON' transition + return; + } + if (!newValue && (state->moveToLevel != minimumLevelAllowedForTheDevice)) + { + // new state should be 'OFF' but we're performing an 'ON' transition. + // prevent that the current dimming level (used by the transition effect) + // is being stored as "temporary new level" + temporaryCurrentLevelCache.SetNonNull(state->moveToLevel); + } + if ((newValue) && (state->moveToLevel == minimumLevelAllowedForTheDevice)) + { + // new state should be 'ON' but we're performing an 'OFF' transition. + // in this case - without intercepting this sequence - the ON transition would + // restart at the minimum level. + resolvedLevel.SetNonNull(state->storedLevel); + } + } + if (newValue) { // If newValue is OnOff::Commands::On::Id... // "Set CurrentLevel to minimum level allowed for the device." - status = SetCurrentLevelQuietReport(endpoint, state, minimumLevelAllowedForTheDevice, false /*isEndOfTransition*/); - if (status != Status::Success) + // ... except if an On/Off transition is already in progress, then don't manipulate the + // current level but instead just adjust the "moveTo" direction. + if (!state->commandIsOnOff) { - ChipLogProgress(Zcl, "ERR: reading current level %x", to_underlying(status)); - return; + status = SetCurrentLevelQuietReport(endpoint, state, minimumLevelAllowedForTheDevice, false /*isEndOfTransition*/); + if (status != Status::Success) + { + ChipLogProgress(Zcl, "ERR: reading current level %x", to_underlying(status)); + return; + } } // "Move CurrentLevel to OnLevel, or to the stored level if OnLevel is not // defined, over the time period OnOffTransitionTime." moveToLevelHandler(endpoint, Commands::MoveToLevel::Id, resolvedLevel.Value(), transitionTime, chip::NullOptional, - chip::NullOptional, - INVALID_STORED_LEVEL); // Don't revert to stored level + chip::NullOptional, INVALID_STORED_LEVEL, true); // Don't revert to stored level } else { @@ -1451,17 +1495,32 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new { // If OnLevel is defined, don't revert to stored level. moveToLevelHandler(endpoint, Commands::MoveToLevelWithOnOff::Id, minimumLevelAllowedForTheDevice, transitionTime, - chip::NullOptional, chip::NullOptional, INVALID_STORED_LEVEL); + chip::NullOptional, chip::NullOptional, INVALID_STORED_LEVEL, true); } else { // If OnLevel is not defined, set the CurrentLevel to the stored level. moveToLevelHandler(endpoint, Commands::MoveToLevelWithOnOff::Id, minimumLevelAllowedForTheDevice, transitionTime, - chip::NullOptional, chip::NullOptional, temporaryCurrentLevelCache.Value()); + chip::NullOptional, chip::NullOptional, temporaryCurrentLevelCache.Value(), true); } } } +void emberAfOnOffClusterLevelControlIsEffectActiveCallback(EndpointId endpoint, bool * const isActive, bool * const targetState) +{ + EmberAfLevelControlState * state = getState(endpoint); + if (state == nullptr) + { + ChipLogProgress(Zcl, "ERR: Level control cluster not available on ep%d", endpoint); + return; + } + *isActive = state->commandIsOnOff; + if (*isActive) + { + *targetState = state->moveToLevel != state->minLevel; + } +} + void emberAfLevelControlClusterServerInitCallback(EndpointId endpoint) { EmberAfLevelControlState * state = getState(endpoint); diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp index d18aa09383a0d5..e6c0042c63a800 100644 --- a/src/app/clusters/on-off-server/on-off-server.cpp +++ b/src/app/clusters/on-off-server/on-off-server.cpp @@ -373,17 +373,67 @@ Status OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::CommandId com return status; } - // if the value is already what we want to set it to then do nothing - if ((!currentValue && command == Commands::Off::Id) || (currentValue && command == Commands::On::Id)) +#ifdef MATTER_DM_PLUGIN_LEVEL_CONTROL + // the level control cluster supports on/off transitions or effects, e.g., controlled via + // the OnOffTransitionTime attribute: in a transition to "off", the level control cluster + // changes the CurrentLevel attribute using a timer until it reaches the minimum supported value + // and only then changes the OnOff value in the OnOff cluster. therefore, comparing the current + // OnOff state with the required state is not sufficient for OnOff cluster values that support + // level control: + // - the current state might be "On" + // - even though the level control cluster changes the state towards "Off" + // - and thus any other "On" value should prevent the cluster from turning off. + // + // this restriction does not exist for the "Off" command since level movements by definition + // are only available while the OnOff state is "On". + if (!LevelControlWithOnOffFeaturePresent(endpoint) || (command == Commands::Off::Id)) +#endif { - ChipLogProgress(Zcl, "Endpoint %x On/off already set to new value", endpoint); - return Status::Success; + // if the value is already what we want to set it to then do nothing + if ((!currentValue && command == Commands::Off::Id) || (currentValue && command == Commands::On::Id)) + { + ChipLogProgress(Zcl, "Endpoint %x On/off already set to new value", endpoint); + return Status::Success; + } } - // we either got a toggle, or an on when off, or an off when on, - // so we need to swap the value - newValue = !currentValue; - ChipLogProgress(Zcl, "Toggle ep%x on/off from state %x to %x", endpoint, currentValue, newValue); + switch (command) + { + case Commands::Off::Id: + newValue = false; + break; + + case Commands::On::Id: + newValue = true; + break; + + case Commands::Toggle::Id: + newValue = !currentValue; +#ifdef MATTER_DM_PLUGIN_LEVEL_CONTROL + if (LevelControlWithOnOffFeaturePresent(endpoint)) + { + bool effectActive = false; + bool targetValueOn = false; + emberAfOnOffClusterLevelControlIsEffectActiveCallback(endpoint, &effectActive, &targetValueOn); + if (effectActive) + { + newValue = !targetValueOn; + } + } +#endif + break; + + case Commands::OffWithEffect::Id: + case Commands::OnWithRecallGlobalScene::Id: + case Commands::OnWithTimedOff::Id: + default: + // this function is not supposed to be called with the given commands. + ChipLogProgress(Zcl, "ERR: unknown/unsupported command %x", command); + return Status::Failure; + break; + } + + ChipLogProgress(Zcl, "Change ep%x on/off from state %x to %x", endpoint, currentValue, newValue); // the sequence of updating on/off attribute and kick off level change effect should // be depend on whether we are turning on or off. If we are turning on the light, we @@ -415,11 +465,14 @@ Status OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::CommandId com } // write the new on/off value - status = Attributes::OnOff::Set(endpoint, newValue); - if (status != Status::Success) + if (newValue != currentValue) { - ChipLogProgress(Zcl, "ERR: writing on/off %x", to_underlying(status)); - return status; + status = Attributes::OnOff::Set(endpoint, newValue); + if (status != Status::Success) + { + ChipLogProgress(Zcl, "ERR: writing on/off %x", to_underlying(status)); + return status; + } } #ifdef MATTER_DM_PLUGIN_LEVEL_CONTROL @@ -461,11 +514,14 @@ Status OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::CommandId com #endif { // write the new on/off value - status = Attributes::OnOff::Set(endpoint, newValue); - if (status != Status::Success) + if (newValue != currentValue) { - ChipLogProgress(Zcl, "ERR: writing on/off %x", to_underlying(status)); - return status; + status = Attributes::OnOff::Set(endpoint, newValue); + if (status != Status::Success) + { + ChipLogProgress(Zcl, "ERR: writing on/off %x", to_underlying(status)); + return status; + } } if (SupportsLightingApplications(endpoint)) @@ -517,7 +573,7 @@ void OnOffServer::initOnOffServer(chip::EndpointId endpoint) Status status = getOnOffValueForStartUp(endpoint, onOffValueForStartUp); if (status == Status::Success) { - status = setOnOffValue(endpoint, onOffValueForStartUp, true); + status = setOnOffValue(endpoint, onOffValueForStartUp ? Commands::On::Id : Commands::Off::Id, true); } #if defined(MATTER_DM_PLUGIN_SCENES_MANAGEMENT) && CHIP_CONFIG_SCENES_USE_DEFAULT_HANDLERS @@ -972,8 +1028,8 @@ static inline void unreg(OnOffEffect * inst) OnOffEffect::OnOffEffect(chip::EndpointId endpoint, OffWithEffectTriggerCommand offWithEffectTrigger, EffectIdentifierEnum effectIdentifier, uint8_t effectVariant) : - mEndpoint(endpoint), - mOffWithEffectTrigger(offWithEffectTrigger), mEffectIdentifier(effectIdentifier), mEffectVariant(effectVariant) + mEndpoint(endpoint), mOffWithEffectTrigger(offWithEffectTrigger), mEffectIdentifier(effectIdentifier), + mEffectVariant(effectVariant) { reg(this); }; diff --git a/src/app/clusters/on-off-server/on-off-server.h b/src/app/clusters/on-off-server/on-off-server.h index 9bf8a5d4b03208..68f2cc277dd28c 100644 --- a/src/app/clusters/on-off-server/on-off-server.h +++ b/src/app/clusters/on-off-server/on-off-server.h @@ -158,6 +158,9 @@ struct OnOffEffect */ void emberAfOnOffClusterLevelControlEffectCallback(chip::EndpointId endpoint, bool newValue); +void emberAfOnOffClusterLevelControlIsEffectActiveCallback(chip::EndpointId endpoint, bool * const isActive, + bool * const targetState); + /********************************************************** * Callbacks *********************************************************/