[BUG] Java API does not expose list reads on a reasonable way

[jonsmirl]

Reproduction steps

Is this correct behavior? I have a wild card subscription and the Descriptor attributes are coming back in pieces.
Do other attributes report in pieces? If so, how can you differentiate a piece from a change in the attribute?

These are the events I am getting from the subscription API.

2023-08-14 01:26:28.250  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000004 [29, 31, 40, 48, 49, 51, 60, 62, 63, 54, 4294179841]
2023-08-14 01:26:28.251  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000004 []
2023-08-14 01:26:28.258  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000004 [29, 3, 4, 5, 80, 30, 6, 8, 768, 59, 64, 65]
2023-08-14 01:26:28.259  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000004 [6, 8, 768]
2023-08-14 01:26:28.261  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000004 [29]
2023-08-14 01:26:28.262  3036-3121  DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000004 []

Bug prevalence

always

GitHub hash of the SDK that was being used

5b4f800

Platform

core

Platform Version(s)

No response

Anything else?

No response

[yunhanw-google]

DeviceTypeList
ServerList
ClientList
PartsList
TagList

See these attributes, they are list, if it is too long to fit in a packet, the interaction model code would handle reports with chunks.

Chunking required when reading a large list or a large set of attributes.
the minimal IPv6 MTU is 1280 bytes, and we use 1024 bytes as a safe upper bound for encoding interaction model payloads. Then it is not enough to fill the response in a single read response when there are a few fabrics on the device.
The case for reading a large set of attributes is just the case for wildcard reading – we can only fit 30-40 AttributeDataIB-s in a single AttributeDataReport message, while reading all attributes in all clusters under on all endpoints will results in a report for hundreds of attributes, makes it critical for supporting chunking in report.

[jonsmirl]

So how do I differentiate a single chuncked response from multiple responses? There needs to be an indicator in the response API to tell my code to combine or replace.

[bzbarsky-apple]

There needs to be an indicator in the response API to tell my code to combine or replace.

There is. The attribute path tells you what's going on. See the mListOp member of ConcreteDataAttributePath.

[jonsmirl]

@bzbarsky-apple Can you please reopen, I don't have the ability.

As far as I can see the Java API is not exposing mListOp.

https://github.com/project-chip/connectedhomeip/blob/master/src/controller/java/AndroidCallbacks.cpp#L254

ReportCallback::OnAttributeData receives the ConcreteDataAttributePath. When it creates the Java AttributeState object it does not include the mListOp status telling you to append, replace, etc.

[jonsmirl]

@yunhanw-google Shouldn't this API be coalescing the chunking into a single value and not sending the pieces into the user app?

BTW, thread is chunking lots of stuff. Previously I was using wifi and it never chunked anything. When I started testing with Thread devices I immediately hit this chunking issue.

[yunhanw-google]

@jonsmirl Android's OnReportCallback use ClusterStateCache, and ClusterStateCache use bufferedReadCallback
https://github.com/project-chip/connectedhomeip/blob/master/src/controller/java/AndroidCallbacks.h#L85
https://github.com/project-chip/connectedhomeip/blob/master/src/app/ClusterStateCache.h#L659
The chunked list has been coalesced into a single value in https://github.com/project-chip/connectedhomeip/blob/master/src/app/BufferedReadCallback.cpp#L84

Therefore when using Java/Jni's onReport, https://github.com/project-chip/connectedhomeip/blob/master/src/controller/java/AndroidCallbacks.cpp#L219
You should get a complete view for the NodeState, where you can retrieve each attribute as a whole under endpoint/cluster/attribute with tlv blob or json format.

[jonsmirl]

So attributeState.getValue where the 'value' is returned as Java ChipStructs is basically unusable? I have to rework everything to use JSON or TLV directly? That is giant pain because I round trip these values. I modify the structs and then write them back to the CHIP attributes. And the CHIP API requires the ChipStructs. I need to go explore Kotlin json support. Maybe the Kotlin json library can convert these back into ChipStructs.

Does this same problem occur on a direct read of the attributes?

[jonsmirl]

This is a total mess for user apps. Now I have to add code to convert every possible JSON object back into ChipStructs. That is the kind of code zap should be generating. This API should be returning coalesced ChipStructs so that they can be round-tripped back into the CHIP API.

[yunhanw-google]

Sorry, we do provide 3 output in report, JSON, TLV/AttributeTLVValue
You can still use attributeState.getValue with "USE_JAVA_TLV_ENCODE_DECODE"

connectedhomeip/src/controller/java/BUILD.gn

Line 72 in 990bcea

