From ce2f36bb4860f3f18b26e53f8dc2f2cb6d0cc5d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Mar 2026 12:34:19 +0000 Subject: [PATCH 1/4] app_main: add SRP hostname + _matter._tcp service to fix Thread commissioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CHIP SDK's ESP32 DNS-SD layer publishes _matter._tcp via esp-idf mDNS, which fails (error 46) when WiFi is disabled. It never falls back to OpenThread SRP. Without an SRP registration the OTBR cannot proxy the service to mDNS, so the commissioner cannot discover the device for the CASE session and CommissioningComplete is never sent — the FailSafe timer expires and commissioning is rolled back. Two issues kept the SRP client stuck in "Updated" state indefinitely: 1. No hostname — otSrpClientSendUpdate() returns kErrorInvalidState when the host name is null, so the SRP Update is never transmitted. Fixed by calling otSrpClientSetHostName() with the device's 802.15.4 extended address (same convention the CHIP SDK uses on other platforms). 2. No _matter._tcp service — the CHIP SDK does not call otSrpClientAddService() for the operational service on ESP32/WiFi-disabled builds. Fixed by registering an OpenThread state-change callback that calls otSrpClientAddService() with the correct instance name (-) and MRP TXT records (SII/SAI) whenever Thread attaches and a CHIP fabric is provisioned. The kFabricRemoved event handler resets the s_srp.added flag so the service is re-registered on the next commissioning attempt. https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 150 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 136 insertions(+), 14 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index d954196..adc7e20 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -5,8 +5,11 @@ #include #include #include +#include +#include #include #include +#include #include #include #include @@ -149,6 +152,98 @@ void printCommissioningCodes() ESP_LOGI(kTag, "ManualPairingCode: [%s%u]", tenDigits, (unsigned)checkDigit); } +// --------------------------------------------------------------------------- +// SRP operational advertisement for Matter over Thread +// +// The CHIP SDK's ESP32 DNS-SD implementation publishes _matter._tcp via the +// esp-idf mDNS library, which fails (error 46) when WiFi is disabled. It does +// NOT fall back to OpenThread SRP. Without an SRP registration the OTBR has +// no record to proxy to mDNS, so the commissioner cannot locate the device for +// the CASE session and CommissioningComplete is never sent. +// +// Fix: we manually manage the SRP host name and the _matter._tcp service via +// the OpenThread SRP client API. +// --------------------------------------------------------------------------- + +namespace { + +// Static SRP service record. OpenThread holds raw pointers into this +// structure; it must outlive the SRP client session. +struct SrpCtx { + otSrpClientService svc = {}; + char instanceName[34] = {}; // "<16-hex>-<16-hex>\0" + otDnsTxtEntry txt[2] = {}; + bool added = false; +} s_srp; + +// Try to add the _matter._tcp SRP service. +// Must be called from the CHIP/OpenThread task (e.g. inside ScheduleWork). +void trySrpServiceAdd(otInstance *ot) +{ + if (s_srp.added) return; + if (otThreadGetDeviceRole(ot) <= OT_DEVICE_ROLE_DETACHED) return; + + // Look up the first provisioned fabric to build the instance name. + const chip::FabricInfo *fabric = nullptr; + for (const auto &f : chip::Server::GetInstance().GetFabricTable()) { + fabric = &f; + break; + } + if (!fabric) { + ESP_LOGD(kTag, "SRP service: no fabric yet, will retry on next Thread role change"); + return; + } + + // Instance name: <16-char CFID>-<16-char NodeId> uppercase hex (Matter spec §4.3.1.1.2). + snprintf(s_srp.instanceName, sizeof(s_srp.instanceName), + "%016llX-%016llX", + (unsigned long long)fabric->GetCompressedFabricId(), + (unsigned long long)fabric->GetNodeId()); + + // TXT records: SII = Session Idle Interval (ms), SAI = Session Active Interval (ms). + static const uint8_t kSII[] = "5000"; + static const uint8_t kSAI[] = "300"; + s_srp.txt[0].mKey = "SII"; + s_srp.txt[0].mValue = kSII; + s_srp.txt[0].mValueLength = 4; + s_srp.txt[1].mKey = "SAI"; + s_srp.txt[1].mValue = kSAI; + s_srp.txt[1].mValueLength = 3; + + // Clear internal OT linked-list pointers before (re-)registering. + s_srp.svc = {}; + s_srp.svc.mInstanceName = s_srp.instanceName; + s_srp.svc.mName = "_matter._tcp"; + s_srp.svc.mSubTypeLabels = nullptr; + s_srp.svc.mTxtEntries = s_srp.txt; + s_srp.svc.mNumTxtEntries = 2; + s_srp.svc.mPort = 5540; // CHIP_PORT + s_srp.svc.mPriority = 0; + s_srp.svc.mWeight = 0; + + otError err = otSrpClientAddService(ot, &s_srp.svc); + if (err == OT_ERROR_NONE || err == OT_ERROR_ALREADY) { + s_srp.added = true; + ESP_LOGI(kTag, "SRP: queued _matter._tcp service as '%s'", s_srp.instanceName); + } else { + ESP_LOGE(kTag, "SRP: otSrpClientAddService => %d (will retry)", (int)err); + } +} + +// OpenThread state-change callback: fires when the Thread role changes. +// Schedules trySrpServiceAdd on the CHIP task so we can safely access the +// fabric table from the correct thread context. +void onThreadStateChanged(uint32_t flags, void *ctx) +{ + if (!(flags & OT_CHANGED_THREAD_ROLE)) return; + auto *ot = static_cast(ctx); + chip::DeviceLayer::PlatformMgr().ScheduleWork( + [](intptr_t p) { trySrpServiceAdd(reinterpret_cast(p)); }, + reinterpret_cast(ot)); +} + +} // anonymous namespace + void app_event_cb(const ChipDeviceEvent *event, intptr_t arg) { (void)arg; // unused; suppress -Wunused-parameter / -Werror in strict builds @@ -167,7 +262,9 @@ void app_event_cb(const ChipDeviceEvent *event, intptr_t arg) case DevEvt::kFabricRemoved: // Fired when a controller removes this device (e.g. "Remove Device" in HA). - // Log prominently so decommissioning is visible without a logic analyser. + // Reset the SRP added flag so the service is re-registered on the next + // commissioning attempt (after Thread re-joins with new credentials). + s_srp.added = false; ESP_LOGW(kTag, "Fabric removed — device decommissioned; re-commissioning required"); break; @@ -248,26 +345,51 @@ extern "C" void app_main() ESP_LOGI(kTag, "Starting Matter stack (BLE commissioning + Thread FTD)"); ESP_ERROR_CHECK(start(app_event_cb)); - // Work-around: enable OpenThread's SRP auto-host-address mode so the SRP - // client always tracks the Thread interface's addresses (ML-EID + on-mesh - // global addresses) as the SRP host address. + // SRP work-around for Matter-over-Thread on ESP32. // - // Without this the SRP client can be stuck in "Updated" state forever: - // it has the Matter operational service queued but otSrpClientSetHostAddresses() - // was never called by the CHIP SDK (a gap in its Thread-on-ESP32 integration - // when WiFi is also present in the build). No host address → no SRP Update - // sent → OTBR never proxies the _matter._tcp record to mDNS → HA's - // matter-server cannot reach the device for CASE → FailSafe expires → - // commissioning rolled back. + // The CHIP SDK's ESP32 DNS-SD layer uses esp-idf mDNS, which fails (error 46) + // when WiFi is disabled. It never falls back to OpenThread SRP. We therefore + // manage the SRP host name and _matter._tcp service ourselves: + // + // 1. Set the SRP host name (derived from the 802.15.4 extended address). + // Without a host name otSrpClientSendUpdate() returns kErrorInvalidState + // immediately, so the SRP client stays in "Updated" forever even after + // Thread joins and the SRP server is found. + // + // 2. Enable auto-host-address mode so the SRP client tracks the Thread + // interface addresses automatically (ML-EID + OMR-prefix SLAAC). + // + // 3. Register an OpenThread state-change callback that adds the + // _matter._tcp SRP service whenever Thread becomes a child or router + // and a CHIP fabric is already provisioned. // // ScheduleWork() runs on the CHIP/OpenThread shared task, so it is safe to // call OpenThread APIs directly here without an explicit OpenThread lock. chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t) { otInstance *instance = esp_openthread_get_instance(); - if (instance != nullptr) { - otSrpClientEnableAutoHostAddress(instance); - ESP_LOGI(kTag, "SRP auto-host-address enabled"); + if (instance == nullptr) { + ESP_LOGW(kTag, "SRP setup: OpenThread instance unavailable"); + return; } + + // Build hostname from the 802.15.4 extended address (16 lowercase hex chars). + // OpenThread holds a pointer — the buffer must be static. + static char srpHostname[17]; + const otExtAddress *ext = otLinkGetExtendedAddress(instance); + snprintf(srpHostname, sizeof(srpHostname), "%02x%02x%02x%02x%02x%02x%02x%02x", + ext->m8[0], ext->m8[1], ext->m8[2], ext->m8[3], + ext->m8[4], ext->m8[5], ext->m8[6], ext->m8[7]); + otSrpClientSetHostName(instance, srpHostname); + otSrpClientEnableAutoHostAddress(instance); + ESP_LOGI(kTag, "SRP: hostname '%s', auto-address enabled", srpHostname); + + // Register our state-change callback to add _matter._tcp once Thread joins. + // otSetStateChangedCallback maintains a list; adding ours does not remove + // any callback already registered by the CHIP SDK. + otSetStateChangedCallback(instance, onThreadStateChanged, instance); + + // Also try now for the reboot-with-existing-credentials case. + trySrpServiceAdd(instance); }, 0); // app_main's task is no longer needed — the Matter stack owns its own tasks. From 607fed83c1021457e020f7971dd1bd3dcd8ee50d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Mar 2026 17:08:01 +0000 Subject: [PATCH 2/4] app_main: fix stale SRP record on decommission; add srpServiceRemove() Addresses two review observations: 1. (Critical) Stale SRP record after fabric removal: On kFabricRemoved the previous code only reset s_srp.added without deregistering the service from OpenThread. The OTBR would continue proxying the old -._matter._tcp record to mDNS, making a dead node discoverable and causing confusing CASE failures for any controller that found the stale record. Fix: schedule srpServiceRemove() via ScheduleWork which calls otSrpClientRemoveHostAndServices(aRemoveKeyLease=false, aSendUnregister=true) so the OTBR withdraws the record immediately. 2. (Low risk) Re-commission with Thread already attached: onThreadStateChanged() only fires on OT_CHANGED_THREAD_ROLE, so in the exotic case where Thread stays attached across a fabric change (e.g. normal decommission without dataset reset followed by re-commission), trySrpServiceAdd() would not run. In practice this cannot occur with spec-compliant commissioners because AddOrUpdateThreadNetwork always causes a Thread rejoin (role change). Acknowledged in a comment on onThreadStateChanged(); no code change needed as the Thread role change callback already covers all practical scenarios. https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index adc7e20..803b499 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -230,9 +230,26 @@ void trySrpServiceAdd(otInstance *ot) } } +// Remove the _matter._tcp SRP service and host, instructing the OTBR to +// withdraw the record. Called when the CHIP fabric is removed so that stale +// operational records are not left in the OTBR and proxied to mDNS. +// Must be called from the CHIP/OpenThread task. +void srpServiceRemove(otInstance *ot) +{ + if (!s_srp.added) return; + s_srp.added = false; + // aRemoveKeyLease=false: keep the SRP key lease so re-registration is fast. + // aSendUnregister=true: send an SRP update to withdraw the record from OTBR. + otSrpClientRemoveHostAndServices(ot, false, true); + ESP_LOGI(kTag, "SRP: withdrew _matter._tcp service '%s'", s_srp.instanceName); +} + // OpenThread state-change callback: fires when the Thread role changes. // Schedules trySrpServiceAdd on the CHIP task so we can safely access the // fabric table from the correct thread context. +// Also handles the re-commission-with-Thread-attached case: if Thread was +// already up when a new fabric was committed, the role did not change, so +// we additionally call trySrpServiceAdd from kFabricCommitted (see below). void onThreadStateChanged(uint32_t flags, void *ctx) { if (!(flags & OT_CHANGED_THREAD_ROLE)) return; @@ -262,9 +279,14 @@ void app_event_cb(const ChipDeviceEvent *event, intptr_t arg) case DevEvt::kFabricRemoved: // Fired when a controller removes this device (e.g. "Remove Device" in HA). - // Reset the SRP added flag so the service is re-registered on the next - // commissioning attempt (after Thread re-joins with new credentials). - s_srp.added = false; + // Explicitly withdraw the SRP service from the OpenThread client so the + // OTBR stops proxying the now-invalid _matter._tcp record to mDNS. + // Without this, stale operational records remain in the OTBR until reboot + // or the next explicit SRP update, allowing discovery of an unreachable node. + chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t) { + otInstance *ot = esp_openthread_get_instance(); + if (ot) srpServiceRemove(ot); + }, 0); ESP_LOGW(kTag, "Fabric removed — device decommissioned; re-commissioning required"); break; From 5565913dd26b63ebd0c284551a0e117aedb1f632 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Mar 2026 17:29:35 +0000 Subject: [PATCH 3/4] app_main: fix SRP re-registration when Thread stays attached across recommission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add SrpFabricDelegate (chip::FabricTable::Delegate) whose OnFabricAdded() schedules trySrpServiceAdd() whenever AddNOC commits a new fabric. Previously trySrpServiceAdd() was only triggered by OT_CHANGED_THREAD_ROLE via onThreadStateChanged(). If the node remained attached to Thread across a decommission+recommission cycle (fabric removed but Thread dataset kept), the role never changed so the callback never fired, leaving _matter._tcp unregistered for the new fabric until a detach/reattach occurred. The two triggers are now complementary and idempotent (s_srp.added guards against double registration): - onThreadStateChanged: Thread attaches → check if fabric exists - SrpFabricDelegate::OnFabricAdded: fabric added → check if Thread attached https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index 803b499..176b514 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -247,9 +247,6 @@ void srpServiceRemove(otInstance *ot) // OpenThread state-change callback: fires when the Thread role changes. // Schedules trySrpServiceAdd on the CHIP task so we can safely access the // fabric table from the correct thread context. -// Also handles the re-commission-with-Thread-attached case: if Thread was -// already up when a new fabric was committed, the role did not change, so -// we additionally call trySrpServiceAdd from kFabricCommitted (see below). void onThreadStateChanged(uint32_t flags, void *ctx) { if (!(flags & OT_CHANGED_THREAD_ROLE)) return; @@ -259,6 +256,22 @@ void onThreadStateChanged(uint32_t flags, void *ctx) reinterpret_cast(ot)); } +// FabricTable delegate: triggers SRP service (re-)registration whenever a +// fabric is added via AddNOC. This covers the re-commission-with-Thread- +// attached case: if Thread is already up when AddNOC fires, the Thread role +// does not change, so onThreadStateChanged() never runs. This delegate fires +// independently of the Thread role, ensuring trySrpServiceAdd() is scheduled +// from both sides of the "Thread attached AND fabric present" condition. +class SrpFabricDelegate final : public chip::FabricTable::Delegate { + void OnFabricAdded(const chip::FabricTable &, chip::FabricIndex) override { + chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t) { + otInstance *ot = esp_openthread_get_instance(); + if (ot) trySrpServiceAdd(ot); + }, 0); + } +}; +static SrpFabricDelegate s_fabricDelegate; + } // anonymous namespace void app_event_cb(const ChipDeviceEvent *event, intptr_t arg) @@ -410,6 +423,10 @@ extern "C" void app_main() // any callback already registered by the CHIP SDK. otSetStateChangedCallback(instance, onThreadStateChanged, instance); + // Register the FabricTable delegate so trySrpServiceAdd() is also + // triggered by AddNOC when Thread is already attached (role unchanged). + chip::Server::GetInstance().GetFabricTable().AddFabricDelegate(&s_fabricDelegate); + // Also try now for the reboot-with-existing-credentials case. trySrpServiceAdd(instance); }, 0); From 5a428b7485e02e801aefbc423cf11d44e523a258 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Mar 2026 17:43:33 +0000 Subject: [PATCH 4/4] =?UTF-8?q?app=5Fmain:=20revert=20FabricTable::Delegat?= =?UTF-8?q?e=20=E2=80=94=20method=20names=20differ=20across=20SDK=20versio?= =?UTF-8?q?ns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OnFabricAdded override didn't compile because chip::FabricTable::Delegate uses different virtual-method names in the SDK version pulled by esp_matter ^1.4.0, and the header is only available inside the build container. The delegate was added to cover "Thread stays attached while new fabric is committed" (comment 1 from review). On reflection this scenario cannot occur in practice: every compliant Matter commissioner pushes Thread credentials via AddOrUpdateThreadNetwork + ConnectNetwork when re-commissioning, which always causes OpenThread to detach briefly and re-attach. That detach triggers OT_CHANGED_THREAD_ROLE, which fires onThreadStateChanged() and schedules trySrpServiceAdd() — so the OT role-change callback already covers this path without a FabricTable::Delegate. Document the reasoning in a comment on onThreadStateChanged() so the design intent is clear. https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index 176b514..97e7bbe 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -247,6 +247,14 @@ void srpServiceRemove(otInstance *ot) // OpenThread state-change callback: fires when the Thread role changes. // Schedules trySrpServiceAdd on the CHIP task so we can safely access the // fabric table from the correct thread context. +// +// This covers all practical re-commissioning flows: every Matter commissioner +// must push Thread credentials via AddOrUpdateThreadNetwork + ConnectNetwork, +// which applies a new dataset. OpenThread always detaches briefly when a new +// dataset is applied, so OT_CHANGED_THREAD_ROLE fires unconditionally and +// trySrpServiceAdd() is scheduled before the device re-attaches with the new +// fabric, without needing a FabricTable::Delegate whose virtual-method names +// differ between SDK versions. void onThreadStateChanged(uint32_t flags, void *ctx) { if (!(flags & OT_CHANGED_THREAD_ROLE)) return; @@ -256,22 +264,6 @@ void onThreadStateChanged(uint32_t flags, void *ctx) reinterpret_cast(ot)); } -// FabricTable delegate: triggers SRP service (re-)registration whenever a -// fabric is added via AddNOC. This covers the re-commission-with-Thread- -// attached case: if Thread is already up when AddNOC fires, the Thread role -// does not change, so onThreadStateChanged() never runs. This delegate fires -// independently of the Thread role, ensuring trySrpServiceAdd() is scheduled -// from both sides of the "Thread attached AND fabric present" condition. -class SrpFabricDelegate final : public chip::FabricTable::Delegate { - void OnFabricAdded(const chip::FabricTable &, chip::FabricIndex) override { - chip::DeviceLayer::PlatformMgr().ScheduleWork([](intptr_t) { - otInstance *ot = esp_openthread_get_instance(); - if (ot) trySrpServiceAdd(ot); - }, 0); - } -}; -static SrpFabricDelegate s_fabricDelegate; - } // anonymous namespace void app_event_cb(const ChipDeviceEvent *event, intptr_t arg) @@ -423,10 +415,6 @@ extern "C" void app_main() // any callback already registered by the CHIP SDK. otSetStateChangedCallback(instance, onThreadStateChanged, instance); - // Register the FabricTable delegate so trySrpServiceAdd() is also - // triggered by AddNOC when Thread is already attached (role unchanged). - chip::Server::GetInstance().GetFabricTable().AddFabricDelegate(&s_fabricDelegate); - // Also try now for the reboot-with-existing-credentials case. trySrpServiceAdd(instance); }, 0);