feat(matter): add large-payload TCP transport capability with MRP fallback

[cleithner-comcast]

Add a kLargePayloadWithMRPFallback transport capability that prefers a TCP session
for large payloads (e.g. WebRTC SDP) and transparently falls back to MRP/UDP for peers
without TCP support. Use it as the default for all controller connections.

• Patch the Matter library: add the capability and its SessionManager/
  OperationalSessionSetup handling, default GetConnectedDevice to it, and fall back to
  UDP (with a log) when the active TCP connection pool is full.
• Allow large-payload CommandSender buffers when the session supports them.
• Initiate GetConnectedDevice synchronously on the Matter thread in ConnectAndExecute.
• Log the GetConnectedDevice error in DeviceDataCache.
• Raise CHIP_CONFIG_MAX_ACTIVE_TCP_CONNECTIONS from 4 to 50.

[Copilot]

Pull request overview

Adds a new Matter transport payload capability that prefers TCP for large payloads but gracefully falls back to MRP/UDP when the peer doesn’t support TCP server mode, and wires this capability through connection/session and command-sending flows to avoid large-payload serialization failures.

Changes:

• Introduces kLargePayloadWithMRPFallback via a Matter library patch (SessionManager + OperationalSessionSetup behavior).
• Extends MatterDeviceDriver::ConnectAndExecute and controller connection initiation paths to pass a transport payload capability (defaulting to the new fallback mode).
• Enables CommandSender to allow large payload buffers when the underlying session supports it, and uses the fallback capability for DeviceDataCache reads.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
third_party/matter/barton-library/patches/0003-transport-add-kLargePayloadWithMRPFallback-capabilit.patch	Adds new transport payload capability and fallback selection logic to the patched Matter stack.
core/src/subsystems/matter/DeviceDataCache.cpp	Uses the new fallback capability for cache connection/reads; forwards subscription-established notification to registered callback.
core/deviceDrivers/matter/MatterDeviceDriver.h	Adds a transport payload capability parameter to ConnectAndExecute with a default of fallback mode.
core/deviceDrivers/matter/MatterDeviceDriver.cpp	Passes the capability into GetConnectedDevice via a scheduled SystemLayer lambda.
core/deviceDrivers/matter/MatterDevice.cpp	Enables large-payload support in CommandSender when the session allows it; improves error logging for deferred command TLV handling.

[tleacmcsa]

It looks like the point is to make all of our connections use the new transport capability. Why not make that the default rather than trying to catch all the connection points?

Looks good to me after you address comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[cleithner-comcast]

The previous force push was an agent overstepping, i've restored the branch to the first pushed commit while I review comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

core/deviceDrivers/matter/MatterDeviceDriver.cpp:969

• ConnectAndExecute now relies on RunOnMatterSync to run GetConnectedDevice, but RunOnMatterSync can return early / fail to schedule its lambda (see RunOnMatterSync: ScheduleLambda error path). In that case getConnectedErr stays CHIP_NO_ERROR and this code will block until timeout, and the later cancel attempt may also not run, risking callbacks firing after ConnectAndExecute returns.

        chip::TransportPayloadCapability transportPayloadCapability;
    };

    ConnectScheduleContext scheduleContext {
        Subsystem::Matter::UuidToNodeId(deviceId), &successCb, &failCb, transportPayloadCapability};

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

core/deviceDrivers/matter/MatterDeviceDriver.cpp:976

• scheduleContext is a stack object that’s captured by reference in a lambda scheduled onto the Matter SystemLayer. If ConnectAndExecute hits the timeout path (or otherwise returns early), the scheduled lambda may still run later and dereference scheduleContext (and the callback pointers it contains), causing a use-after-free. The nearby comment claiming the context outlives the async task is not true on the timeout path.