if (matter_enable_java_generated_api) {

. We provide the option to enable/disable these generated IM and cluster code since some users in community don't wanna use them and wanna decrease the size. we could introduce another flag to control the only zap-generated/CHIPAttributeTLVValueDecoder.cpp.
Now this flag is enabled at default.

You could use JSON or Tlv as well, you may need to match JSOO or TLV to cluster attribute.
We also provide the kotlin JSON/Tlv conversion support https://github.com/project-chip/connectedhomeip/tree/990bceaed0c3fd2af38561d4950f8a5e760c5d31/src/controller/java/src/chip/jsontlv

[jonsmirl]

The JSON is identical to the ChipStruct, neither are coalesced.

First line is the ChipStruct, second is the JSON from each attribute report.

2023-08-30 13:11:08.651 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000019 [29, 31, 40, 48, 49, 51, 60, 62, 63, 54, 42]
2023-08-30 13:11:08.652 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000019 {"value":[29,31,40,48,49,51,60,62,63,54,42]}
2023-08-30 13:11:08.653 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000019 [41]
2023-08-30 13:11:08.655 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000019 {"value":[41]}
2023-08-30 13:11:08.660 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000019 [29, 3, 4, 5, 6, 8, 768]
2023-08-30 13:11:08.662 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Server ab0de00000000019 {"value":[29,3,4,5,6,8,768]}
2023-08-30 13:11:08.663 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000019 []
2023-08-30 13:11:08.664 11409-11482 DeviceServ...1$onReport com.lowpan.m2                        D  JDS Descriptor Client ab0de00000000019 {"value":[]}

                                        } else if ((clusterId == ChipClusters.DescriptorCluster.CLUSTER_ID) && (attributeId == 1L)) {
                                            Timber.d(
                                                "JDS Descriptor Server %x %s",
                                                deviceId,
                                                attributeState.value.toString()
                                            )
                                            Timber.d(
                                                "JDS Descriptor Server %x %s",
                                                deviceId,
                                                attributeState.json.toString()
                                            )
                                        } else if ((clusterId == ChipClusters.DescriptorCluster.CLUSTER_ID) && (attributeId == 2L)) {
                                            Timber.d(
                                                "JDS Descriptor Client %x %s",
                                                deviceId,
                                                attributeState.value.toString()
                                            )
                                            Timber.d(
                                                "JDS Descriptor Client %x %s",
                                                deviceId,
                                                attributeState.json.toString()
                                            )
                                        }

[jonsmirl]

DeviceTypeList ServerList ClientList PartsList TagList

See these attributes, they are list, if it is too long to fit in a packet, the interaction model code would handle reports with chunks.

Chunking required when reading a large list or a large set of attributes. the minimal IPv6 MTU is 1280 bytes, and we use 1024 bytes as a safe upper bound for encoding interaction model payloads. Then it is not enough to fill the response in a single read response when there are a few fabrics on the device. The case for reading a large set of attributes is just the case for wildcard reading – we can only fit 30-40 AttributeDataIB-s in a single AttributeDataReport message, while reading all attributes in all clusters under on all endpoints will results in a report for hundreds of attributes, makes it critical for supporting chunking in report.

You said that earlier, but how does this work on Thread with an MTU of 127 bytes? If only these (DeviceTypeList ServerList ClientList PartsList TagList) are impacted, I don't care I am just printing them out as an easy way to test.

But are ACLs going to get chunked on Thread? I can't tolerate partial ACL responses. Modifying the ACL has to be a read-modify-write operation.

If this API is going to send chunks into the user apps, then doesn't it have to include the mListOp field so that I can know what to do with the chunks?

    enum class ListOperation
    {
        NotList,     // Path points to an attribute that isn't a list.
        ReplaceAll,  // Path points to an attribute that is a list, indicating that the contents of the list should be replaced in
                     // its entirety.
        ReplaceItem, // Path points to a specific item in a list, indicating that that item should be replaced in its entirety.
        DeleteItem,  // Path points to a specific item in a list, indicating that that item should be deleted from the list.
        AppendItem   // Path points to an attribute that is a list, indicating that an item should be appended into the list.
    };

[yunhanw-google]

As I mentioned, from NodeState, you would see the attribute as a whole via json/tlv/ChipStruct, the underlying bufferReader in c++ layer has coalesced them together as a attribute if the list is oversized and sent via chunk

[jonsmirl]

I printed it out above, the json is not coalesced.

Timber.d(
"JDS Descriptor Server %x %s",
deviceId,
attributeState.json.toString()
}

[yunhanw-google]

you may add additional log in https://github.com/project-chip/connectedhomeip/blob/master/src/app/BufferedReadCallback.cpp#L70, you may confirm whether the actual coalescing happens or not

[jonsmirl]

I need to investigate this more. There is a bridge in the system and after debugging this appears it could be from the bridge and not directly from the chunking code. I added printing code as you suggested including mListOp and they are all 1, if it was a local chunking issue there would be some 2s. So maybe the chunks aren't making it through the bridge as expected. I will open new bug when I figure out more.

[yunhanw-google]

@jonsmirl #28970
I just write one test, I create several attribute path including one list with serverList, I confirm the server list is chunked, then I dump its json string, it shows the attribute is complete, which means chunked list is working as expected

[jonsmirl]

Now that we figured out to look at the bridge code we found the problem. Those reports that look like chunks aren't really chunks. They are actually the Descriptors from three different endpoints on the device and the bridge was not tracking the EPs correctly and reported them all on EP0.

@yunhanw-google thanks for the help looking at this.