Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add force send parameter to setOnOffValue and send it on matter start #35287

Closed
wants to merge 2 commits into from
Closed

Add force send parameter to setOnOffValue and send it on matter start #35287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de1c261
Add force send parameter to setOnOffValue and send it on matter start
damian-kurek-wizzdev Aug 29, 2024
570b93c
Restyled by clang-format
restyled-commits Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions src/app/clusters/on-off-server/on-off-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,11 @@ Status OnOffServer::getOnOffValue(chip::EndpointId endpoint, bool * currentOnOff
* @param endpoint Ver.: always
* @param command Ver.: always
* @param initiatedByLevelChange Ver.: always
* @param forceSend Send value of the On/Off even when it is the same as current On/Off value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Send what and where?

At startup, if some hardware state needs to be synced to the state of the On/Off cluster, the right sequence of actions is:

  1. Start up the Matter stack, which initializes the On/Off cluster instance(s).
  2. Read the relevant attributes from those cluster instances (which might involve the StartUpOnOff value, or might involve reading the persisted OnOff state from flash, or some combination thereof).
  3. Sync the hardware state to the desired state as represented by the attributes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think name is a bit incorrect and should be something like force_ember.
What you proposed as syncing of the state will work but it causes duplicate of logic on matter side, I changed it to wait for matter to be started and get this value. I will discard the pr

* This parameter is useful at the start when you try to determine startup state of On/Off relay that does not store state after
* losing power
*/
Status OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::CommandId command, bool initiatedByLevelChange)
Status OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::CommandId command, bool initiatedByLevelChange, bool forceSend)
{
MATTER_TRACE_SCOPE("setOnOffValue", "OnOff");
Status status;
Expand All @@ -355,18 +358,32 @@ 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))
if (forceSend)
{
ChipLogProgress(Zcl, "Endpoint %x On/off already set to new value", endpoint);
return Status::Success;
if (command == Commands::Off::Id)
{
newValue = false;
}
else if (command == Commands::On::Id)
{
newValue = true;
}
// I guess that, I can only get ON/OFF at startup
}
else
{
// 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);

// 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);
}
// 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
// should update the on/off attribute before kicking off level change, if we are
Expand Down Expand Up @@ -499,7 +516,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, true, true);
}

#if defined(MATTER_DM_PLUGIN_SCENES_MANAGEMENT) && CHIP_CONFIG_SCENES_USE_DEFAULT_HANDLERS
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/on-off-server/on-off-server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class OnOffServer
void updateOnOffTimeCommand(chip::EndpointId endpoint);
chip::Protocols::InteractionModel::Status getOnOffValue(chip::EndpointId endpoint, bool * currentOnOffValue);
chip::Protocols::InteractionModel::Status setOnOffValue(chip::EndpointId endpoint, chip::CommandId command,
bool initiatedByLevelChange);
bool initiatedByLevelChange, bool forceSend = false);
chip::Protocols::InteractionModel::Status getOnOffValueForStartUp(chip::EndpointId endpoint, bool & onOffValueForStartUp);

bool HasFeature(chip::EndpointId endpoint, Feature feature);
Expand Down
Loading