Skip to content

Commit

Permalink
DNS-SD: Remove calls to set T flag (#32083)
Browse files Browse the repository at this point in the history
* DNS-SD: Remove calls to set T flag

Nothing currently supports TCP, therefore nothing should be setting
this flag. Per the new spec text, it is now forbidden to have
the lowest bit marked. This will bring the SDK examples into compliance
with 1.3.

Note that this PR does not fix the advertising parameters API. We
will need a new API for this flag when TCP support lands. Leaving
this for a follow up PR as changing the API should happen separately
from bringing the examples into conformance as it may affect the
platforms, who have implementations outside of the SDK.

Test: tested with all-clusters and avahi. Also fixed tests to
      omit this flag and ensure it does not appear in the TXT
      record.
      Automated cert test for operational records is in progress
      and requires this PR to pass.

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
cecille and restyled-commits authored Feb 13, 2024
1 parent 63c614d commit 10d1409
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 13 deletions.
3 changes: 1 addition & 2 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
.SetPort(GetSecuredPort())
.SetInterfaceId(GetInterfaceId())
.SetLocalMRPConfig(GetLocalMRPConfig())
.SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT))
.EnableIpV4(true);

#if CHIP_CONFIG_ENABLE_ICD_SERVER
Expand Down Expand Up @@ -255,7 +254,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetDeviceName(chip::Optional<const char *>::Value(deviceName));
}

advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));
advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig());

#if CHIP_CONFIG_ENABLE_ICD_SERVER
AddICDKeyToAdvertisement(advertiseParameters);
Expand Down
2 changes: 2 additions & 0 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class BaseAdvertisingParams
return *reinterpret_cast<Derived *>(this);
}
const Optional<ReliableMessageProtocolConfig> & GetLocalMRPConfig() const { return mLocalMRPConfig; }

// NOTE: The SetTcpSupported API is deprecated and not compliant with 1.3. T flag should not be set.
Derived & SetTcpSupported(Optional<bool> tcpSupported)
{
mTcpSupported = tcpSupported;
Expand Down
10 changes: 3 additions & 7 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ OperationalAdvertisingParameters operationalParams1 =
.SetMac(ByteSpan(kMac))
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetTcpSupported(chip::Optional<bool>(false))
.SetLocalMRPConfig(chip::Optional<ReliableMessageProtocolConfig>::Value(
32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms
OperationalAdvertisingParameters operationalParams2 =
Expand All @@ -93,7 +92,7 @@ OperationalAdvertisingParameters operationalParams5 =
OperationalAdvertisingParameters().SetPeerId(kPeerId5).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
OperationalAdvertisingParameters operationalParams6 =
OperationalAdvertisingParameters().SetPeerId(kPeerId6).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000", "T=0" };
const QNamePart txtOperational1Parts[] = { "SII=32", "SAI=30", "SAT=4000" };
PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName);
PtrResourceRecord ptrOperational1 = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1);
SrvResourceRecord srvOperational1 = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT);
Expand Down Expand Up @@ -181,14 +180,12 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
.SetPairingInstruction(chip::Optional<const char *>("Pair me"))
.SetProductId(chip::Optional<uint16_t>(897))
.SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"))
.SetTcpSupported(chip::Optional<bool>(true))
.SetICDOperatingAsLIT(chip::Optional<bool>(false))
// 3600005 is more than the max so should be adjusted down
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32, 65535_ms16));
QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000",
"DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3",
"SAI=3600000", "SII=3600000", "SAT=65535", "T=1",
"ICD=0" };
"SAI=3600000", "SII=3600000", "SAT=65535", "ICD=0" };
FullQName txtCommissionableNodeParamsLargeEnhancedName = FullQName(txtCommissionableNodeParamsLargeEnhancedParts);
TxtResourceRecord txtCommissionableNodeParamsLargeEnhanced =
TxtResourceRecord(instanceName, txtCommissionableNodeParamsLargeEnhancedName);
Expand All @@ -207,13 +204,12 @@ CommissionAdvertisingParameters commissionableNodeParamsEnhancedAsICDLIT =
.SetPairingHint(chip::Optional<uint16_t>(3))
.SetPairingInstruction(chip::Optional<const char *>("Pair me"))
.SetProductId(chip::Optional<uint16_t>(897))
.SetTcpSupported(chip::Optional<bool>(true))
.SetICDOperatingAsLIT(chip::Optional<bool>(true))
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600000_ms32, 65535_ms16));
// With ICD Operation as LIT, SII key will not be added to the advertisement
QNamePart txtCommissionableNodeParamsEnhancedAsICDLITParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000",
"DN=testy-test", "PI=Pair me", "PH=3", "SAI=3600000",
"SAT=65535", "T=1", "ICD=1" };
"SAT=65535", "ICD=1" };
FullQName txtCommissionableNodeParamsEnhancedAsICDLITName = FullQName(txtCommissionableNodeParamsEnhancedAsICDLITParts);
TxtResourceRecord txtCommissionableNodeParamsEnhancedAsICDLIT =
TxtResourceRecord(instanceName, txtCommissionableNodeParamsEnhancedAsICDLITName);
Expand Down
4 changes: 0 additions & 4 deletions src/lib/dnssd/platform/tests/TestPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ OperationalAdvertisingParameters operationalParams2 =
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below
.SetTcpSupported(Optional<bool>(true))
.SetICDOperatingAsLIT(Optional<bool>(false));
test::ExpectedCall operationalCall2 = test::ExpectedCall()
.SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp)
Expand All @@ -64,7 +63,6 @@ test::ExpectedCall operationalCall2 = test::ExpectedCall()
.AddTxt("SII", "32")
.AddTxt("SAI", "30")
.AddTxt("SAT", "10")
.AddTxt("T", "1")
.AddTxt("ICD", "0");

CommissionAdvertisingParameters commissionableNodeParamsSmall =
Expand Down Expand Up @@ -97,7 +95,6 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic =
.SetPairingInstruction(Optional<const char *>("Pair me"))
.SetProductId(Optional<uint16_t>(897))
.SetRotatingDeviceId(Optional<const char *>("id_that_spins"))
.SetTcpSupported(Optional<bool>(true))
.SetICDOperatingAsLIT(Optional<bool>(false))
// 3600005 is over the max, so this should be adjusted by the platform
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32, 65535_ms16));
Expand All @@ -114,7 +111,6 @@ test::ExpectedCall commissionableLargeBasic = test::ExpectedCall()
.AddTxt("RI", "id_that_spins")
.AddTxt("PI", "Pair me")
.AddTxt("PH", "3")
.AddTxt("T", "1")
.AddTxt("ICD", "0")
.AddTxt("SII", "3600000")
.AddTxt("SAI", "3600000")
Expand Down

0 comments on commit 10d1409

Please sign in to comment.