Address DNS-SD issues (#16625)

* fix PH, allow for longer extended discovery timeout

* ci fix

* address comments

* fix v6-only tv-app casting

* fix CI

Author: chrisdecenzo

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

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);
+    ChipLogDetail(Discovery, "Setting extended discovery timeout to %" PRId32 "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);
+    ChipLogDetail(Discovery, "Scheduling extended discovery timeout in %" PRId32 "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<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));
 
-    if (mode != chip::Dnssd::CommissioningMode::kEnabledEnhanced)
+    if (!HaveOperationalCredentials())
     {
         if (DeviceLayer::ConfigurationMgr().GetInitialPairingHint(value) != CHIP_NO_ERROR)
         {

src/app/server/Dnssd.h

@@ -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,
@@ -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();

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
+#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

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);