From 22a77f48a02381815f66ea9defd5ffd4510c4ce6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 20 Mar 2026 07:27:30 +0000 Subject: [PATCH 1/2] =?UTF-8?q?app=5Fmain:=20fix=20commissioning=20failure?= =?UTF-8?q?=20=E2=80=94=20SRP=20record=20not=20updated=20after=20addNoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During commissioning the controller's "Reconnect" step (step 18) needs to resolve the device's NEW operational address via mDNS before it can open a CASE session. The mDNS record is sourced from the OTBR's SRP proxy. Root cause: trySrpServiceAdd() ran when Thread first attached (via the onThreadStateChanged callback) and built the SRP instance name from the FIRST fabric in the table at that time (either an old committed fabric from a previous session, or nothing if no fabric existed yet). When addNoc subsequently committed a new fabric with a different CompressedFabricId / NodeId, the OTBR SRP record still carried the old instance name. DNS-SD resolution for the new -._matter._tcp name timed out (~35 s) and commissioning failed with "something went wrong". Fix: add a FabricTable::Delegate (SrpFabricDelegate) that overrides OnFabricCommitted. It fires synchronously on the CHIP/OpenThread task immediately after addNoc persists the new fabric, giving enough time to update the SRP record before the CASE reconnect DNS-SD lookup is issued. The delegate: 1. Clears any stale SRP service (otSrpClientClearHostAndServices — avoids the OnSrpClientNotification crash fixed in the previous commit). 2. Calls trySrpServiceAdd() with the specific newly committed FabricIndex so FindFabricWithIndex() picks the right fabric even when an older fabric still occupies index 1. trySrpServiceAdd() gains an optional preferIndex parameter for this path; the onThreadStateChanged / startup paths continue to use the first-fabric fallback as before. The delegate is registered in the startup ScheduleWork after chip::Server is initialised. https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 60 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index 271ae93..621ad4d 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -178,16 +178,25 @@ struct SrpCtx { // Try to add the _matter._tcp SRP service. // Must be called from the CHIP/OpenThread task (e.g. inside ScheduleWork). -void trySrpServiceAdd(otInstance *ot) +// preferIndex: if not kUndefinedFabricIndex, use that specific fabric (e.g. +// the one just committed by addNoc) instead of falling back to the first entry. +void trySrpServiceAdd(otInstance *ot, + chip::FabricIndex preferIndex = chip::kUndefinedFabricIndex) { if (s_srp.added) return; if (otThreadGetDeviceRole(ot) <= OT_DEVICE_ROLE_DETACHED) return; - // Look up the first provisioned fabric to build the instance name. + // Prefer the specific fabric index when supplied (e.g. from OnFabricCommitted). + // Fall back to the first fabric in the table for the Thread-attach path. const chip::FabricInfo *fabric = nullptr; - for (const auto &f : chip::Server::GetInstance().GetFabricTable()) { - fabric = &f; - break; + if (preferIndex != chip::kUndefinedFabricIndex) { + fabric = chip::Server::GetInstance().GetFabricTable().FindFabricWithIndex(preferIndex); + } + if (!fabric) { + 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"); @@ -273,6 +282,43 @@ void srpServiceRemove(otInstance *ot) s_srp.instanceName); } +// FabricTable delegate: refreshes the SRP _matter._tcp record when a new NOC +// is committed (addNoc / updateNoc succeeds, step 6 of the commissioning flow). +// +// Problem: the commissioner's "Reconnect" step (step 7) needs to resolve the +// device's NEW operational address via DNS-SD/mDNS before it can open a CASE +// session. If the SRP record in OTBR still carries the OLD fabric's instance +// name (-._matter._tcp), DNS-SD resolution times out +// (~35 s) and commissioning fails. +// +// OnFabricCommitted fires synchronously on the CHIP/OpenThread task immediately +// after addNoc persists the fabric, giving us time to update the SRP record +// before the CASE reconnect is attempted. +class SrpFabricDelegate : public chip::FabricTable::Delegate +{ +public: + void OnFabricCommitted(const chip::FabricTable &, chip::FabricIndex fabricIndex) override + { + otInstance *ot = esp_openthread_get_instance(); + if (!ot) return; + + // Clear any stale record registered under a different fabric's credentials. + // Use Clear (not RemoveHostAndServices) — see srpServiceRemove() for why. + if (s_srp.added) { + s_srp.added = false; + otSrpClientClearHostAndServices(ot); + ESP_LOGI(kTag, "SRP: cleared old record for fabric update"); + } + + // Re-register immediately using the specific newly committed fabric so + // the OTBR has the correct -._matter._tcp entry before + // the commissioner's DNS-SD resolution attempt. + trySrpServiceAdd(ot, fabricIndex); + } +}; + +static SrpFabricDelegate s_fabricDelegate; + // 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. @@ -438,6 +484,10 @@ extern "C" void app_main() otSrpClientEnableAutoHostAddress(instance); ESP_LOGI(kTag, "SRP: hostname '%s', auto-address enabled", srpHostname); + // Register our FabricTable delegate so the SRP record is refreshed + // immediately when addNoc commits a new fabric during commissioning. + chip::Server::GetInstance().GetFabricTable().AddFabricDelegate(&s_fabricDelegate); + // 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. From bacda27d05bf825d483134800e86c0c92c4d9c9c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 20 Mar 2026 07:46:40 +0000 Subject: [PATCH 2/2] app_main: re-apply SRP host config after Clear in OnFabricCommitted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit otSrpClientClearHostAndServices() resets the SRP client's host state (hostname pointer → null, auto-host-address mode disabled) in addition to removing services. The previous commit called it in SrpFabricDelegate::OnFabricCommitted but then only called trySrpServiceAdd(), which goes straight to otSrpClientAddService(). Without a configured host, the SRP client rejects AddService or silently fails to send an update, so the new -._matter._tcp record never reaches OTBR and CASE reconnect still times out. Fix: - Extract setupSrpHost(otInstance*): sets hostname and re-enables auto-host-address mode using the buffer in s_srp. - Move the hostname buffer from a lambda-local static into SrpCtx (s_srp.hostname) so setupSrpHost() and the delegate share one buffer. - Call setupSrpHost() in OnFabricCommitted immediately after Clear and before trySrpServiceAdd(). - Startup lambda uses setupSrpHost() too, eliminating the duplicate snprintf/SetHostName/EnableAutoHostAddress block. https://claude.ai/code/session_01SxqAiQApiRTwXefk9aQ6bR --- main/app_main.cpp | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/main/app_main.cpp b/main/app_main.cpp index 621ad4d..36cb4b3 100644 --- a/main/app_main.cpp +++ b/main/app_main.cpp @@ -167,15 +167,31 @@ void printCommissioningCodes() namespace { -// Static SRP service record. OpenThread holds raw pointers into this -// structure; it must outlive the SRP client session. +// Static SRP context. OpenThread holds raw pointers into this structure; +// it must outlive the SRP client session. struct SrpCtx { - otSrpClientService svc = {}; + otSrpClientService svc = {}; char instanceName[34] = {}; // "<16-hex>-<16-hex>\0" - otDnsTxtEntry txt[2] = {}; - bool added = false; + char hostname[17] = {}; // 16 hex chars + NUL + otDnsTxtEntry txt[2] = {}; + bool added = false; } s_srp; +// (Re-)apply the SRP host name and auto-address mode. +// Must be called on the CHIP/OpenThread task. +// otSrpClientClearHostAndServices() resets both, so this must be called +// after any Clear before otSrpClientAddService() will succeed. +void setupSrpHost(otInstance *ot) +{ + const otExtAddress *ext = otLinkGetExtendedAddress(ot); + snprintf(s_srp.hostname, sizeof(s_srp.hostname), + "%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(ot, s_srp.hostname); + otSrpClientEnableAutoHostAddress(ot); +} + // Try to add the _matter._tcp SRP service. // Must be called from the CHIP/OpenThread task (e.g. inside ScheduleWork). // preferIndex: if not kUndefinedFabricIndex, use that specific fabric (e.g. @@ -304,12 +320,17 @@ class SrpFabricDelegate : public chip::FabricTable::Delegate // Clear any stale record registered under a different fabric's credentials. // Use Clear (not RemoveHostAndServices) — see srpServiceRemove() for why. + // NOTE: Clear resets the SRP client's host state (hostname pointer and + // auto-host-address mode) in addition to the service list, so we must + // call setupSrpHost() before trySrpServiceAdd() or AddService will fail. if (s_srp.added) { s_srp.added = false; otSrpClientClearHostAndServices(ot); ESP_LOGI(kTag, "SRP: cleared old record for fabric update"); } + setupSrpHost(ot); + // Re-register immediately using the specific newly committed fabric so // the OTBR has the correct -._matter._tcp entry before // the commissioner's DNS-SD resolution attempt. @@ -473,16 +494,12 @@ extern "C" void app_main() 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); + // Configure SRP host name (derived from the 802.15.4 extended address) + // and enable auto-host-address mode. The buffer lives in s_srp so the + // pointer remains valid for the lifetime of the SRP client session and + // setupSrpHost() can re-apply it after otSrpClientClearHostAndServices(). + setupSrpHost(instance); + ESP_LOGI(kTag, "SRP: hostname '%s', auto-address enabled", s_srp.hostname); // Register our FabricTable delegate so the SRP record is refreshed // immediately when addNoc commits a new fabric during commissioning.