Bridged Node Name still not persistent across reboot (CON-1259)

[ahmed8812]

Describe the bug Bridged Node Node Label is not persistent across reboot

Environment

• ESP-Matter Commit Id: release/v1.2
• ESP-IDF Commit Id: a322e6bdad4b6675d4597fb2722eea2851ba88cb
• SoC (eg: ESP32 or ESP32-C3): ESP32S3
• Device Logs (Please attach the log file):
• Host Machine OS: Ubuntu
• Commissioner app and versions if present:
• Commissioner's logs if present:

Hi,

This is a snippet from my code using the API to register node label.

if(app_bridge_get_device_by_blemesh_addr(blemesh_addr)) 
{
    uint16_t endpoint_id = app_bridge_get_matter_endpointid_by_blemesh_addr(blemesh_addr);
    endpoint_t *endpoint = endpoint::get(node, endpoint_id);
    cluster_t *cluster = cluster::get(endpoint, BridgedDeviceBasicInformation::Id);
    char name[16];
    snprintf(name, sizeof(name), "NODE_%04X", blemesh_addr);
    cluster::bridged_device_basic_information::attribute::create_node_label(cluster, name, strlen(name));
}

And create_node_label should have been saved in non-volatile. But upon reboot, reading back this attribute it is null and the report log as follows:

if(app_bridge_get_device_by_blemesh_addr(blemesh_addr)) 
{
    uint16_t endpoint_id = app_bridge_get_matter_endpointid_by_blemesh_addr(blemesh_addr);
    endpoint_t *endpoint = endpoint::get(node, endpoint_id);
    cluster_t *cluster = cluster::get(endpoint, BridgedDeviceBasicInformation::Id);
    attribute_t *attribute = attribute::get(cluster, BridgedDeviceBasicInformation::Attributes::NodeLabel::Id);
    esp_matter_attr_val_t val = esp_matter_invalid(NULL);
    attribute::get_val(attribute, &val);
    char *name = (char *)val.val.a.b;
    uint16_t s = val.val.a.s;
    char fi_name[16];
    memcpy(fi_name, name, s);
    fi_name[s] = '\0'; 
    ESP_LOGW(BRIDGE_TAG, "Load Label meshId:%04X Label:%s", blemesh_addr, fi_name); 
}

I (11850) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000039's Attribute 0x0000FFFC is 0 ********** I (11850) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000039's Attribute 0x0000FFFD is 2 ********** I (11870) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000039's Attribute 0x00000011 is 1 ********** I (11880) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000003's Attribute 0x0000FFFC is 0 ********** I (11900) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000003's Attribute 0x0000FFFD is 4 ********** I (11900) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000003's Attribute 0x00000000 is 0 ********** I (11920) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000003's Attribute 0x00000001 is 0 ********** I (11930) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000004's Attribute 0x0000FFFC is 1 ********** I (11940) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000004's Attribute 0x0000FFFD is 4 ********** I (11960) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000004's Attribute 0x00000000 is 128 ********** I (11970) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000005's Attribute 0x00000006 is 0 ********** I (11970) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000005's Attribute 0x0000FFFC is 0 ********** I (11980) esp_matter_attribute: ********** R : Endpoint 0x0002's Cluster 0x00000005's Attribute 0x00000006 is 0 **********

So there is no Cluster 0x0039 Attribute 0x0005 is being reported

Thanks

[ronny-antoon]

try to make a Full Clean before flashing

1. erase flash "on device"
2. remove/delete ("sdkconfig","dependencies.lock","build","managed_components")
3. new build 4)flash and monitor

[ahmed8812]

Hi @ronny-antoon

Did you encounter the same problem?

If you did the steps as you mentioned, is the problem solved? If I add a device later dynamically and put the node label, does it still work?

Regards

[jonsmirl]

The attribute is marked NONVOLATILE so it should be persisted. The esp-matter code looks right to me... you can use this tool to dump the NVS flash and see what it is doing... https://github.com/AFontaine79/Espressif-NVS-Analyzer

