project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.52k stars 2.02k forks source link

[BUG] Differentiation of commands in window-covering-server not possible without PositionAware feature set #35813

Open luko6202 opened 1 month ago

luko6202 commented 1 month ago

Reproduction steps

I am currently implementing a Matter gateway for controlling receivers of a proprietary standard. I encountered a problem when mapping a motor switch:

While Amazon does not currently support a WindowCoveringDevice without the PositionAware feature, GoogleHome and Homeassistant do. When using the window covering server from the Git repository, it currently makes no sense to use the UpOrOpen or DownOrClose commands.

bool emberAfWindowCoveringClusterUpOrOpenCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
                                                  const Commands::UpOrOpen::DecodableType & commandData)
{
    EndpointId endpoint = commandPath.mEndpointId;

    ChipLogProgress(Zcl, "UpOrOpen command received");

    Status status = GetMotionLockStatus(endpoint);
    if (Status::Success != status)
    {
        ChipLogProgress(Zcl, "Err device locked");
        commandObj->AddStatus(commandPath, status);
        return true;
    }

    if (HasFeature(endpoint, Feature::kPositionAwareLift))
    {
        Attributes::TargetPositionLiftPercent100ths::Set(endpoint, WC_PERCENT100THS_MIN_OPEN);
    }
    if (HasFeature(endpoint, Feature::kPositionAwareTilt))
    {
        Attributes::TargetPositionTiltPercent100ths::Set(endpoint, WC_PERCENT100THS_MIN_OPEN);
    }

    Delegate * delegate = GetDelegate(endpoint);
    if (delegate)
    {
        if (HasFeature(endpoint, Feature::kPositionAwareLift))
        {
            LogErrorOnFailure(delegate->HandleMovement(WindowCoveringType::Lift));
        }

        if (HasFeature(endpoint, Feature::kPositionAwareTilt))
        {
            LogErrorOnFailure(delegate->HandleMovement(WindowCoveringType::Tilt));
        }
    }
    else
    {
        ChipLogProgress(Zcl, "WindowCovering has no delegate set for endpoint:%u", endpoint);
    }

    commandObj->AddStatus(commandPath, Status::Success);

    return true;
}

When processing the commands (here using the UpOrOpen example), an attribute is only updated if the PositionAware feature is set. The HandleMovement() function, which is overwritten in my own software, is then called. Without set features, the commands for controlling up and down do not differ. Therefore, I would be happy about an adjustment in the library.

There are certainly workarounds for this, currently I modified the delegate call in the window-covering-server.

Bug prevalence

Whenever the UpOrOpen or the DownOrClose command is used

GitHub hash of the SDK that was being used

ea679d2d*

Platform

other

Platform Version(s)

No response

Anything else?

No response

jmeg-sfy commented 1 month ago

Indeed the conditions are wrong and should not be kPositionAwareTilt but kTilt and kLift for the basic Open/Close/Stop commands.

The PositionAware should affect the Current/Target attributes In this piece of code i would suggest also to use the helpers HasFeaturePaLift(chip::EndpointId endpoint) instead of HasFeature(endpoint, Feature::kPositionAwareLift) because the conformance stated is Lift + PA_Lift

luko6202 commented 1 month ago

Hi, thanks for the answer. Please note that even when setting the kTilt or kLift feature, there is no directional parameter in the delegate callback or an attribute set that could be queried in the callback.