Skip to content

app_main: fix commissioning failure — SRP record not updated after ad… #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
GuvHas wants to merge 2 commits into from
Closed

app_main: fix commissioning failure — SRP record not updated after ad… #44

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22a77f4
app_main: fix commissioning failure — SRP record not updated after ad…
claude Mar 20, 2026
bacda27
app_main: re-apply SRP host config after Clear in OnFabricCommitted
claude Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 87 additions & 20 deletions main/app_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,27 +167,52 @@ 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).
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");
Expand Down Expand Up @@ -273,6 +298,48 @@ 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 (<old-CFID>-<old-NodeId>._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.
// 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);

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reapply SRP host config after clearing host/services

In SrpFabricDelegate::OnFabricCommitted, calling otSrpClientClearHostAndServices() drops the SRP host metadata along with services, but the follow-up path only calls trySrpServiceAdd() (which just adds a service). That means recommissioning cases where s_srp.added is already true can lose the hostname/host-address state set at startup and fail to publish the new operational _matter._tcp instance, so DNS-SD/CASE reconnect can still time out.

Useful? React with 👍 / 👎.

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 <CFID>-<NodeId>._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.
Expand Down Expand Up @@ -427,16 +494,16 @@ 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.
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
Expand Down