attribute_t *create_node_label(cluster_t *cluster, char *value, uint16_t length)
{
    if (length > k_max_node_label_length) {
        ESP_LOGE(TAG, "Could not create attribute, string length out of bound");
        return NULL;
    }
    return esp_matter::attribute::create(cluster, BridgedDeviceBasicInformation::Attributes::NodeLabel::Id,
                                         ATTRIBUTE_FLAG_WRITABLE | ATTRIBUTE_FLAG_NONVOLATILE,
                                         esp_matter_char_str(value, length), k_max_node_label_length);
}

and then...

    bool attribute_updated = false;
    if (attribute->flags & ATTRIBUTE_FLAG_NONVOLATILE) {
        // Lets directly read into attribute->val so that we don't have to set the attribute value again.
        esp_err_t err = get_val_from_nvs(attribute->endpoint_id, attribute->cluster_id, attribute->attribute_id,
                                            attribute->val);
        if (err == ESP_OK) {
            attribute_updated = true;
        }
    }
    if (!attribute_updated) {
        set_val((attribute_t *)attribute, &val);
    }

[ronny-antoon]

Hi @ronny-antoon

Did you encounter the same problem?

If you did the steps as you mentioned, is the problem solved? If I add a device later dynamically and put the node label, does it still work?

Regards

The problem is when you created endpoint without the Non-Volatile attribute, Then you just add the Non-Volatile attribute and flashed it without erase the NVS, then it will not work as expected.

[ahmed8812]

Hi @ronny-antoon

So your steps are first create the bridge, then create the name which is non volatile. But if check the codes, when created the bridge its already save in non volatile memory.

Can you show a snippet of codes how you created bridge and add names to it? , really appreciate that

Regards

[ronny-antoon]

Hi @ronny-antoon

So your steps are first create the bridge, then create the name which is non volatile.

But if check the codes, when created the bridge its already save in non volatile memory.

Can you show a snippet of codes how you created bridge and add names to it? , really appreciate that

Regards

Look at my repository, it still in progress. If you like it give it a star an watch.

https://github.com/ronny-antoon/DeviceModule/blob/be208a475442f2cadb50600a267cc8382ab763ae/components/DeviceModule/include/BaseDeviceInterface.hpp#L44C1-L44C131

[pimpalemahesh]

@ahmed8812 When the device is rebooted, instead of creating a new bridge device, it calls resume_device, which internally creates the bridged_device_basic_information cluster again. This is why the create_node_label, which is an optional attribute, is not persisted.

[ahmed8812]

Hi @ronny-antoon thanks for your help. I get the idea how to persist the node label.

Hi @pimpalemahesh I used the app_bridge_create_bridged_device API, then I created the node label. However this has limitation, for example after I created the bridged node using this API, then if I created node label afterwards, what happen is that the node label will not appears in Google Home or other IoT providers because the esp_matter::endpoint::enable() has already been called when using the app_bridge_create_bridged_device API.

So I did some modifications inside the API to create node label before the esp_matter::endpoint::enable() so the node label appears after registering the bridged device.

For rebooting, the app_bridge_initialize calls the resume_device which does not load any of the bridged_device_basic_information.

For now, I modify the resume_device to load the node label to ensure once I reboot, the node label will also loads. If I create node label afterwards it will not work because esp_matter::endpoint::enable() is already called during resume_device.

These are my temporary fixes for now, but I think the node label has to taken into consideration for user experience while using Google or Apple or any other Iot providers.

Please suggest alternatives so when creates the bridged node dynamically the node label can appears, and also to load the node label when reboot

Thanks

[ronny-antoon]

The good thing that's solved, The app bridge create not really good in my opinion, for that i made my functions.

[pimpalemahesh]

@ahmed8812 Can you share patch of your changes so that I can get better idea of what you are doing.

[ahmed8812]

Hi @pimpalemahesh

Its my tempo fix and its ugly. But it does the job for now while I am busy with some other things

in esp_matter_bridge.cpp, in resume_device()

if (!(dev->endpoint)) {
    ESP_LOGE(TAG, "Could not resume esp_matter endpoint for bridged device");
    esp_matter_mem_free(dev);
    erase_bridged_device_info(device_endpoint_id);
    return NULL;
}

//------------- Added-----------//
cluster_t *cluster = cluster::get(dev->endpoint, BridgedDeviceBasicInformation::Id);
char name[16];
strcpy(name, "DUMMY");//dummy, when `create_node_label` will check if this already exists, then will load node-label
cluster::bridged_device_basic_information::attribute::create_node_label(cluster, name, strlen(name));
//------------- Added -----------//

if (set_device_type(dev, persistent_info.device_type_id) != ESP_OK) {
    ESP_LOGE(TAG, "Failed to add the device type for the bridged device");
    remove_device(dev);
    return NULL;
}

in app_bridged_device.cpp, in app_bridge_create_bridged_device()

if (ESP_OK != app_bridge_store_bridged_device_info(new_dev)) {
    ESP_LOGW(TAG, "Failed to store the bridged device information");
}

//------------ Added -------------//
//1st to create the node label
uint16_t endpoint_id = app_bridge_get_matter_endpointid_by_blemesh_addr(bridged_device_address.blemesh_addr);
endpoint_t *endpoint = endpoint::get(node, endpoint_id);
cluster_t *cluster = cluster::get(endpoint, BridgedDeviceBasicInformation::Id); 
char name[16];
snprintf(name, sizeof(name), "ND_%04X", bridged_device_address.blemesh_addr);
cluster::bridged_device_basic_information::attribute::create_node_label(cluster, name, strlen(name));

//2nd it just dummy, just so to load the node-label since its already created
char name_2[16];
strcpy(name_2, "DUMMY");//dummy, when `create_node_label` will check if this already exists, then will load node label
cluster::bridged_device_basic_information::attribute::create_node_label(cluster, name_2, strlen(name_2));
ESP_LOGW(TAG, "REGISTER BRIDGE, Cluster:%s Label Node:%s", cluster != NULL? "Yes":"No", name);
//------------ Added -------------//

// Enable the created endpoint
esp_matter::endpoint::enable(new_dev->dev->endpoint);

[pimpalemahesh]

@ahmed8812 We are working on it and will fix this soon.

[pimpalemahesh]

@ahmed8812 Move the node label attribute creation logic to the app_main.cpp/create_bridge_devices() callback.

You can use following code snippet as a reference:

using namespace chip::app::Clusters;

esp_err_t create_bridge_devices(esp_matter::endpoint_t *ep, uint32_t device_type_id, void *priv_data)
{
    esp_err_t err = ESP_OK;

    cluster_t *cluster = cluster::get(ep, BridgedDeviceBasicInformation::Id);
    char name[16];
    snprintf(name, sizeof(name), "NODE_LABEL");
    cluster::bridged_device_basic_information::attribute::create_node_label(cluster, name, strlen(name));
    
    ...
}

Additional extra data models can be added within this callback as needed.

[ahmed8812]

Hi @pimpalemahesh

I tried this before I did the patch. It wont work. It will not load the node label unless I did the patch first then I can load the node labels.

Thanks

[pimpalemahesh]

@ahmed8812 Please close this issue if it is resolved!

[ahmed8812]

Will the patch be ready on new release?

On Mon, 16 Sept 2024, 17:24 Mahesh, @.***> wrote:

@ahmed8812 https://github.com/ahmed8812 Please close this issue if it is resolved!

— Reply to this email directly, view it on GitHub https://github.com/espressif/esp-matter/issues/1013#issuecomment-2352410175, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2MY7UOHDBHXFXELETIIK2TZW2PV3AVCNFSM6AAAAABK5UDJBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJSGQYTAMJXGU . You are receiving this because you were mentioned.Message ID: @.***>

[pimpalemahesh]

@ahmed8812 Node Label is optional attribute so this changes are user specific. These changes are expected to be made by the user as per requirement.

[ahmed8812]

I really hope the issue resolved in the latest release