From 4038dc0f001d0f761fc31278a9f1c1c7b967f96f Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Thu, 24 Mar 2022 07:44:33 -0700 Subject: [PATCH 1/5] fix PH, allow for longer extended discovery timeout --- src/app/server/Dnssd.cpp | 16 ++++++++-------- src/app/server/Dnssd.h | 10 +++++----- src/include/platform/CHIPDeviceConfig.h | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 59cefa261346da..049890aa59f950 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -84,15 +84,15 @@ bool DnssdServer::HaveOperationalCredentials() #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY -void DnssdServer::SetExtendedDiscoveryTimeoutSecs(int16_t secs) +void DnssdServer::SetExtendedDiscoveryTimeoutSecs(int32_t secs) { ChipLogDetail(Discovery, "Setting extended discovery timeout to %" PRId16 "s", secs); chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(DefaultStorageKeyAllocator::DNSExtendedDiscoveryTimeout(), secs); } -int16_t DnssdServer::GetExtendedDiscoveryTimeoutSecs() +int32_t DnssdServer::GetExtendedDiscoveryTimeoutSecs() { - int16_t secs; + int32_t secs; CHIP_ERROR err = chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Get( DefaultStorageKeyAllocator::DNSExtendedDiscoveryTimeout(), &secs); @@ -199,7 +199,7 @@ void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAp ChipLogDetail(Discovery, "OnDiscoveryExpiration callback for valid session"); #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY - int16_t extTimeout = GetExtendedDiscoveryTimeoutSecs(); + int32_t extTimeout = GetExtendedDiscoveryTimeoutSecs(); if (extTimeout != CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED) { CHIP_ERROR err = AdvertiseCommissionableNode(chip::Dnssd::CommissioningMode::kDisabled); @@ -232,16 +232,16 @@ CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration() #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration() { - int16_t extendedDiscoveryTimeoutSecs = GetExtendedDiscoveryTimeoutSecs(); + int32_t extendedDiscoveryTimeoutSecs = GetExtendedDiscoveryTimeoutSecs(); if (extendedDiscoveryTimeoutSecs == CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT) { return CHIP_NO_ERROR; } ChipLogDetail(Discovery, "Scheduling extended discovery timeout in %" PRId16 "s", extendedDiscoveryTimeoutSecs); - mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(extendedDiscoveryTimeoutSecs); + mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds32(extendedDiscoveryTimeoutSecs); - return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(extendedDiscoveryTimeoutSecs), + return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(extendedDiscoveryTimeoutSecs), HandleExtendedDiscoveryExpiration, nullptr); } #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY @@ -378,7 +378,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi advertiseParameters.SetMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional(INET_CONFIG_ENABLE_TCP_ENDPOINT)); - if (mode != chip::Dnssd::CommissioningMode::kEnabledEnhanced) + if (!HaveOperationalCredentials()) { if (DeviceLayer::ConfigurationMgr().GetInitialPairingHint(value) != CHIP_NO_ERROR) { diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index efc7b86ea5ce08..d9ad0eafcb8f06 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -60,10 +60,10 @@ class DLL_EXPORT DnssdServer Inet::InterfaceId GetInterfaceId() { return mInterfaceId; } /// Sets the factory-new state commissionable node discovery timeout - void SetDiscoveryTimeoutSecs(int16_t secs) { mDiscoveryTimeoutSecs = secs; } + void SetDiscoveryTimeoutSecs(int32_t secs) { mDiscoveryTimeoutSecs = secs; } /// Gets the factory-new state commissionable node discovery timeout - int16_t GetDiscoveryTimeoutSecs() const { return mDiscoveryTimeoutSecs; } + int32_t GetDiscoveryTimeoutSecs() const { return mDiscoveryTimeoutSecs; } // // Override the referenced fabric table from the default that is present @@ -88,7 +88,7 @@ class DLL_EXPORT DnssdServer #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY /// Sets the extended discovery timeout. Value will be persisted across reboots - void SetExtendedDiscoveryTimeoutSecs(int16_t secs); + void SetExtendedDiscoveryTimeoutSecs(int32_t secs); /// Callback from Extended Discovery Expiration timer /// Checks if extended discovery has expired and if so, @@ -165,7 +165,7 @@ class DLL_EXPORT DnssdServer /// schedule next discovery expiration CHIP_ERROR ScheduleDiscoveryExpiration(); - int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; + int32_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; System::Clock::Timestamp mDiscoveryExpiration = kTimeoutCleared; /// return true if expirationMs is valid (not cleared and not in the future) @@ -173,7 +173,7 @@ class DLL_EXPORT DnssdServer #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY /// get the current extended discovery timeout (from persistent storage) - int16_t GetExtendedDiscoveryTimeoutSecs(); + int32_t GetExtendedDiscoveryTimeoutSecs(); /// schedule next extended discovery expiration CHIP_ERROR ScheduleExtendedDiscoveryExpiration(); diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index 7dd950f32a825a..2f5322950c1883 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/src/include/platform/CHIPDeviceConfig.h @@ -1237,7 +1237,7 @@ */ #define CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED 0 #define CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT -1 -#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (15 * 60) +#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (24 * 60 * 60) /** * CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DEVICE_TYPE From 9c6a35aaa3e4dc6f2012bed53ff30216c9f8a0b2 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Thu, 24 Mar 2022 08:45:53 -0700 Subject: [PATCH 2/5] ci fix --- src/app/server/Dnssd.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 049890aa59f950..e49255050ce448 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -86,7 +86,7 @@ bool DnssdServer::HaveOperationalCredentials() void DnssdServer::SetExtendedDiscoveryTimeoutSecs(int32_t secs) { - ChipLogDetail(Discovery, "Setting extended discovery timeout to %" PRId16 "s", secs); + ChipLogDetail(Discovery, "Setting extended discovery timeout to %" PRId32 "s", secs); chip::DeviceLayer::PersistedStorage::KeyValueStoreMgr().Put(DefaultStorageKeyAllocator::DNSExtendedDiscoveryTimeout(), secs); } @@ -237,7 +237,7 @@ CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration() { return CHIP_NO_ERROR; } - ChipLogDetail(Discovery, "Scheduling extended discovery timeout in %" PRId16 "s", extendedDiscoveryTimeoutSecs); + ChipLogDetail(Discovery, "Scheduling extended discovery timeout in %" PRId32 "s", extendedDiscoveryTimeoutSecs); mExtendedDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds32(extendedDiscoveryTimeoutSecs); From 7bec8a16daab9ecb307cca475c3994ff6c1578f5 Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Thu, 24 Mar 2022 14:45:37 -0700 Subject: [PATCH 3/5] address comments --- .../tv-app/tv-common/include/CHIPProjectAppConfig.h | 10 +++++++++- src/include/platform/CHIPDeviceConfig.h | 6 ++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h b/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h index e660c0747b93f3..e1e7c4d913d559 100644 --- a/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h +++ b/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h @@ -33,19 +33,27 @@ // TVs need to be both commissioners and commissionees #define CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE 1 +// TVs that are not commissionees, +// or that don't automatically enter commissioning mode should set this to 0 +#define CHIP_DEVICE_CONFIG_ENABLE_PAIRING_AUTOSTART 1 + // TVs do not typically need this - enable for debugging // #define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT 1 +// Enable extended discovery, set timeout to 24 hours #define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1 +#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (24 * 60 * 60) +// Advertise TV device type in DNS-SD #define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DEVICE_TYPE 1 #define CHIP_DEVICE_CONFIG_DEVICE_TYPE 35 // 0x0023 = 35 = Video Player +// Include device name in discovery for casting use case #define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DEVICE_NAME 1 - #define CHIP_DEVICE_CONFIG_DEVICE_NAME "Test TV" +// Enable app platform #define CHIP_DEVICE_CONFIG_APP_PLATFORM_ENABLED 1 // overrides CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT in CHIPProjectConfig diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index 2f5322950c1883..53405932479738 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/src/include/platform/CHIPDeviceConfig.h @@ -1233,11 +1233,13 @@ * Default time in seconds that a device will advertise commissionable node discovery * after commissioning mode ends. This value can be overridden by the user. * - * Only valid when CCHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY==1 + * Only valid when CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY==1 */ #define CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED 0 #define CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT -1 -#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (24 * 60 * 60) +#ifndef CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS +#define CHIP_DEVICE_CONFIG_EXTENDED_DISCOVERY_TIMEOUT_SECS (15 * 60) +#endif /** * CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DEVICE_TYPE From fc6701abf5df16b4a28339b902a7c118d96d34bd Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Fri, 25 Mar 2022 09:48:19 -0700 Subject: [PATCH 4/5] fix v6-only tv-app casting --- .../UserDirectedCommissioningServer.cpp | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp b/src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp index b5b7d43a8a507c..cf46a3c6e8afc4 100644 --- a/src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp +++ b/src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp @@ -111,12 +111,12 @@ void UserDirectedCommissioningServer::OnCommissionableNodeFound(const Dnssd::Dis { if (nodeData.numIPs == 0) { - ChipLogError(AppServer, "SetUDCClientProcessingState no IP addresses returned for instance name=%s", nodeData.instanceName); + ChipLogError(AppServer, "OnCommissionableNodeFound no IP addresses returned for instance name=%s", nodeData.instanceName); return; } if (nodeData.port == 0) { - ChipLogError(AppServer, "SetUDCClientProcessingState no port returned for instance name=%s", nodeData.instanceName); + ChipLogError(AppServer, "OnCommissionableNodeFound no port returned for instance name=%s", nodeData.instanceName); return; } @@ -127,7 +127,8 @@ void UserDirectedCommissioningServer::OnCommissionableNodeFound(const Dnssd::Dis (int) client->GetUDCClientProcessingState(), (int) UDCClientProcessingState::kPromptingUser); client->SetUDCClientProcessingState(UDCClientProcessingState::kPromptingUser); - // currently IPv6 addresses do not work for some reason +#if INET_CONFIG_ENABLE_IPV4 + // prefer IPv4 if its an option bool foundV4 = false; for (unsigned i = 0; i < nodeData.numIPs; ++i) { @@ -143,6 +144,25 @@ void UserDirectedCommissioningServer::OnCommissionableNodeFound(const Dnssd::Dis { client->SetPeerAddress(chip::Transport::PeerAddress::UDP(nodeData.ipAddress[0], nodeData.port)); } +#else // INET_CONFIG_ENABLE_IPV4 + // if we only support V6, then try to find a v6 address + bool foundV6 = false; + for (unsigned i = 0; i < nodeData.numIPs; ++i) + { + if (nodeData.ipAddress[i].IsIPv6()) + { + foundV6 = true; + client->SetPeerAddress(chip::Transport::PeerAddress::UDP(nodeData.ipAddress[i], nodeData.port)); + break; + } + } + // last resort, try with what we have + if (!foundV6) + { + ChipLogError(AppServer, "OnCommissionableNodeFound no v6 returned for instance name=%s", nodeData.instanceName); + client->SetPeerAddress(chip::Transport::PeerAddress::UDP(nodeData.ipAddress[0], nodeData.port)); + } +#endif // INET_CONFIG_ENABLE_IPV4 client->SetDeviceName(nodeData.deviceName); client->SetLongDiscriminator(nodeData.longDiscriminator); From 11dd3912888c43a6bcd7ccae568bd0a544ac353e Mon Sep 17 00:00:00 2001 From: chrisdecenzo Date: Fri, 25 Mar 2022 10:22:36 -0700 Subject: [PATCH 5/5] fix CI --- src/app/server/Dnssd.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index d9ad0eafcb8f06..5724d84a2f3dc1 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -60,10 +60,10 @@ class DLL_EXPORT DnssdServer Inet::InterfaceId GetInterfaceId() { return mInterfaceId; } /// Sets the factory-new state commissionable node discovery timeout - void SetDiscoveryTimeoutSecs(int32_t secs) { mDiscoveryTimeoutSecs = secs; } + void SetDiscoveryTimeoutSecs(int16_t secs) { mDiscoveryTimeoutSecs = secs; } /// Gets the factory-new state commissionable node discovery timeout - int32_t GetDiscoveryTimeoutSecs() const { return mDiscoveryTimeoutSecs; } + int16_t GetDiscoveryTimeoutSecs() const { return mDiscoveryTimeoutSecs; } // // Override the referenced fabric table from the default that is present @@ -165,7 +165,7 @@ class DLL_EXPORT DnssdServer /// schedule next discovery expiration CHIP_ERROR ScheduleDiscoveryExpiration(); - int32_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; + int16_t mDiscoveryTimeoutSecs = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS; System::Clock::Timestamp mDiscoveryExpiration = kTimeoutCleared; /// return true if expirationMs is valid (not cleared and not in the future)