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.

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 ReplacementProductList to Resource Monitoring Cluster #28095

Merged
merged 9 commits into from
Jul 20, 2023

Conversation

cliffamzn
Copy link
Contributor

@cliffamzn cliffamzn commented Jul 20, 2023

This adds the implementation to fetch the replacement product list from the aliased resource-monitoring-cluster. It adds support for Hepa and Activated Carbon filter examples.

Closes out issues #27802, #27801, #27577

Testing

I tested this using the all-clusters-app and the resource-monitoring-example-app

>>>> hepafiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xd31d0b60,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x71,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xhepaxx" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xhe" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xhepaxxx" (14 chars),
        },
],

>>>> activatedcarbonfiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xfd145260,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x72,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xcarbon" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xca" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xacarbon" (14 chars),
        },
],

closes #27577

This adds the implementation to fetch the replacement product list from
the aliased resource-monitoring-cluster. It adds support for Hepa and
Activated Carbon filter examples.

Closes out issues project-chip#27802, project-chip#27801, project-chip#27577

### Testing

I tested this using the resource-monitoring-example-app

```
>>>> hepafiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xd31d0b60,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x71,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xhepaxx" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xhe" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xhepaxxx" (14 chars),
        },
],

>>>> activatedcarbonfiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xfd145260,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x72,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xcarbon" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xca" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xacarbon" (14 chars),
        },
],
```
@cliffamzn cliffamzn force-pushed the replacable-monitoring-impl branch from ba93661 to d9e6a9d Compare July 20, 2023 16:22
@cliffamzn cliffamzn merged commit d2d4701 into project-chip:master Jul 20, 2023
@cliffamzn cliffamzn deleted the replacable-monitoring-impl branch July 20, 2023 19:02
CHIP_ERROR err;
if (Instance::HasFeature(ResourceMonitoring::Feature::kReplacementProductList))
{
ReplacementProductListManager * productListManagerInstance = Instance::GetReplacementProductListManagerInstance();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need the Instance:: prefix? This is not a static method.... and we are inside Instance anyway.


err = aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
Attributes::ReplacementProductStruct::Type replacementProductStruct;
while (productListManagerInstance->Next(replacementProductStruct) == CHIP_NO_ERROR)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this dropping errors on the floor? If the callee returns some "I am in a bad state" error, should we not return that instead of silently truncating the list?

void Reset() { mIndex = 0; }

// Returns total size of Replacement Products List.
virtual uint8_t Size() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be unused.

@bzbarsky-apple
Copy link
Contributor

And #28148 in addition to those issues.

erwinpan1 pushed a commit to erwinpan1/connectedhomeip that referenced this pull request Jul 21, 2023
…ip#28095)

* Add ReplacementProductList to Resource Monitoring Cluster

This adds the implementation to fetch the replacement product list from
the aliased resource-monitoring-cluster. It adds support for Hepa and
Activated Carbon filter examples.

Closes out issues project-chip#27802, project-chip#27801, project-chip#27577

### Testing

I tested this using the all-clusters-app and the resource-monitoring-example-app

```
>>>> hepafiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xd31d0b60,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x71,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xhepaxx" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xhe" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xhepaxxx" (14 chars),
        },
],

>>>> activatedcarbonfiltermonitoring read replacement-product-list 0x1 0x1

DataVersion = 0xfd145260,
AttributePathIB =
{
        Endpoint = 0x1,
        Cluster = 0x72,
        Attribute = 0x0000_0005,
}

Data = [

        {
                0x0 = 0,
                0x1 = "upc12xcarbon" (13 chars),
        },
        {
                0x0 = 1,
                0x1 = "gtin8xca" (9 chars),
        },
        {
                0x0 = 2,
                0x1 = "ean13xacarbon" (14 chars),
        },
],
```

* Addressing @tobiasgraf's comments

* Addressing @tcarmelveilleux's comments and restyled

* Simplifying example app and adding support for resource monitor list in
the all clusters app

* Addressing @tcarmelveilleux's suggestions

* Restyled by whitespace

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <[email protected]>
cliffamzn added a commit to cliffamzn/connectedhomeip that referenced this pull request Jul 21, 2023
cliffamzn added a commit to cliffamzn/connectedhomeip that referenced this pull request Jul 21, 2023
Addressing feedback from PR project-chip#28095 and also addressing project-chip#28148

Tested to ensure functionality still works as expected using the all-clusters-app as well as the resource-monitoring-app
cliffamzn added a commit that referenced this pull request Jul 27, 2023
* Modifying ReplacementProductListManager to be generic

Addressing feedback from PR #28095 and also addressing #28148

This change simplifies the implementation of the ReplacementProductList and also introduces a DynamicReplacementProductList example which better represents devices fetching this product list from flash.

It includes some simplifications to the structure of the code and a few additional unit tests to exercise the behavior.

Tested to ensure functionality still works as expected using the all-clusters-app as well as the resource-monitoring-app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants