TC-RR-1.1 Constrain Error (CON-1074)

[PhLuReh]

Describe the bug
In TC-RR-1.1 from python_testing library the Node-Label of the BasicInformation cluster is written with the sequence Before Subscriptions 12345678912. This one is 32 characters long. It results in the failure:

IM Error 0x00000587: General error: 0x87 (CONSTRAINT_ERROR)

If I shorten the sequence by 1 it writes with SUCCESS. I read through the spec, an the maximum length is set to 32. But the situation is really unclear (from Matter 22-27349-002_Matter-1.1-Core-Specification.pdf):

Note that the character string type is a bounded sequence of characters whose size bound format is
not specified in the data model, but rather a property of the underlying encoding. Therefore, no
assumptions are to be made about the presence or absence of a length prefix or null-terminator
byte, or other implementation considerations.

What should we do? Am I the first one running this test?

Environment
ESP-Matter commit f511d22
ESP-IDF commit e088c3766ba440e72268b458a68f27b6e7d63986
connectedhomeip commit 32992decc576355eb7bedc97c3e18c8efdd58e6c
SOC: ESP32
Host Machine OS: Linux version 6.1.29-1-MANJARO (builduser@fv-az292-908) (gcc (GCC) 12.2.1 20230201, GNU ld (GNU Binutils) 2.40) #1 SMP PREEMPT_DYNAMIC Wed May 17 14:00:55 UTC 2023

• Commissioner app and versions if present: chip-tool from official TH v2.8.1-official
• Commissioner's logs if present:
  20240307-TC-RR-1_1-chiptool.log

[shubhamdp]

@PhLuReh We have identified this issue, and the fix will be out soon.

[shubhamdp]

@PhLuReh can you please try this on main branch, we have pushed out the change d481fa9

[shubhamdp]

And backports for release/v1.1 and release/v1.2 will be out soone

[PhLuReh]

Ok, thanks, that was quick, but I need to wait for release/v1.1.

[PhLuReh]

I tried to do my best and created a patch for release/v1.1. If you are keen, you could use this a base or check if I did something good. I will test this fix on my hardware later this afternoon.
esp-matter-862.patch

[shubhamdp]

@PhLuReh changes are ready, but having some other troubles, getting mainline, I'll give you a patch till that time. Hopefully this patch will be mainlined by today or tomorrow.
657.patch

[PhLuReh]

I uploaded the firmware to the device, but neither mine nor your patch is working. Still getting the CONSTRAINT_ERROR with the chip-tool. I again read through the sources but I'm missing the point where the check is done. I only found a hint in the source:

    // The size in the attribute metadata remains constant and is verified during write operations.
    uint16_t max_val_size;

Doesn't the write operation also need a patch?

[PhLuReh]

I also wanted to add the git status of my modified esp matter sdk. The esp_matter_ota.cpp is modified to #815 (comment)

root@smartproducts-server:/opt/espressif/esp-matter/components/esp_matter# git status .
On branch release/v1.1
Your branch is up to date with 'origin/release/v1.1'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   esp_matter_attribute.cpp
        modified:   esp_matter_attribute.h
        modified:   esp_matter_cluster.cpp
        modified:   esp_matter_cluster.h
        modified:   esp_matter_core.cpp
        modified:   esp_matter_core.h
        modified:   esp_matter_feature.h
        modified:   esp_matter_ota.cpp

[shubhamdp]

Doesn't the write operation also need a patch?

This check happens in attribute_storage.cpp.

Which attribute are you trying to write? I tested with node-label and it worked, can you please share the logs

[PhLuReh]

I was just referring to the commend inside the code, which says verified during write operations.

This check happens in attribute_storage.cpp.

attribute_storage.cpp is not part of the patch, nor of the commit?

The chip-tool log is still the same. I try to send a DUT-log in 3 hours.

[PhLuReh]

I reapplied your patch 657, and rand some attribute write commands:
20240314_TC-RR-1_1-DUT.txt
20240314_TC-RR-1_1-chiptool.txt

Device now only allows 13 characters to be written: ./chip-tool basicinformation write node-label "12345567890123" 1 0
14th character is rejected:

./chip-tool basicinformation write node-label "123455678901234" 1 0
Run command failure: IM Error 0x00000587: General error: 0x87 (CONSTRAINT_ERROR)

[shubhamdp]

Thanks @PhLuReh for testing this out, I found the problem, it was fixed onwards v1.2 and was not backported, 28b37de.

This occurs after first reboot.

[PhLuReh]

Thanks for pointing this out. For those who are searching for it, I found the corresponding lines in esp_matter_core.cpp (looks like it was refactored in v1.2).

With patch 657 and the fix in 28b37de, I'm still not able to write 32 characters, only 31:
202403141352_TC-RR-1_1-chiptool.txt
202403141352_TC-RR-1_1-DUT.txt

I still do not know, where attribute->max_val_size get's checked on write access - and I think this is the problem.

[shubhamdp]

@PhLuReh You would still need the 657.patch

[PhLuReh]

@PhLuReh You would still need the 657.patch

that's excatly what I wrote and have:

With patch 657 and the fix in 28b37de

[shubhamdp]

Sorry for all the fuzz. I tried following and it works in both first boot and after every boot. can you please try --> git am str-attr-fix.patch.

esp-matter: f511d22

chip-tool basicinformation write node-label "01234567890123456789012345678901" 1 0

chip-tool.log

[PhLuReh]

As git am was new to me, this was the output, after checking out heat of the previously patched files:

Applying: Fix problems with string type attributes
Applying: Fix active locale attributes
Applying: Fix the return type for invalid event id
Applying: Fix CI building for esp-now bridge
Applying: update the size after updating total size

I now also ran the original code from the test and both are returning SUCCESS now:

./chip-tool basicinformation write node-label "Before Subscriptions 12345678912" 1 0

Will there still be effort to merge this into release/v1.1?

[shubhamdp]

Thanks @PhLuReh for your confirmation. This patch is merged to release/v1.1.