search

espressif / esp-matter

Espressif's SDK for Matter
Apache License 2.0
686 stars 155 forks source link

Unsupported ClusterRevision 2 reported (CON-906) #747

Closed PhLuReh closed 11 months ago

PhLuReh commented 11 months ago

Describe the bug During TC-BINFO-1.1 and TC-DESC-1.1 attribute value 2 is reported for ClusterRevision.

In TC-BINFO-1.1 for Endpoint = 0x0, Cluster = 0x28, Attribute = 0x0000_FFFD the value 2 is reported, but specified and implemented is the value 1. And TC-DESC-1.1 checks for the same Attribute.

./chip-tool basicinformation read cluster-revision 12344321 0: log.txt

In esp_matter_cluster.cpp:99 the attribute gets initialized to value 1 from the config.

I currently found a patch, but I really don't know, where the problem is originally coming from:

esp_err_t clusterrev_cb(esp_matter::attribute::callback_type_t type, uint16_t endpoint_id, uint32_t cluster_id, uint32_t attribute_id, esp_matter_attr_val_t *val, void *priv_data)
{
    // hardcoded revision 1
    ESP_LOGI(TAG, "custerrev_cb: Endpoint=%d, Cluster=%ld, Attribute=%ld", endpoint_id, cluster_id, attribute_id);
    val->type = ESP_MATTER_VAL_TYPE_UINT16;
    val->val.u16 = 1;
    return ESP_OK;
}

and once for initialization:

main() {
...
    endpoint_t *root_ep = endpoint::get(node, 0);
    cluster_t *desc_cl = cluster::get(root_ep, chip::app::Clusters::BasicInformation::Id);
    esp_matter::attribute_t *clusterrev = attribute::get(desc_cl, Globals::Attributes::ClusterRevision::Id);
    attribute::set_override_callback(clusterrev, &clusterrev_cb);
...
}

Environment

VaishaliAvhale commented 11 months ago

@PhLuReh, why you wish to change the value of the ClusterRevision attribute?

PhLuReh commented 11 months ago

I don't want to, the menitoned tests from TH v2.8.1-official require value of Global ClusterRevision to be 1, as it is specified to be so. But I was advised to update to latest main branch (https://github.com/espressif/esp-matter/issues/653#issuecomment-1731290000).

VaishaliAvhale commented 11 months ago

The commit you have used belongs to release/v1.2, and according to the 1.2 specification, the cluster revision value should be 2. It is fixed for the core cluster of the specific release.

TH v2.8.1-official requires the value of global ClusterRevision to be 1

The answer to this is TH v2.8.1 is the latest 1.2 TH version and ClusterRevision attribute value requirement specified in the PICS file can you please share that PICS here and send me complete DUT and Chip-tool side logs.

PhLuReh commented 11 months ago

Ok thanks for making this obvious point clear. I found, that the required commit also was pushed to the branch release/v1.1.

I tried to solve this. My build environment is built as Docker container on-top of espressif/esp-matter:latest_idf_v5.1.1, with then having bootstraping, create a user, setup sshd and so on. This worked pretty good so far. But now, I tried to switch the branch of /opt/espressif/esp-matter to release/v1.1, which I succeeded in:

 git log
commit dd4f34ec98ee1d5467bcfa8f6c885d53de58e50e (HEAD -> release/v1.1, origin/release/v1.1)
Merge: 0b2efa8 df3f50d
Author: Shu Chen <chenshu@espressif.com>
Date:   Mon Nov 13 16:46:01 2023 +0800

    Merge branch 'update_submodule/v1.1.0.2' into 'release/v1.1'

    Update chip submodule to v1.1.0.2 on  esp-matter release/v1.1.

    See merge request app-frameworks/esp-matter!542

I then updated the repo by running ./install.sh in /opt/espressif/esp-matter, which fails with ERROR: Cannot install mypy-protobuf<3.6.0 and >=3.5.0 because these package versions have conflicting dependencies.. log: matter_install.txt

