I can't create a Window Covering device with a custom end_product_type (CON-363)

[Diegorro98]

To create a Window Covering device with a custom end_product_type I used the following code:

window_covering_device::config_t config;
config.window_covering.end_product_type = (uint8_t)chip::app::Clusters::WindowCovering::EndProductType::kTiltOnlyInteriorBlind;

Now I can't create a window covering device which has a different end_product_type value than 0 because now it is declared as const in this line:

esp-matter/components/esp_matter/esp_matter_cluster.h

Line 325 in 3484600

const uint8_t end_product_type;

Is this expected? Then how do we specify what type of end product is?
In commit 3484600 message, there is a mention that says "sync with spec", there is an unreleased matter spec where end_product_type can't be different than 0? or maybe there is another way to modify it...?

[wqx6]

Can we set the const value when we create the config?

window_covering_device:config_t config = {
    . window_covering  = {
        .end_product_type = ANOTHER_END_PRODUCT_TYPE,
    },
};

[Diegorro98]

@wqx6 I tried something similar, and to ensure that wasn't a problem from window_covering_device::config, I tried this:

cluster::window_covering::config_t config = {
    .end_product_type = (uint8_t)chip::app::Clusters::WindowCovering::Type::kTiltBlindTiltOnly,
};

But didn't work, the build resulted on this error:

error: could not convert '{7}' from '<brace-enclosed initializer list>' to 'esp_matter::cluster::window_covering::config_t' {aka 'esp_matter::cluster::window_covering::config'}
     };
     ^

[shubhamdp]

cluster::window_covering::config_t config = {
    .end_product_type = (uint8_t)chip::app::Clusters::WindowCovering::Type::kTiltBlindTiltOnly,
};

This is not possible because we have a default constructor for config. We should define the default values in the struct itself, so that constructor can be used for different things.

[shubhamdp]

Would it be possible for you to try the attached move_inits_to_struct_itself.txt?

And in application please use

cluster::window_covering::config_t wc_config(static_cast<uint8_t>(chip::app::Clusters::WindowCovering::Type::kTiltBlindTiltOnly));

[Diegorro98]

The patch couldn't be applied over the last commit (80a22dc), but I tried to do a merge with the following result:

namespace window_covering {
typedef struct config {
    uint16_t cluster_revision = 5;
    uint8_t type = 0;
    uint8_t config_status;
    uint8_t operational_status = 0;
    const uint8_t end_product_type = 0;
    uint8_t mode = 0;
    feature::lift::config_t lift;

    config (uint8_t end_product_type = 0)
    {
        this->end_product_type = end_product_type;
    }
} config_t;
...
}

I keept the const keyword because in the current last commit is the root of the problem, and with this patch, remains as the root of the problem, because if a remove the keyword, I can run the code you requested me to try and the code from the first comment:

To create a Window Covering device with a custom end_product_type I used the following code:

window_covering_device::config_t config;
config.window_covering.end_product_type = (uint8_t)chip::app::Clusters::WindowCovering::EndProductType::kTiltOnlyInteriorBlind;

...

[shubhamdp]

@Diegorro98 Sorry for the late turnaround. I got the problem here, you are creating the device type and it adds the window covering cluster and since it is making a cluster config object, you won't be able to configure any const that are defined inside that cluster.

If we remove const then it sort of breaks the specification. In specification, quality of end_product_type is fixed, so it shall not change once configured. If we remove const it breaks the spec.

Will check what can be done.

[Diegorro98]

@shubhamdp I finally found a way to keep end_product_type fixed with const but also be able to set it at the start.
First I used the constructor from the patch you gave me in last comment.

namespace window_covering {
typedef struct config {
    uint16_t cluster_revision;
    uint8_t type;
    uint8_t config_status;
    uint8_t operational_status;
    const uint8_t end_product_type;
    uint8_t mode;
    feature::lift::config_t lift;
    config(uint8_t end_product_type = 0) : cluster_revision(5), type(0), config_status(0), operational_status(0), end_product_type(end_product_type), mode(0) {}
} config_t;

cluster_t *create(endpoint_t *endpoint, config_t *config, uint8_t flags, uint32_t features);
} /* window_covering */

This works, it lets me create an instance of esp_matter::cluster::window_covering::config_t with a custom end product type. But then, the next problem I have is that esp_matter::cluster::window_covering::config_t is inside esp_matter::endpoint::window_covering_device::config_t declared as window_covering. So when you declare a esp_matter::endpoint::window_covering_device::config_t variable, esp_matter::cluster::window_covering::config_t has been constructed.

So what I did is create a constructor also for esp_matter::endpoint::window_covering_device::config_t:

namespace window_covering {
typedef struct config {
    cluster::identify::config_t identify;
    cluster::groups::config_t groups;
    cluster::scenes::config_t scenes;
    cluster::window_covering::config_t window_covering;
    config(uint8_t end_product_type = 0) : window_covering(end_product_type) {}
} config_t;
...
} /* window_covering */

This way I can create a window covering device config with a custom end_product_type:

window_covering_device::config_t wcd_config(static_cast<uint8_t>(chip::app::Clusters::WindowCovering::EndProductType::kTiltOnlyInteriorBlind));

If you are ok with this solution, I can create a PR with this change and also for esp_matter::cluster::pump_configuration_and_control::config_t which contains const members too.

[shubhamdp]

@Diegorro98 Once you set the const through the constructor, remaining non-const members can be set to other values by config.<member> = some-value. This would work, but this behaviour has to be documented as well. You surely can put up a PR.
You can add a section under defining your own data model and mention what you have mentioned in your comment above as a documentation.