Skip to content

Commit

Permalink
Allow target value updates to be skipped (#25860)
Browse files Browse the repository at this point in the history
* Added CHIP_ERROR_IN_PROGRESS error code
* If delegate->HandleStopMotion returns CHIP_ERROR_IN_PROGRESS, the
  target value will not be set.

The attributes "TargetPositionLiftPercent100ths" and
"TargetPositionTiltPercent100ths" will be updated to the attribute
value of their coresponding current values.

But the attribute value and the real state may be differ. It may have a
delay in updating the real state to the attribute value.
It results in the target value mismatched with the real value we want.

emberAfWindowCoveringClusterStopMotionCallback set the target value to
an out-dated value first. And then, when the hardware reports its state
later, the corrected value will be updated again.
The subscribers will receive two attribute "Report", the first one has
an incorrect value, and the second one has the correct value.

This patch fix this issue by allowing applications skip the first update
on the target value.

When the application code wants to update the target value by itself,
it can return CHIP_ERROR_IN_PROGRESS in HandleStopMotion. Then it
updates the target value later.

And then the first incorrect value will no longer be reported to
subscribers.

Signed-off-by: Zhai Zhaoxuan <[email protected]>
  • Loading branch information
Kxuan authored and pull[bot] committed Jan 30, 2024
1 parent e704d88 commit 54d9e0d
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 10 deletions.
1 change: 1 addition & 0 deletions docs/ERROR_CODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ This file was **AUTOMATICALLY** generated by
| 159 | 0x9F | `CHIP_ERROR_PERSISTED_STORAGE_FAILED` |
| 160 | 0xA0 | `CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND` |
| 161 | 0xA1 | `CHIP_ERROR_IM_FABRIC_DELETED` |
| 164 | 0xA4 | `CHIP_ERROR_IN_PROGRESS` |
| 165 | 0xA5 | `CHIP_ERROR_ACCESS_DENIED` |
| 166 | 0xA6 | `CHIP_ERROR_UNKNOWN_RESOURCE_ID` |
| 167 | 0xA7 | `CHIP_ERROR_VERSION_MISMATCH` |
Expand Down
31 changes: 22 additions & 9 deletions src/app/clusters/window-covering-server/window-covering-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -735,26 +735,39 @@ bool emberAfWindowCoveringClusterStopMotionCallback(app::CommandHandler * comman
return true;
}

bool changeTarget = true;

Delegate * delegate = GetDelegate(endpoint);
if (delegate)
{
LogErrorOnFailure(delegate->HandleStopMotion());
CHIP_ERROR err = delegate->HandleStopMotion();
if (err == CHIP_ERROR_IN_PROGRESS)
{
changeTarget = false;
}
else
{
LogErrorOnFailure(err);
}
}
else
{
emberAfWindowCoveringClusterPrint("WindowCovering has no delegate set for endpoint:%u", endpoint);
}

if (HasFeaturePaLift(endpoint))
if (changeTarget)
{
(void) Attributes::CurrentPositionLiftPercent100ths::Get(endpoint, current);
(void) Attributes::TargetPositionLiftPercent100ths::Set(endpoint, current);
}
if (HasFeaturePaLift(endpoint))
{
(void) Attributes::CurrentPositionLiftPercent100ths::Get(endpoint, current);
(void) Attributes::TargetPositionLiftPercent100ths::Set(endpoint, current);
}

if (HasFeaturePaTilt(endpoint))
{
(void) Attributes::CurrentPositionTiltPercent100ths::Get(endpoint, current);
(void) Attributes::TargetPositionTiltPercent100ths::Set(endpoint, current);
if (HasFeaturePaTilt(endpoint))
{
(void) Attributes::CurrentPositionTiltPercent100ths::Get(endpoint, current);
(void) Attributes::TargetPositionTiltPercent100ths::Set(endpoint, current);
}
}

return CHIP_NO_ERROR == commandObj->AddStatus(commandPath, Status::Success);
Expand Down
3 changes: 3 additions & 0 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
case CHIP_ERROR_IM_FABRIC_DELETED.AsInteger():
desc = "The fabric is deleted, and the corresponding IM resources are released";
break;
case CHIP_ERROR_IN_PROGRESS.AsInteger():
desc = "The operation is still in progress";
break;
case CHIP_ERROR_ACCESS_DENIED.AsInteger():
desc = "The CHIP message is not granted access";
break;
Expand Down
9 changes: 8 additions & 1 deletion src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,14 @@ using CHIP_ERROR = ::chip::ChipError;

// AVAILABLE: 0xa2
// AVAILABLE: 0xa3
// AVAILABLE: 0xa4

/**
* @def CHIP_ERROR_IN_PROGRESS
*
* @brief
* The operation is still in progress
*/
#define CHIP_ERROR_IN_PROGRESS CHIP_CORE_ERROR(0xa4)

/**
* @def CHIP_ERROR_ACCESS_DENIED
Expand Down
1 change: 1 addition & 0 deletions src/lib/core/tests/TestCHIPErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ static const CHIP_ERROR kTestElements[] =
CHIP_ERROR_INVALID_FILE_IDENTIFIER,
CHIP_ERROR_BUSY,
CHIP_ERROR_HANDLER_NOT_SET,
CHIP_ERROR_IN_PROGRESS,
};
// clang-format on

Expand Down

0 comments on commit 54d9e0d

Please sign in to comment.