To make this safe, the scheduled work needs to capture an owning heap object (e.g., std::shared_ptr state containing the nodeId, capability, and owning storage for the callbacks/promise) and/or use an abort flag so late execution becomes a no-op after timeout.

    // SDK event size limitations (LambdaBridge caps captures at 24 bytes) prevent directly
    // capturing too many objects. Bundle everything the scheduled task needs into a single
    // local context and capture just a pointer to it. The context outlives the async task
    // because this function blocks on connectFuture until the work completes or times out.
    // connectPromise is directly pointed at in failCb.mContext, so it is indirectly
    // captured via failCb.
    struct ConnectScheduleContext
    {
        chip::NodeId nodeId;
        chip::Callback::Callback<OnDeviceConnected> *successCb;
        chip::Callback::Callback<OnDeviceConnectionFailure> *failCb;
        chip::TransportPayloadCapability transportPayloadCapability;
    };

    ConnectScheduleContext scheduleContext {
        Subsystem::Matter::UuidToNodeId(deviceId), &successCb, &failCb, transportPayloadCapability};

    auto err = chip::DeviceLayer::SystemLayer().ScheduleLambda([&scheduleContext]() {
        CHIP_ERROR getConnectedErr =
            Matter::GetInstance().GetCommissioner()->GetConnectedDevice(scheduleContext.nodeId,
                                                                        scheduleContext.successCb,
                                                                        scheduleContext.failCb,
                                                                        scheduleContext.transportPayloadCapability);

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

[cleithner-comcast]

It looks like the point is to make all of our connections use the new transport capability. Why not make that the default rather than trying to catch all the connection points?

Looks good to me after you address comments.

Good callout, I modified the patch to make this "Prefer TCP, fallback to UDP" behavior the default so call sites aren't opt-in anymore.

Additionally, I remembered that the default number of concurrent TCP connections is 4 so I upped it to 50 (match UDP), as well as modified the patch to detect a full TCP session pool and fallback to UDP plus log the issue. I don't expect us to run into this ever but wanted something detectable and non-fatal in case we do.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

third_party/matter/barton-library/patches/0003-transport-add-kLargePayloadWithMRPFallback-capabilit.patch:53

• Changing the default GetConnectedDevice transportPayloadCapability from kMRPPayload to kLargePayloadWithMRPFallback makes TCP-preference the implicit behavior for all call sites that omit this argument. That’s a behavioral/API change (opt-out instead of opt-in) and conflicts with the stated goal of letting a controller request TCP when needed. In BartonCore there are existing calls that rely on the default (e.g. commissioning/discovery paths), which would now start preferring TCP unintentionally.

Consider keeping the default as kMRPPayload and explicitly passing kLargePayloadWithMRPFallback only in the specific paths that need it (e.g. DeviceDataCache reads / large commands).

     GetConnectedDevice(NodeId peerNodeId, Callback::Callback<OnDeviceConnected> * onConnection,
                        Callback::Callback<OnDeviceConnectionFailure> * onFailure,
-                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kMRPPayload)
+                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kLargePayloadWithMRPFallback)
     {

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

third_party/matter/barton-library/patches/0003-transport-add-kLargePayloadWithMRPFallback-capabilit.patch:53

• This patch changes the default transportPayloadCapability for GetConnectedDevice(...) from kMRPPayload to kLargePayloadWithMRPFallback, which is an API/behavior change for every caller that relies on the default (e.g. discovery/commissioning paths). Consider keeping the default as kMRPPayload and making large-payload-with-fallback an explicit opt-in at call sites that need it, to avoid unexpected increases in TCP session attempts and connection pool usage.

     virtual CHIP_ERROR
     GetConnectedDevice(NodeId peerNodeId, Callback::Callback<OnDeviceConnected> * onConnection,
                        Callback::Callback<OnDeviceConnectionFailure> * onFailure,
-                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kMRPPayload)
+                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kLargePayloadWithMRPFallback)
     {

third_party/matter/barton-library/patches/0003-transport-add-kLargePayloadWithMRPFallback-capabilit.patch:62

• Same concern as above: changing the default payload capability here alters connection behavior for all callers that omit the argument. Prefer leaving the default as kMRPPayload and opting in explicitly where large-payload/TCP preference is needed.

     CHIP_ERROR
     GetConnectedDevice(NodeId peerNodeId, Callback::Callback<OnDeviceConnected> * onConnection,
                        chip::Callback::Callback<OperationalSessionSetup::OnSetupFailure> * onSetupFailure,
-                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kMRPPayload)
+                       TransportPayloadCapability transportPayloadCapability = TransportPayloadCapability::kLargePayloadWithMRPFallback)
     {

core/src/subsystems/matter/DeviceDataCache.cpp:127

• PR description says DeviceDataCache reads should use kLargePayloadWithMRPFallback, but this call relies on the Matter library default instead of requesting the capability explicitly. Passing the capability here makes the intent clear and avoids behavior changes if the upstream default is adjusted later.

            CHIP_ERROR err =
                self->controller->GetConnectedDevice(barton::Subsystem::Matter::UuidToNodeId(self->deviceUuid),
                                                     &self->mOnDeviceConnectedCallback,
                                                     &self->mOnDeviceConnectionFailureCallback);