Skip to content

Commit

Permalink
[dns-sd] Set default extended discovery timeout to 15 minutes
Browse files Browse the repository at this point in the history
The spec recommends that the extended discovery by default
time out after a period recommended in the "Announcement
duration" section. Change the default from "no timeout" to
15 minutes.

Use DefaultStorageKeyAllocator when persisting the timeout.

Make extended discovery logs more meaningful for a
non-implementer and remove some irrelevant messages.
  • Loading branch information
Damian-Nordic committed Mar 16, 2022
1 parent 20d18cb commit 38734fa
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 15 deletions.
30 changes: 17 additions & 13 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <lib/core/Optional.h>
#include <lib/dnssd/Advertiser.h>
#include <lib/dnssd/ServiceNaming.h>
#include <lib/support/DefaultStorageKeyAllocator.h>
#include <lib/support/Span.h>
#include <lib/support/logging/CHIPLogging.h>
#include <messaging/ReliableMessageProtocolConfig.h>
Expand Down Expand Up @@ -87,24 +88,27 @@ constexpr const char kExtendedDiscoveryTimeoutKeypairStorage[] = "ExtDiscKey";

void DnssdServer::SetExtendedDiscoveryTimeoutSecs(int16_t secs)
{
ChipLogDetail(Discovery, "SetExtendedDiscoveryTimeoutSecs %d", secs);
chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(kExtendedDiscoveryTimeoutKeypairStorage, &secs, sizeof(secs));
ChipLogDetail(Discovery, "Setting extended discovery timeout to %" PRId16 "s", secs);
chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(DefaultStorageKeyAllocator::DNSExtendedDiscoveryTimeout(), secs);
}

int16_t DnssdServer::GetExtendedDiscoveryTimeoutSecs()
{
CHIP_ERROR err = CHIP_NO_ERROR;
int16_t secs;
CHIP_ERROR err = chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(
DefaultStorageKeyAllocator::DNSExtendedDiscoveryTimeout(), &secs);

err = chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get(kExtendedDiscoveryTimeoutKeypairStorage, &secs, sizeof(secs));
if (err != CHIP_NO_ERROR)
if (err == CHIP_NO_ERROR)
{
return secs;
}

if (err != CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
{
ChipLogError(Discovery, "Failed to get extended timeout configuration err: %s", chip::ErrorStr(err));
secs = CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS;
ChipLogError(Discovery, "Failed to load extended discovery timeout: %" CHIP_ERROR_FORMAT, err.Format());
}

ChipLogDetail(Discovery, "GetExtendedDiscoveryTimeoutSecs %d", secs);
return secs;
return CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS;
}

/// Callback from Extended Discovery Expiration timer
Expand All @@ -117,11 +121,11 @@ void DnssdServer::OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, vo
{
if (!DnssdServer::OnExpiration(mExtendedDiscoveryExpiration))
{
ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for cleared session");
ChipLogDetail(Discovery, "Extended discovery timeout cancelled");
return;
}

ChipLogDetail(Discovery, "OnExtendedDiscoveryExpiration callback for valid session");
ChipLogDetail(Discovery, "Extended discovery timed out");

mExtendedDiscoveryExpiration = kTimeoutCleared;
}
Expand Down Expand Up @@ -219,7 +223,7 @@ CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration()
{
return CHIP_NO_ERROR;
}
ChipLogDetail(Discovery, "Scheduling Discovery timeout in secs=%d", mDiscoveryTimeoutSecs);
ChipLogDetail(Discovery, "Scheduling discovery timeout in %" PRId16 "s", mDiscoveryTimeoutSecs);

mDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(mDiscoveryTimeoutSecs);

Expand All @@ -235,7 +239,7 @@ CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration()
{
return CHIP_NO_ERROR;
}
ChipLogDetail(Discovery, "Scheduling Extended Discovery timeout in secs=%d", extendedDiscoveryTimeoutSecs);
ChipLogDetail(Discovery, "Scheduling extended discovery timeout in %" PRId16 "s", extendedDiscoveryTimeoutSecs);

mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(extendedDiscoveryTimeoutSecs);

Expand Down
4 changes: 2 additions & 2 deletions src/include/platform/CHIPDeviceConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@
* Only valid when CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DISCOVERY==1
*/
#ifndef CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS
#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS 15 * 60
#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS (15 * 60)
#endif

/**
Expand Down Expand Up @@ -1256,7 +1256,7 @@
*/
#define CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED 0
#define CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT -1
#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT
#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (15 * 60)

/**
* CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DEVICE_TYPE
Expand Down
2 changes: 2 additions & 0 deletions src/lib/support/DefaultStorageKeyAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class DefaultStorageKeyAllocator
static const char * OTACurrentProvider() { return "o/cp"; }
static const char * OTAUpdateToken() { return "o/ut"; }

static const char * DNSExtendedDiscoveryTimeout() { return "dns/edt"; }

private:
// The ENFORCE_FORMAT args are "off by one" because this is a class method,
// with an implicit "this" as first arg.
Expand Down

0 comments on commit 38734fa

Please sign in to comment.