The connectedhomeip repo also got updated:

/opt/espressif/esp-matter/connectedhomeip/connectedhomeip# git log
commit c49852a85368fe39e947ea4885cdead1d9352865 (HEAD, tag: v1.1.0.2)
Author: Jerry-ESP <107675966+Jerry-ESP@users.noreply.github.com>
Date:   Wed Oct 18 01:55:52 2023 +0800

    add a config to enable event list attribute (#29745)

Is there a way to change the branch of your docker image? Or create a new tag for Matter v1.1?

PhLuReh commented 11 months ago

I'm not able to get the repos managed in a proper way. In my desperation, I searched for alternative solutions and I found that the workflow-file creating the docker image already is prepared for branch-releases. But this action seems not to be triggered. Could you please enable this feature, it would save plenty of our time...

VaishaliAvhale commented 11 months ago

It seems like you are not following the correct steps after checking out the esp-matter branch. Please check the following links and try the steps outlined below:

  1. Verify the required components for release/v1.1 by referring to this link: https://github.com/espressif/esp-matter/tree/release/v1.1.
  2. Set up your environment by following the steps provided in this guide: https://docs.espressif.com/projects/esp-matter/en/latest/esp32c6/developing.html.
PhLuReh commented 11 months ago

As I already said, I'm running on your esp-matter docker container, so I would prefer some production ready solution for this issue (and so I hope I'm not alone with this wish). And as I stated, the effort should be small, as the github-workflow already is implemented, but likely does not get triggered.

However, I'm now running on release/v1.1 - somehow I did it get managed. But had to manually interfere while bootstrapping. I finally had to apply a path to connectedhomeip/src/platform/ESP32/route_hook/ESP32RouteHook.c as execution got aborted due to an unlocked LWIP. I really do not understand why, because if have LWIP_TCPIP_CORE_LOCKING(=y). Here's my patch, if somebody stumbles upon:

diff --git a/src/platform/ESP32/route_hook/ESP32RouteHook.c b/src/platform/ESP32/route_hook/ESP32RouteHook.c
index df19adf1de..a8752789e1 100644
--- a/src/platform/ESP32/route_hook/ESP32RouteHook.c
+++ b/src/platform/ESP32/route_hook/ESP32RouteHook.c
@@ -21,6 +21,7 @@
 #include "lwip/prot/ip6.h"
 #include "lwip/prot/nd6.h"
 #include "lwip/raw.h"
+#include "lwip/tcpip.h"

 #define HOPLIM_MAX 255
 #define PIO_FLAG_ON_LINK (1 << 7)
@@ -160,6 +161,7 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif)
     {
         return ESP_ERR_INVALID_SIZE;
     }
+LOCK_TCPIP_CORE();
     lwip_netif = netif_get_by_index((uint8_t) netif_idx);
     ESP_RETURN_ON_FALSE(lwip_netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface");

@@ -168,13 +170,16 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif)
         if (iter->netif == lwip_netif)
         {
             ESP_LOGI(TAG, "Hook already installed on netif, skip...");
-            return ESP_OK;
+UNLOCK_TCPIP_CORE();
+           return ESP_OK;
         }
     }
+UNLOCK_TCPIP_CORE();

     hook = (esp_route_hook_t *) malloc(sizeof(esp_route_hook_t));
     ESP_RETURN_ON_FALSE(hook != NULL, ESP_ERR_NO_MEM, TAG, "Cannot allocate hook");

+LOCK_TCPIP_CORE();
     ESP_GOTO_ON_FALSE(mld6_joingroup_netif(lwip_netif, ip_2_ip6(&router_group)) == ESP_OK, ESP_FAIL, exit, TAG,
                       "Failed to join multicast group");
     hook->netif = lwip_netif;
@@ -193,5 +198,6 @@ exit:
     {
         free(hook);
     }
+UNLOCK_TCPIP_CORE();
     return ret;
 }