From 9f704cfe9505c5aee0cd3fbae6ed28b5cacb0bd6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:19:11 -0400 Subject: [PATCH 01/30] std optional in recordwriter --- src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp | 10 +++++----- src/lib/dnssd/minimal_mdns/core/RecordWriter.h | 9 +++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp b/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp index 41e9985687ad81..772f4d86da5cac 100644 --- a/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp +++ b/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp @@ -50,12 +50,12 @@ RecordWriter & RecordWriter::WriteQName(const FullQName & qname) remaining.nameCount = qname.nameCount - i; // Try to find a valid offset - chip::Optional offset = FindPreviousName(remaining); + std::optional offset = FindPreviousName(remaining); - if (offset.HasValue()) + if (offset.has_value()) { // Pointer to offset: set the highest 2 bits - mOutput->Put16(offset.Value() | 0xC000); + mOutput->Put16(*offset | 0xC000); if (mOutput->Fit() && !isFullyCompressed) { @@ -85,13 +85,13 @@ RecordWriter & RecordWriter::WriteQName(const SerializedQNameIterator & qname) SerializedQNameIterator copy = qname; while (true) { - chip::Optional offset = FindPreviousName(copy); + std::optional offset = FindPreviousName(copy); if (offset.HasValue()) { // Pointer to offset: set the highest 2 bits // We guarantee that offsets saved are <= kMaxReuseOffset - mOutput->Put16(offset.Value() | 0xC000); + mOutput->Put16(*offset | 0xC000); if (mOutput->Fit() && !isFullyCompressed) { diff --git a/src/lib/dnssd/minimal_mdns/core/RecordWriter.h b/src/lib/dnssd/minimal_mdns/core/RecordWriter.h index 35e563fe870ec2..0bb80fce7cffd7 100644 --- a/src/lib/dnssd/minimal_mdns/core/RecordWriter.h +++ b/src/lib/dnssd/minimal_mdns/core/RecordWriter.h @@ -17,10 +17,11 @@ #pragma once #include -#include #include #include +#include + namespace mdns { namespace Minimal { @@ -99,7 +100,7 @@ class RecordWriter /// Find the offset at which this qname was previously seen (if any) /// works with QName and SerializedQNameIterator template - chip::Optional FindPreviousName(const T & name) const + std::optional FindPreviousName(const T & name) const { for (size_t i = 0; i < kMaxCachedReferences; i++) { @@ -110,7 +111,7 @@ class RecordWriter { if (previous == name) { - return chip::MakeOptional(static_cast(previous.OffsetInCurrentValidData())); + return std::make_optional(static_cast(previous.OffsetInCurrentValidData())); } if (!previous.Next()) @@ -120,7 +121,7 @@ class RecordWriter } } - return chip::Optional::Missing(); + return std::nullopt; } /// Gets the iterator corresponding to the previous name From 2793df98f39924df9237d22c20cc087cef6e6259 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:20:42 -0400 Subject: [PATCH 02/30] Fix a call --- src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp b/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp index 772f4d86da5cac..693fc9418e91a7 100644 --- a/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp +++ b/src/lib/dnssd/minimal_mdns/core/RecordWriter.cpp @@ -87,7 +87,7 @@ RecordWriter & RecordWriter::WriteQName(const SerializedQNameIterator & qname) { std::optional offset = FindPreviousName(copy); - if (offset.HasValue()) + if (offset.has_value()) { // Pointer to offset: set the highest 2 bits // We guarantee that offsets saved are <= kMaxReuseOffset From d5cb158980b9355e72f70ae5d182d812864d6f07 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:36:31 -0400 Subject: [PATCH 03/30] Change some advertiser.h to std::optional --- src/app/server/Dnssd.cpp | 26 +++---- src/lib/dnssd/Advertiser.h | 74 +++++++++---------- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 46 ++++++------ .../minimal_mdns/tests/TestAdvertiser.cpp | 32 ++++---- 4 files changed, 89 insertions(+), 89 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 3df796434e2671..5ae0859b0c499d 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -206,7 +206,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() .SetMac(mac) .SetPort(GetSecuredPort()) .SetInterfaceId(GetInterfaceId()) - .SetLocalMRPConfig(GetLocalMRPConfig()) + .SetLocalMRPConfig(GetLocalMRPConfig().std_optional()) .EnableIpV4(true); #if CHIP_CONFIG_ENABLE_ICD_SERVER @@ -251,7 +251,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetVendorId(chip::Optional::Value(value)); + advertiseParameters.SetVendorId(std::make_optional(value)); } if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(value) != CHIP_NO_ERROR) @@ -260,23 +260,23 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetProductId(chip::Optional::Value(value)); + advertiseParameters.SetProductId(std::make_optional(value)); } if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceTypeEnabled() && DeviceLayer::ConfigurationMgr().GetDeviceTypeId(val32) == CHIP_NO_ERROR) { - advertiseParameters.SetDeviceType(chip::Optional::Value(val32)); + advertiseParameters.SetDeviceType(std::make_optional(val32)); } char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 1]; if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled() && DeviceLayer::ConfigurationMgr().GetCommissionableDeviceName(deviceName, sizeof(deviceName)) == CHIP_NO_ERROR) { - advertiseParameters.SetDeviceName(chip::Optional::Value(deviceName)); + advertiseParameters.SetDeviceName(std::make_optional(deviceName)); } - advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()); + advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig().std_optional()); #if CHIP_CONFIG_ENABLE_ICD_SERVER AddICDKeyToAdvertisement(advertiseParameters); @@ -303,7 +303,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi #if CHIP_ENABLE_ROTATING_DEVICE_ID && defined(CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID) char rotatingDeviceIdHexBuffer[RotatingDeviceId::kHexMaxLength]; ReturnErrorOnFailure(GenerateRotatingDeviceId(rotatingDeviceIdHexBuffer, ArraySize(rotatingDeviceIdHexBuffer))); - advertiseParameters.SetRotatingDeviceId(chip::Optional::Value(rotatingDeviceIdHexBuffer)); + advertiseParameters.SetRotatingDeviceId(std::make_optional(rotatingDeviceIdHexBuffer)); #endif if (!HaveOperationalCredentials()) @@ -314,7 +314,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetPairingHint(chip::Optional::Value(value)); + advertiseParameters.SetPairingHint(std::make_optional(value)); } if (DeviceLayer::ConfigurationMgr().GetInitialPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR) @@ -323,7 +323,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetPairingInstruction(chip::Optional::Value(pairingInst)); + advertiseParameters.SetPairingInstruction(std::make_optional(pairingInst)); } } else @@ -334,7 +334,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetPairingHint(chip::Optional::Value(value)); + advertiseParameters.SetPairingHint(std::make_optional(value)); } if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR) @@ -343,7 +343,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi } else { - advertiseParameters.SetPairingInstruction(chip::Optional::Value(pairingInst)); + advertiseParameters.SetPairingInstruction(std::make_optional(pairingInst)); } } } @@ -357,10 +357,10 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance(); ChipLogProgress(Discovery, "Advertise commission parameter vendorID=%u productID=%u discriminator=%04u/%02u cm=%u cp=%u", - advertiseParameters.GetVendorId().ValueOr(0), advertiseParameters.GetProductId().ValueOr(0), + advertiseParameters.GetVendorId().value_or(0), advertiseParameters.GetProductId().value_or(0), advertiseParameters.GetLongDiscriminator(), advertiseParameters.GetShortDiscriminator(), to_underlying(advertiseParameters.GetCommissioningMode()), - advertiseParameters.GetCommissionerPasscodeSupported().ValueOr(false) ? 1 : 0); + advertiseParameters.GetCommissionerPasscodeSupported().value_or(false) ? 1 : 0); return mdnsAdvertiser.Advertise(advertiseParameters); } diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 64fa9ec3fd5d33..02888181f98181 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -19,9 +19,9 @@ #include #include +#include #include -#include #include #include #include @@ -95,20 +95,20 @@ class BaseAdvertisingParams const chip::ByteSpan GetMac() const { return chip::ByteSpan(mMacStorage, mMacLength); } // Common Flags - Derived & SetLocalMRPConfig(const Optional & config) + Derived & SetLocalMRPConfig(const std::optional & config) { mLocalMRPConfig = config; return *reinterpret_cast(this); } - const Optional & GetLocalMRPConfig() const { return mLocalMRPConfig; } + const std::optional & 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 tcpSupported) + Derived & SetTcpSupported(std::optional tcpSupported) { mTcpSupported = tcpSupported; return *reinterpret_cast(this); } - Optional GetTcpSupported() const { return mTcpSupported; } + std::optional GetTcpSupported() const { return mTcpSupported; } Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode) { @@ -123,8 +123,8 @@ class BaseAdvertisingParams bool mEnableIPv4 = true; uint8_t mMacStorage[kMaxMacSize] = {}; size_t mMacLength = 0; - Optional mLocalMRPConfig; - Optional mTcpSupported; + std::optional mLocalMRPConfig; + std::optional mTcpSupported; ICDModeAdvertise mICDModeAdvertise = ICDModeAdvertise::kNone; }; @@ -176,19 +176,19 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams vendorId) + CommissionAdvertisingParameters & SetVendorId(std::optional vendorId) { mVendorId = vendorId; return *this; } - Optional GetVendorId() const { return mVendorId; } + std::optional GetVendorId() const { return mVendorId; } - CommissionAdvertisingParameters & SetProductId(Optional productId) + CommissionAdvertisingParameters & SetProductId(std::optional productId) { mProductId = productId; return *this; } - Optional GetProductId() const { return mProductId; } + std::optional GetProductId() const { return mProductId; } CommissionAdvertisingParameters & SetCommissioningMode(CommissioningMode mode) { @@ -197,18 +197,18 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams deviceType) + CommissionAdvertisingParameters & SetDeviceType(std::optional deviceType) { mDeviceType = deviceType; return *this; } - Optional GetDeviceType() const { return mDeviceType; } + std::optional GetDeviceType() const { return mDeviceType; } - CommissionAdvertisingParameters & SetDeviceName(Optional deviceName) + CommissionAdvertisingParameters & SetDeviceName(std::optional deviceName) { - if (deviceName.HasValue()) + if (deviceName.has_value()) { - Platform::CopyString(mDeviceName, sizeof(mDeviceName), deviceName.Value()); + Platform::CopyString(mDeviceName, sizeof(mDeviceName), *deviceName); mDeviceNameHasValue = true; } else @@ -217,16 +217,16 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetDeviceName() const + std::optional GetDeviceName() const { - return mDeviceNameHasValue ? Optional::Value(mDeviceName) : Optional::Missing(); + return mDeviceNameHasValue ? std::make_optional(mDeviceName) : std::nullopt; } - CommissionAdvertisingParameters & SetRotatingDeviceId(Optional rotatingId) + CommissionAdvertisingParameters & SetRotatingDeviceId(std::optional rotatingId) { - if (rotatingId.HasValue()) + if (rotatingId.has_value()) { - Platform::CopyString(mRotatingId, sizeof(mRotatingId), rotatingId.Value()); + Platform::CopyString(mRotatingId, sizeof(mRotatingId), *rotatingId); mRotatingIdHasValue = true; } else @@ -235,16 +235,16 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetRotatingDeviceId() const + std::optional GetRotatingDeviceId() const { - return mRotatingIdHasValue ? Optional::Value(mRotatingId) : Optional::Missing(); + return mRotatingIdHasValue ? std::make_optional(mRotatingId) : std::nullopt; } - CommissionAdvertisingParameters & SetPairingInstruction(Optional pairingInstr) + CommissionAdvertisingParameters & SetPairingInstruction(std::optional pairingInstr) { - if (pairingInstr.HasValue()) + if (pairingInstr.has_value()) { - Platform::CopyString(mPairingInstr, sizeof(mPairingInstr), pairingInstr.Value()); + Platform::CopyString(mPairingInstr, sizeof(mPairingInstr), *pairingInstr); mPairingInstrHasValue = true; } else @@ -253,17 +253,17 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetPairingInstruction() const + std::optional GetPairingInstruction() const { - return mPairingInstrHasValue ? Optional::Value(mPairingInstr) : Optional::Missing(); + return mPairingInstrHasValue ? std::make_optional(mPairingInstr) : std::nullopt; } - CommissionAdvertisingParameters & SetPairingHint(Optional pairingHint) + CommissionAdvertisingParameters & SetPairingHint(std::optional pairingHint) { mPairingHint = pairingHint; return *this; } - Optional GetPairingHint() const { return mPairingHint; } + std::optional GetPairingHint() const { return mPairingHint; } CommissionAdvertisingParameters & SetCommissionAdvertiseMode(CommssionAdvertiseMode mode) { @@ -272,22 +272,22 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams commissionerPasscodeSupported) + CommissionAdvertisingParameters & SetCommissionerPasscodeSupported(std::optional commissionerPasscodeSupported) { mCommissionerPasscodeSupported = commissionerPasscodeSupported; return *this; } - Optional GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } + std::optional GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } private: uint8_t mShortDiscriminator = 0; uint16_t mLongDiscriminator = 0; // 12-bit according to spec CommssionAdvertiseMode mMode = CommssionAdvertiseMode::kCommissionableNode; CommissioningMode mCommissioningMode = CommissioningMode::kEnabledBasic; - chip::Optional mVendorId; - chip::Optional mProductId; - chip::Optional mDeviceType; - chip::Optional mPairingHint; + std::optional mVendorId; + std::optional mProductId; + std::optional mDeviceType; + std::optional mPairingHint; char mDeviceName[kKeyDeviceNameMaxLength + 1]; bool mDeviceNameHasValue = false; @@ -298,7 +298,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams mCommissionerPasscodeSupported; + std::optional mCommissionerPasscodeSupported; }; /** diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index e25487f781860f..ec139871b4a5cc 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -228,9 +228,9 @@ class AdvertiserMinMdns : public ServiceAdvertiser, { auto optionalMrp = params.GetLocalMRPConfig(); - if (optionalMrp.HasValue()) + if (optionalMrp.has_value()) { - auto mrp = optionalMrp.Value(); + auto mrp = *optionalMrp; // An ICD operating as a LIT shall not advertise its slow polling interval. // Don't include the SII key in the advertisement when operating as so. @@ -278,10 +278,10 @@ class AdvertiserMinMdns : public ServiceAdvertiser, txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf; } } - if (params.GetTcpSupported().HasValue()) + if (params.GetTcpSupported().has_value()) { size_t writtenCharactersNumber = static_cast( - snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", params.GetTcpSupported().Value())); + snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *params.GetTcpSupported())); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); txtFields[numTxtFields++] = storage.tcpSupportedBuf; @@ -670,10 +670,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } - if (params.GetVendorId().HasValue()) + if (params.GetVendorId().has_value()) { MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), - DiscoveryFilter(DiscoveryFilterType::kVendorId, params.GetVendorId().Value())); + DiscoveryFilter(DiscoveryFilterType::kVendorId, *params.GetVendorId())); FullQName vendorServiceName = allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); @@ -688,10 +688,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } - if (params.GetDeviceType().HasValue()) + if (params.GetDeviceType().has_value()) { MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), - DiscoveryFilter(DiscoveryFilterType::kDeviceType, params.GetDeviceType().Value())); + DiscoveryFilter(DiscoveryFilterType::kDeviceType, *params.GetDeviceType())); FullQName vendorServiceName = allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); @@ -812,28 +812,28 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis : &mQueryResponderAllocatorCommissioner; char txtVidPid[chip::Dnssd::kKeyVendorProductMaxLength + 4]; - if (params.GetProductId().HasValue() && params.GetVendorId().HasValue()) + if (params.GetProductId().has_value() && params.GetVendorId().has_value()) { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", params.GetVendorId().Value(), params.GetProductId().Value()); + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *params.GetVendorId(), *params.GetProductId()); txtFields[numTxtFields++] = txtVidPid; } - else if (params.GetVendorId().HasValue()) + else if (params.GetVendorId().has_value()) { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", params.GetVendorId().Value()); + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", *params.GetVendorId()); txtFields[numTxtFields++] = txtVidPid; } char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4]; - if (params.GetDeviceType().HasValue()) + if (params.GetDeviceType().has_value()) { - snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, params.GetDeviceType().Value()); + snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *params.GetDeviceType()); txtFields[numTxtFields++] = txtDeviceType; } char txtDeviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 4]; - if (params.GetDeviceName().HasValue()) + if (params.GetDeviceName().has_value()) { - snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", params.GetDeviceName().Value()); + snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *params.GetDeviceName()); txtFields[numTxtFields++] = txtDeviceName; } CommonTxtEntryStorage commonStorage; @@ -858,27 +858,27 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", static_cast(params.GetCommissioningMode())); txtFields[numTxtFields++] = txtCommissioningMode; - if (params.GetRotatingDeviceId().HasValue()) + if (params.GetRotatingDeviceId().has_value()) { - snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", params.GetRotatingDeviceId().Value()); + snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *params.GetRotatingDeviceId()); txtFields[numTxtFields++] = txtRotatingDeviceId; } - if (params.GetPairingHint().HasValue()) + if (params.GetPairingHint().has_value()) { - snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", params.GetPairingHint().Value()); + snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *params.GetPairingHint()); txtFields[numTxtFields++] = txtPairingHint; } - if (params.GetPairingInstruction().HasValue()) + if (params.GetPairingInstruction().has_value()) { - snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", params.GetPairingInstruction().Value()); + snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *params.GetPairingInstruction()); txtFields[numTxtFields++] = txtPairingInstr; } } else { - if (params.GetCommissionerPasscodeSupported().ValueOr(false)) + if (params.GetCommissionerPasscodeSupported().value_or(false)) { snprintf(txtCommissionerPasscode, sizeof(txtCommissionerPasscode), "CP=%d", static_cast(1)); txtFields[numTxtFields++] = txtCommissionerPasscode; diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp index b39fd5858858c1..523e38b0b17ddf 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp @@ -79,7 +79,7 @@ OperationalAdvertisingParameters operationalParams1 = .SetMac(ByteSpan(kMac)) .SetPort(CHIP_PORT) .EnableIpV4(true) - .SetLocalMRPConfig(chip::Optional::Value( + .SetLocalMRPConfig(std::make_optional( 32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); @@ -151,14 +151,14 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic = .SetMac(ByteSpan(kMac, sizeof(kMac))) .SetLongDiscriminator(22) .SetShortDiscriminator(2) - .SetVendorId(chip::Optional(555)) - .SetDeviceType(chip::Optional(70000)) + .SetVendorId(std::make_optional(555)) + .SetDeviceType(std::make_optional(70000)) .SetCommissioningMode(CommissioningMode::kEnabledBasic) - .SetDeviceName(chip::Optional("testy-test")) - .SetPairingHint(chip::Optional(3)) - .SetPairingInstruction(chip::Optional("Pair me")) - .SetProductId(chip::Optional(897)) - .SetRotatingDeviceId(chip::Optional("id_that_spins")); + .SetDeviceName(std::make_optional("testy-test")) + .SetPairingHint(std::make_optional(3)) + .SetPairingInstruction(std::make_optional("Pair me")) + .SetProductId(std::make_optional(897)) + .SetRotatingDeviceId(std::make_optional("id_that_spins")); QNamePart txtCommissionableNodeParamsLargeBasicParts[] = { "D=22", "VP=555+897", "CM=1", "DT=70000", "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3" }; FullQName txtCommissionableNodeParamsLargeBasicName = FullQName(txtCommissionableNodeParamsLargeBasicParts); @@ -171,17 +171,17 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced = .SetMac(ByteSpan(kMac, sizeof(kMac))) .SetLongDiscriminator(22) .SetShortDiscriminator(2) - .SetVendorId(chip::Optional(555)) - .SetDeviceType(chip::Optional(70000)) + .SetVendorId(std::make_optional(555)) + .SetDeviceType(std::make_optional(70000)) .SetCommissioningMode(CommissioningMode::kEnabledEnhanced) - .SetDeviceName(chip::Optional("testy-test")) - .SetPairingHint(chip::Optional(3)) - .SetPairingInstruction(chip::Optional("Pair me")) - .SetProductId(chip::Optional(897)) - .SetRotatingDeviceId(chip::Optional("id_that_spins")) + .SetDeviceName(std::make_optional("testy-test")) + .SetPairingHint(std::make_optional(3)) + .SetPairingInstruction(std::make_optional("Pair me")) + .SetProductId(std::make_optional(897)) + .SetRotatingDeviceId(std::make_optional("id_that_spins")) .SetICDModeToAdvertise(ICDModeAdvertise::kSIT) // 3600005 is more than the max so should be adjusted down - .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); + .SetLocalMRPConfig(std::make_optional(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", "ICD=0" }; From e8ef93264e5171efdcad23d4571df07c3e82f4db Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:39:18 -0400 Subject: [PATCH 04/30] Fix usage in advertiser.cpp --- examples/minimal-mdns/advertiser.cpp | 29 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/examples/minimal-mdns/advertiser.cpp b/examples/minimal-mdns/advertiser.cpp index 9d2f90c7858e00..6e260257301324 100644 --- a/examples/minimal-mdns/advertiser.cpp +++ b/examples/minimal-mdns/advertiser.cpp @@ -16,6 +16,7 @@ */ #include #include +#include #include #include @@ -45,18 +46,18 @@ struct Options AdvertisingMode advertisingMode = AdvertisingMode::kCommissionableNode; // commissionable node / commissioner params - Optional vendorId; - Optional productId; - Optional deviceType; - Optional deviceName; + std::optional vendorId; + std::optional productId; + std::optional deviceType; + std::optional deviceName; // commissionable node params uint8_t shortDiscriminator = 52; uint16_t longDiscriminator = 840; Dnssd::CommissioningMode commissioningMode = Dnssd::CommissioningMode::kDisabled; - Optional rotatingId; - Optional pairingInstr; - Optional pairingHint; + std::optional rotatingId; + std::optional pairingInstr; + std::optional pairingHint; // operational params uint64_t fabricId = 12345; @@ -130,10 +131,10 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, gOptions.longDiscriminator = static_cast(atoi(aValue)); return true; case kOptionCommissioningVendorId: - gOptions.vendorId = Optional::Value(static_cast(atoi(aValue))); + gOptions.vendorId = std::make_optional(static_cast(atoi(aValue))); return true; case kOptionCommissioningProductId: - gOptions.productId = Optional::Value(static_cast(atoi(aValue))); + gOptions.productId = std::make_optional(static_cast(atoi(aValue))); return true; case kOptionCommissioningMode: cm = static_cast(atoi(aValue)); @@ -147,19 +148,19 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, } return true; case kOptionCommissioningDeviceType: - gOptions.deviceType = Optional::Value(static_cast(atoi(aValue))); + gOptions.deviceType = std::make_optional(static_cast(atoi(aValue))); return true; case kOptionCommissioningDeviceName: - gOptions.deviceName = Optional::Value(static_cast(aValue)); + gOptions.deviceName = std::make_optional(static_cast(aValue)); return true; case kOptionCommissioningRotatingId: - gOptions.rotatingId = Optional::Value(static_cast(aValue)); + gOptions.rotatingId = std::make_optional(static_cast(aValue)); return true; case kOptionCommissioningPairingInstr: - gOptions.pairingInstr = Optional::Value(static_cast(aValue)); + gOptions.pairingInstr = std::make_optional(static_cast(aValue)); return true; case kOptionCommissioningPairingHint: - gOptions.pairingHint = Optional::Value(static_cast(atoi(aValue))); + gOptions.pairingHint = std::make_optional(static_cast(atoi(aValue))); return true; case kOptionOperationalFabricId: if (sscanf(aValue, "%" SCNx64, &gOptions.fabricId) != 1) From c84a658ec0ca6a0d49ee6e16a534fe09790e6189 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:47:37 -0400 Subject: [PATCH 05/30] Make more things compile --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 30 ++++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 2b6b6cd470c618..ba280edcc3f4f4 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -113,10 +113,10 @@ CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t template CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t & entriesCount, char * buffer, size_t bufferLen, - chip::Optional value) + std::optional value) { - VerifyOrReturnError(value.HasValue(), CHIP_NO_ERROR); - return AddPtrRecord(type, entries, entriesCount, buffer, bufferLen, value.Value()); + VerifyOrReturnError(value.has_value(), CHIP_NO_ERROR); + return AddPtrRecord(type, entries, entriesCount, buffer, bufferLen, *value); } CHIP_ERROR ENFORCE_FORMAT(4, 5) @@ -161,36 +161,36 @@ CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, CommissioningMod } template -CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, chip::Optional value) +CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, std::optional value) { - VerifyOrReturnError(value.HasValue(), CHIP_ERROR_UNINITIALIZED); - return CopyTextRecordValue(buffer, bufferLen, value.Value()); + VerifyOrReturnError(value.has_value(), CHIP_ERROR_UNINITIALIZED); + return CopyTextRecordValue(buffer, bufferLen, *value); } -CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, chip::Optional value1, chip::Optional value2) +CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, std::optional value1, std::optional value2) { - VerifyOrReturnError(value1.HasValue(), CHIP_ERROR_UNINITIALIZED); - return value2.HasValue() ? CopyTextRecordValue(buffer, bufferLen, value1.Value(), value2.Value()) - : CopyTextRecordValue(buffer, bufferLen, value1.Value()); + VerifyOrReturnError(value1.has_value(), CHIP_ERROR_UNINITIALIZED); + return value2.has_value() ? CopyTextRecordValue(buffer, bufferLen, *value1, *value2) + : CopyTextRecordValue(buffer, bufferLen, *value1); } -CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, const chip::Optional optional, +CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, const std::optional optional, TxtFieldKey key) { VerifyOrReturnError((key == TxtFieldKey::kSessionIdleInterval || key == TxtFieldKey::kSessionActiveInterval || key == TxtFieldKey::kSessionActiveThreshold), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(optional.HasValue(), CHIP_ERROR_UNINITIALIZED); + VerifyOrReturnError(optional.has_value(), CHIP_ERROR_UNINITIALIZED); CHIP_ERROR err; if (key == TxtFieldKey::kSessionActiveThreshold) { - err = CopyTextRecordValue(buffer, bufferLen, optional.Value().mActiveThresholdTime.count()); + err = CopyTextRecordValue(buffer, bufferLen, optional->mActiveThresholdTime.count()); } else { bool isIdle = (key == TxtFieldKey::kSessionIdleInterval); - auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout; + auto retryInterval = isIdle ? optional->mIdleRetransTimeout : optional->mActiveRetransTimeout; if (retryInterval > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available", @@ -254,7 +254,7 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const return CopyTextRecordValue(buffer, bufferLen, params.GetCommissioningMode()); case TxtFieldKey::kCommissionerPasscode: return CopyTextRecordValue(buffer, bufferLen, - static_cast(params.GetCommissionerPasscodeSupported().ValueOr(false) ? 1 : 0)); + static_cast(params.GetCommissionerPasscodeSupported().value_or(false) ? 1 : 0)); default: return CopyTxtRecord(key, buffer, bufferLen, static_cast>(params)); } From b2bfac4d81a69cef822346e6fc5450ce2f8eead6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 12:48:01 -0400 Subject: [PATCH 06/30] Restyle --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index ec139871b4a5cc..cc7e5f6c2c04bf 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -672,8 +672,7 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & if (params.GetVendorId().has_value()) { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), - DiscoveryFilter(DiscoveryFilterType::kVendorId, *params.GetVendorId())); + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *params.GetVendorId())); FullQName vendorServiceName = allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); From d855d4d9c443149604e88ed63e1551868ff60e0b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:06:15 -0400 Subject: [PATCH 07/30] make clang-tidy happy --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 57 ++++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index cc7e5f6c2c04bf..62722f58421fc7 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -226,7 +226,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser, CHIP_ERROR AddCommonTxtEntries(const BaseAdvertisingParams & params, CommonTxtEntryStorage & storage, char ** txtFields, size_t & numTxtFields) { - auto optionalMrp = params.GetLocalMRPConfig(); + const auto optionalMrp = params.GetLocalMRPConfig(); if (optionalMrp.has_value()) { @@ -278,10 +278,11 @@ class AdvertiserMinMdns : public ServiceAdvertiser, txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf; } } - if (params.GetTcpSupported().has_value()) + const auto tcpSupported = params.GetTcpSupported(); + if (tcpSupported.has_value()) { - size_t writtenCharactersNumber = static_cast( - snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *params.GetTcpSupported())); + size_t writtenCharactersNumber = + static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); txtFields[numTxtFields++] = storage.tcpSupportedBuf; @@ -670,9 +671,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } - if (params.GetVendorId().has_value()) + const auto vendorId = params.GetVendorId(); + if (vendorId.has_value()) { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *params.GetVendorId())); + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); FullQName vendorServiceName = allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); @@ -687,10 +689,10 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } - if (params.GetDeviceType().has_value()) + const auto deviceType = params.GetDeviceType(); + if (deviceType.has_value()) { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), - DiscoveryFilter(DiscoveryFilterType::kDeviceType, *params.GetDeviceType())); + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); FullQName vendorServiceName = allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); @@ -811,28 +813,34 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis : &mQueryResponderAllocatorCommissioner; char txtVidPid[chip::Dnssd::kKeyVendorProductMaxLength + 4]; - if (params.GetProductId().has_value() && params.GetVendorId().has_value()) + + const auto productId = params.GetProductId(); + const auto vendorId = params.GetVendorId(); + + if (productId.has_value() && vendorId.has_value()) { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *params.GetVendorId(), *params.GetProductId()); + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *vendorId, *productId); txtFields[numTxtFields++] = txtVidPid; } - else if (params.GetVendorId().has_value()) + else if (vendorId.has_value()) { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", *params.GetVendorId()); + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", *vendorId); txtFields[numTxtFields++] = txtVidPid; } char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4]; - if (params.GetDeviceType().has_value()) + const auto deviceType = params.GetDeviceType(); + if (deviceType.has_value()) { - snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *params.GetDeviceType()); + snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); txtFields[numTxtFields++] = txtDeviceType; } char txtDeviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 4]; - if (params.GetDeviceName().has_value()) + const auto deviceName = params.GetDeviceName(); + if (deviceName.has_value()) { - snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *params.GetDeviceName()); + snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *deviceName); txtFields[numTxtFields++] = txtDeviceName; } CommonTxtEntryStorage commonStorage; @@ -857,21 +865,24 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", static_cast(params.GetCommissioningMode())); txtFields[numTxtFields++] = txtCommissioningMode; - if (params.GetRotatingDeviceId().has_value()) + const auto rotatingDeviceId = params.GetRotatingDeviceId(); + if (rotatingDeviceId.has_value()) { - snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *params.GetRotatingDeviceId()); + snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); txtFields[numTxtFields++] = txtRotatingDeviceId; } - if (params.GetPairingHint().has_value()) + const auto pairingHint = params.GetPairingHint(); + if (pairingHint.has_value()) { - snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *params.GetPairingHint()); + snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); txtFields[numTxtFields++] = txtPairingHint; } - if (params.GetPairingInstruction().has_value()) + const auto pairingInstruction = params.GetPairingInstruction(); + if (pairingInstruction.has_value()) { - snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *params.GetPairingInstruction()); + snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); txtFields[numTxtFields++] = txtPairingInstr; } } From a4c548bf6d30b342fa34844013ca7574230f7892 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:19:04 -0400 Subject: [PATCH 08/30] Replace chip optional from active resolveattempts --- src/lib/dnssd/ActiveResolveAttempts.cpp | 18 +- src/lib/dnssd/ActiveResolveAttempts.h | 8 +- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 14 +- src/lib/dnssd/platform/Dnssd.h | 4 +- .../dnssd/tests/TestActiveResolveAttempts.cpp | 174 +++++++++--------- 5 files changed, 111 insertions(+), 107 deletions(-) diff --git a/src/lib/dnssd/ActiveResolveAttempts.cpp b/src/lib/dnssd/ActiveResolveAttempts.cpp index f49056896e0b2e..f8f351d85cf05e 100644 --- a/src/lib/dnssd/ActiveResolveAttempts.cpp +++ b/src/lib/dnssd/ActiveResolveAttempts.cpp @@ -218,9 +218,9 @@ void ActiveResolveAttempts::MarkPending(ScheduledAttempt && attempt) entryToUse->nextRetryDelay = System::Clock::Seconds16(1); } -Optional ActiveResolveAttempts::GetTimeUntilNextExpectedResponse() const +std::optional ActiveResolveAttempts::GetTimeUntilNextExpectedResponse() const { - Optional minDelay = Optional::Missing(); + std::optional minDelay = std::nullopt; chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp(); @@ -234,20 +234,20 @@ Optional ActiveResolveAttempts::GetTimeUntilNextExpected if (now >= entry.queryDueTime) { // found an entry that needs processing right now - return Optional::Value(0); + return std::make_optional(0); } System::Clock::Timeout entryDelay = entry.queryDueTime - now; - if (!minDelay.HasValue() || (minDelay.Value() > entryDelay)) + if (!minDelay.has_value() || (*minDelay > entryDelay)) { - minDelay.SetValue(entryDelay); + minDelay.emplace(entryDelay); } } return minDelay; } -Optional ActiveResolveAttempts::NextScheduled() +std::optional ActiveResolveAttempts::NextScheduled() { chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp(); @@ -273,13 +273,13 @@ Optional ActiveResolveAttempts::NextSch entry.queryDueTime = now + entry.nextRetryDelay; entry.nextRetryDelay *= 2; - Optional attempt = MakeOptional(entry.attempt); - entry.attempt.firstSend = false; + std::optional attempt = std::make_optional(entry.attempt); + entry.attempt.firstSend = false; return attempt; } - return Optional::Missing(); + return std::nullopt; } bool ActiveResolveAttempts::ShouldResolveIpAddress(PeerId peerId) const diff --git a/src/lib/dnssd/ActiveResolveAttempts.h b/src/lib/dnssd/ActiveResolveAttempts.h index 55790bd1e3821e..cf0ef573ee41fd 100644 --- a/src/lib/dnssd/ActiveResolveAttempts.h +++ b/src/lib/dnssd/ActiveResolveAttempts.h @@ -18,9 +18,9 @@ #pragma once #include -#include +#include +#include -#include #include #include #include @@ -273,7 +273,7 @@ class ActiveResolveAttempts // Get minimum time until the next pending reply is required. // // Returns missing if no actively tracked elements exist. - chip::Optional GetTimeUntilNextExpectedResponse() const; + std::optional GetTimeUntilNextExpectedResponse() const; // Get the peer Id that needs scheduling for a query // @@ -283,7 +283,7 @@ class ActiveResolveAttempts // now' // - there is NO sorting implied by this call. Returned value will be // any peer that needs a new request sent - chip::Optional NextScheduled(); + std::optional NextScheduled(); /// Check if any of the pending queries are for the given host name for /// IP resolution. diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 403da6effcd36c..64b194339b8bd1 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -644,9 +644,9 @@ CHIP_ERROR MinMdnsResolver::SendAllPendingQueries() { while (true) { - Optional resolve = mActiveResolves.NextScheduled(); + std::optional resolve = mActiveResolves.NextScheduled(); - if (!resolve.HasValue()) + if (!resolve.has_value()) { break; } @@ -657,9 +657,9 @@ CHIP_ERROR MinMdnsResolver::SendAllPendingQueries() QueryBuilder builder(std::move(buffer)); builder.Header().SetMessageId(0); - ReturnErrorOnFailure(BuildQuery(builder, resolve.Value())); + ReturnErrorOnFailure(BuildQuery(builder, *resolve)); - if (resolve.Value().firstSend) + if (resolve->firstSend) { ReturnErrorOnFailure(GlobalMinimalMdnsServer::Server().BroadcastUnicastQuery(builder.ReleasePacket(), kMdnsPort)); } @@ -744,14 +744,14 @@ CHIP_ERROR MinMdnsResolver::ScheduleRetries() ReturnErrorCodeIf(mSystemLayer == nullptr, CHIP_ERROR_INCORRECT_STATE); mSystemLayer->CancelTimer(&RetryCallback, this); - Optional delay = mActiveResolves.GetTimeUntilNextExpectedResponse(); + std::optional delay = mActiveResolves.GetTimeUntilNextExpectedResponse(); - if (!delay.HasValue()) + if (!delay.has_value()) { return CHIP_NO_ERROR; } - return mSystemLayer->StartTimer(delay.Value(), &RetryCallback, this); + return mSystemLayer->StartTimer(*delay, &RetryCallback, this); } void MinMdnsResolver::RetryCallback(System::Layer *, void * self) diff --git a/src/lib/dnssd/platform/Dnssd.h b/src/lib/dnssd/platform/Dnssd.h index fcbeabe1ce8d51..02fb851a12d861 100644 --- a/src/lib/dnssd/platform/Dnssd.h +++ b/src/lib/dnssd/platform/Dnssd.h @@ -26,12 +26,12 @@ #pragma once #include +#include #include #include #include #include -#include #include #include #include @@ -77,7 +77,7 @@ struct DnssdService size_t mTextEntrySize; const char ** mSubTypes; size_t mSubTypeSize; - Optional mAddress; + std::optional mAddress; // Time to live in seconds. Per rfc6762 section 10, because we have a hostname, our default TTL is 120 seconds uint32_t mTtlSeconds = 120; diff --git a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp index 4c2665658f3493..49ea375b774740 100644 --- a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp +++ b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp @@ -31,14 +31,17 @@ PeerId MakePeerId(NodeId nodeId) return peerId.SetNodeId(nodeId).SetCompressedFabricId(123); } -Optional ScheduledPeer(NodeId id, bool first) +std::optional ScheduledPeer(NodeId id, bool first) { - return Optional::Value(ActiveResolveAttempts::ScheduledAttempt(MakePeerId(id), first)); + return std::make_optional( + ActiveResolveAttempts::ScheduledAttempt(MakePeerId(id), first)); } -Optional ScheduledBrowse(const Dnssd::DiscoveryFilter & filter, - const Dnssd::DiscoveryType type, bool first) + +std::optional ScheduledBrowse(const Dnssd::DiscoveryFilter & filter, + const Dnssd::DiscoveryType type, bool first) { - return Optional::Value(ActiveResolveAttempts::ScheduledAttempt(filter, type, first)); + return std::make_optional( + ActiveResolveAttempts::ScheduledAttempt(filter, type, first)); } TEST(TestActiveResolveAttempts, TestSinglePeerAddRemove) @@ -49,39 +52,39 @@ TEST(TestActiveResolveAttempts, TestSinglePeerAddRemove) mockClock.AdvanceMonotonic(1234_ms32); // Starting up, no scheduled peers are expected - EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().has_value()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // Adding a single peer should result in it being scheduled attempts.MarkPending(MakePeerId(1)); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 1000 ms - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); mockClock.AdvanceMonotonic(500_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(500_ms32)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(500_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // past due date: timeout should be 0 mockClock.AdvanceMonotonic(800_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 2000 ms // sincve the timeout doubles every time - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(2000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(2000_ms32)); mockClock.AdvanceMonotonic(100_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1900_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1900_ms32)); // once complete, nothing to schedule attempts.Complete(MakePeerId(1)); - EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().has_value()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); } TEST(TestActiveResolveAttempts, TestSingleBrowseAddRemove) @@ -94,41 +97,41 @@ TEST(TestActiveResolveAttempts, TestSingleBrowseAddRemove) mockClock.AdvanceMonotonic(1234_ms32); // Starting up, no scheduled peers are expected - EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().has_value()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // Adding a single attempt should result in it being scheduled attempts.MarkPending(filter, type); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 1000 ms - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); mockClock.AdvanceMonotonic(500_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(500_ms32)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(500_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // past due date: timeout should be 0 mockClock.AdvanceMonotonic(800_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 2000 ms // sincve the timeout doubles every time - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(2000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(2000_ms32)); mockClock.AdvanceMonotonic(100_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1900_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1900_ms32)); // once complete, nothing to schedule Dnssd::DiscoveredNodeData data; data.Set(); data.Get().longDiscriminator = 1234; attempts.CompleteCommissionable(data); - EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().has_value()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); } TEST(TestActiveResolveAttempts, TestRescheduleSamePeerId) @@ -140,27 +143,27 @@ TEST(TestActiveResolveAttempts, TestRescheduleSamePeerId) attempts.MarkPending(MakePeerId(1)); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 1000 ms - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); // 2nd try goes to 2 seconds (once at least 1 second passes) mockClock.AdvanceMonotonic(1234_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(2000_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(2000_ms32)); // reschedule starts fresh attempts.MarkPending(MakePeerId(1)); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); } TEST(TestActiveResolveAttempts, TestRescheduleSameFilter) @@ -174,27 +177,27 @@ TEST(TestActiveResolveAttempts, TestRescheduleSameFilter) attempts.MarkPending(filter, type); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // one Next schedule is called, expect to have a delay of 1000 ms - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); // 2nd try goes to 2 seconds (once at least 1 second passes) mockClock.AdvanceMonotonic(1234_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(2000_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(2000_ms32)); // reschedule starts fresh attempts.MarkPending(filter, type); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); } TEST(TestActiveResolveAttempts, TestLRU) @@ -208,15 +211,15 @@ TEST(TestActiveResolveAttempts, TestLRU) // add a single very old peer attempts.MarkPending(MakePeerId(9999)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(9999, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); mockClock.AdvanceMonotonic(1000_ms32); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(9999, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); mockClock.AdvanceMonotonic(2000_ms32); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(9999, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // at this point, peer 9999 has a delay of 4 seconds. Fill up the rest of the table @@ -226,25 +229,26 @@ TEST(TestActiveResolveAttempts, TestLRU) mockClock.AdvanceMonotonic(1_ms32); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(i, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); } // +2 because: 1 element skipped, one element is the "current" that has a delay of 1000ms EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), - Optional::Value( + std::make_optional( System::Clock::Milliseconds32(1000 - mdns::Minimal::ActiveResolveAttempts::kRetryQueueSize + 2))); // add another element - this should overwrite peer 9999 attempts.MarkPending(MakePeerId(mdns::Minimal::ActiveResolveAttempts::kRetryQueueSize)); mockClock.AdvanceMonotonic(32_s16); - for (Optional s = attempts.NextScheduled(); s.HasValue(); s = attempts.NextScheduled()) + for (std::optional s = attempts.NextScheduled(); s.has_value(); + s = attempts.NextScheduled()) { - EXPECT_NE(s.Value().ResolveData().peerId.GetNodeId(), 9999u); + EXPECT_NE(s->ResolveData().peerId.GetNodeId(), 9999u); } // Still have active pending items (queue is full) - EXPECT_TRUE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); + EXPECT_TRUE(attempts.GetTimeUntilNextExpectedResponse().has_value()); // expire all of them. Since we double timeout every expiry, we expect a // few iteratios to be able to expire the entire queue @@ -253,18 +257,18 @@ TEST(TestActiveResolveAttempts, TestLRU) int i = 0; for (; i < kMaxIterations; i++) { - Optional ms = attempts.GetTimeUntilNextExpectedResponse(); - if (!ms.HasValue()) + std::optional ms = attempts.GetTimeUntilNextExpectedResponse(); + if (!ms.has_value()) { break; } - mockClock.AdvanceMonotonic(ms.Value()); + mockClock.AdvanceMonotonic(*ms); - Optional s = attempts.NextScheduled(); - while (s.HasValue()) + std::optional s = attempts.NextScheduled(); + while (s.has_value()) { - EXPECT_NE(s.Value().ResolveData().peerId.GetNodeId(), 9999u); + EXPECT_NE(s->ResolveData().peerId.GetNodeId(), 9999u); s = attempts.NextScheduled(); } } @@ -282,48 +286,48 @@ TEST(TestActiveResolveAttempts, TestNextPeerOrdering) attempts.MarkPending(MakePeerId(1)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); mockClock.AdvanceMonotonic(20_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(980_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(980_ms32)); // expect peerid to be resolve within 1 second from now attempts.MarkPending(MakePeerId(2)); // mock that we are querying 2 as well EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(2, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); mockClock.AdvanceMonotonic(80_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(900_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(900_ms32)); // Peer 1 is done, now peer2 should be pending (in 980ms) attempts.Complete(MakePeerId(1)); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(920_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(920_ms32)); mockClock.AdvanceMonotonic(20_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(900_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(900_ms32)); // Once peer 3 is added, queue should be // - 900 ms until peer id 2 is pending // - 1000 ms until peer id 3 is pending attempts.MarkPending(MakePeerId(3)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(3, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(900_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(900_ms32)); // After the clock advance // - 400 ms until peer id 2 is pending // - 500 ms until peer id 3 is pending mockClock.AdvanceMonotonic(500_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(400_ms32)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(400_ms32)); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // advancing the clock 'too long' will return both other entries, in reverse order due to how // the internal cache is built mockClock.AdvanceMonotonic(500_ms32); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(3, false)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(2, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); } TEST(TestActiveResolveAttempts, TestCombination) @@ -342,32 +346,32 @@ TEST(TestActiveResolveAttempts, TestCombination) EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, true)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // At this point, both should reset, so we're back to 1000ms - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(1000_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(1000_ms32)); // We used 20 ms, so the next time for the peer and resolve should be 980 ms mockClock.AdvanceMonotonic(20_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(980_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(980_ms32)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // Add a second Peer mockClock.AdvanceMonotonic(20_ms32); attempts.MarkPending(MakePeerId(2)); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(2, true)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // Advance to the retry time of peer 1 and the resolve mockClock.AdvanceMonotonic(960_ms32); - EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), Optional(0_ms32)); + EXPECT_EQ(attempts.GetTimeUntilNextExpectedResponse(), std::make_optional(0_ms32)); EXPECT_EQ(attempts.NextScheduled(), ScheduledPeer(1, false)); EXPECT_EQ(attempts.NextScheduled(), ScheduledBrowse(filter, type, false)); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); // Complete all, we should see no more scheduled. attempts.Complete(MakePeerId(2)); @@ -377,7 +381,7 @@ TEST(TestActiveResolveAttempts, TestCombination) data.Get().longDiscriminator = 1234; attempts.CompleteCommissionable(data); - EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().HasValue()); - EXPECT_FALSE(attempts.NextScheduled().HasValue()); + EXPECT_FALSE(attempts.GetTimeUntilNextExpectedResponse().has_value()); + EXPECT_FALSE(attempts.NextScheduled().has_value()); } } // namespace From dac2ee588faf9970abb44abec83c7df21046914b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:21:21 -0400 Subject: [PATCH 09/30] Fix platform dns --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index ba280edcc3f4f4..f7d042d1e1585d 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -73,13 +73,13 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser // For some platforms browsed services are already resolved, so verify if resolve is really needed or call resolve callback // Check if SRV, TXT and AAAA records were received in DNS responses - if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !services[i].mAddress.HasValue()) + if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !services[i].mAddress.has_value()) { ChipDnssdResolve(&services[i], services[i].mInterface, HandleNodeResolve, context); } else { - Inet::IPAddress * address = &(services[i].mAddress.Value()); + Inet::IPAddress * address = &(*services[i].mAddress); HandleNodeResolve(context, &services[i], Span(address, 1), error); } } From 07d65236fe4a810fcc0cc28c82ee619c2fd3a696 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:31:23 -0400 Subject: [PATCH 10/30] More test fixes --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 7 ++-- src/lib/dnssd/Resolver.h | 10 +++--- src/lib/dnssd/platform/tests/TestPlatform.cpp | 32 +++++++++---------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index f7d042d1e1585d..03e7f315af81b2 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -760,16 +760,15 @@ CHIP_ERROR DiscoveryImplPlatform::StartDiscovery(DiscoveryType type, DiscoveryFi CHIP_ERROR DiscoveryImplPlatform::StopDiscovery(DiscoveryContext & context) { - if (!context.GetBrowseIdentifier().HasValue()) + const auto browseIdentifier = context.GetBrowseIdentifier(); + if (!browseIdentifier.has_value()) { // No discovery going on. return CHIP_NO_ERROR; } - const auto browseIdentifier = context.GetBrowseIdentifier().Value(); context.ClearBrowseIdentifier(); - - return ChipDnssdStopBrowse(browseIdentifier); + return ChipDnssdStopBrowse(*browseIdentifier); } CHIP_ERROR DiscoveryImplPlatform::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index bf684cef6a12a5..28406e1dc724f1 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -19,13 +19,13 @@ #include #include +#include #include #include #include #include #include -#include #include #include #include @@ -74,9 +74,9 @@ class OperationalResolveDelegate class DiscoveryContext : public ReferenceCounted { public: - void SetBrowseIdentifier(intptr_t identifier) { mBrowseIdentifier.Emplace(identifier); } - void ClearBrowseIdentifier() { mBrowseIdentifier.ClearValue(); } - const Optional & GetBrowseIdentifier() const { return mBrowseIdentifier; } + void SetBrowseIdentifier(intptr_t identifier) { mBrowseIdentifier.emplace(identifier); } + void ClearBrowseIdentifier() { mBrowseIdentifier.reset(); } + const std::optional & GetBrowseIdentifier() const { return mBrowseIdentifier; } void SetDiscoveryDelegate(DiscoverNodeDelegate * delegate) { mDelegate = delegate; } void OnNodeDiscovered(const DiscoveredNodeData & nodeData) @@ -93,7 +93,7 @@ class DiscoveryContext : public ReferenceCounted private: DiscoverNodeDelegate * mDelegate = nullptr; - Optional mBrowseIdentifier; + std::optional mBrowseIdentifier; }; /** diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp index ebdb46643847c6..696c32f46e0c97 100644 --- a/src/lib/dnssd/platform/tests/TestPlatform.cpp +++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp @@ -52,7 +52,7 @@ OperationalAdvertisingParameters operationalParams2 = .SetMac(ByteSpan(kMac)) .SetPort(CHIP_PORT) .EnableIpV4(true) - .SetLocalMRPConfig(Optional::Value(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below + .SetLocalMRPConfig(std::make_optional(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below .SetICDModeToAdvertise(ICDModeAdvertise::kSIT); test::ExpectedCall operationalCall2 = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp) @@ -87,17 +87,17 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic = .SetMac(ByteSpan(kMac, sizeof(kMac))) .SetLongDiscriminator(22) .SetShortDiscriminator(2) - .SetVendorId(Optional(555)) - .SetDeviceType(Optional(70000)) + .SetVendorId(std::make_optional(555)) + .SetDeviceType(std::make_optional(70000)) .SetCommissioningMode(CommissioningMode::kEnabledBasic) - .SetDeviceName(Optional("testy-test")) - .SetPairingHint(Optional(3)) - .SetPairingInstruction(Optional("Pair me")) - .SetProductId(Optional(897)) - .SetRotatingDeviceId(Optional("id_that_spins")) + .SetDeviceName(std::make_optional("testy-test")) + .SetPairingHint(std::make_optional(3)) + .SetPairingInstruction(std::make_optional("Pair me")) + .SetProductId(std::make_optional(897)) + .SetRotatingDeviceId(std::make_optional("id_that_spins")) .SetICDModeToAdvertise(ICDModeAdvertise::kSIT) // 3600005 is over the max, so this should be adjusted by the platform - .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600005_ms32, 65535_ms16)); + .SetLocalMRPConfig(std::make_optional(3600000_ms32, 3600005_ms32, 65535_ms16)); test::ExpectedCall commissionableLargeBasic = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolUdp) @@ -126,14 +126,14 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced = .SetMac(ByteSpan(kMac, sizeof(kMac))) .SetLongDiscriminator(22) .SetShortDiscriminator(2) - .SetVendorId(chip::Optional(555)) - .SetDeviceType(chip::Optional(70000)) + .SetVendorId(std::make_optional(555)) + .SetDeviceType(std::make_optional(70000)) .SetCommissioningMode(CommissioningMode::kEnabledEnhanced) - .SetDeviceName(chip::Optional("testy-test")) - .SetPairingHint(chip::Optional(3)) - .SetPairingInstruction(chip::Optional("Pair me")) - .SetProductId(chip::Optional(897)) - .SetRotatingDeviceId(chip::Optional("id_that_spins")); + .SetDeviceName(std::make_optional("testy-test")) + .SetPairingHint(std::make_optional(3)) + .SetPairingInstruction(std::make_optional("Pair me")) + .SetProductId(std::make_optional(897)) + .SetRotatingDeviceId(std::make_optional("id_that_spins")); test::ExpectedCall commissionableLargeEnhanced = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolUdp) From a4f31bd61467e1d4477b77778dd1243c3a237623 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:34:18 -0400 Subject: [PATCH 11/30] Fix one more compile to not have optional at all in dnssd --- .../dnssd/minimal_mdns/tests/TestAdvertiser.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp index 523e38b0b17ddf..c868939573f1c7 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp @@ -196,15 +196,15 @@ CommissionAdvertisingParameters commissionableNodeParamsEnhancedAsICDLIT = .SetMac(ByteSpan(kMac, sizeof(kMac))) .SetLongDiscriminator(22) .SetShortDiscriminator(2) - .SetVendorId(chip::Optional(555)) - .SetDeviceType(chip::Optional(70000)) + .SetVendorId(std::make_optional(555)) + .SetDeviceType(std::make_optional(70000)) .SetCommissioningMode(CommissioningMode::kEnabledEnhanced) - .SetDeviceName(chip::Optional("testy-test")) - .SetPairingHint(chip::Optional(3)) - .SetPairingInstruction(chip::Optional("Pair me")) - .SetProductId(chip::Optional(897)) + .SetDeviceName(std::make_optional("testy-test")) + .SetPairingHint(std::make_optional(3)) + .SetPairingInstruction(std::make_optional("Pair me")) + .SetProductId(std::make_optional(897)) .SetICDModeToAdvertise(ICDModeAdvertise::kLIT) - .SetLocalMRPConfig(Optional::Value(3600000_ms32, 3600000_ms32, 65535_ms16)); + .SetLocalMRPConfig(std::make_optional(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", From 4d4e7788f6fc12b76dfacd57258413ed18cd837f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:37:21 -0400 Subject: [PATCH 12/30] Restyle --- src/lib/dnssd/ActiveResolveAttempts.h | 1 - src/lib/dnssd/platform/tests/TestPlatform.cpp | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/lib/dnssd/ActiveResolveAttempts.h b/src/lib/dnssd/ActiveResolveAttempts.h index cf0ef573ee41fd..6b1f71a51ea82a 100644 --- a/src/lib/dnssd/ActiveResolveAttempts.h +++ b/src/lib/dnssd/ActiveResolveAttempts.h @@ -17,7 +17,6 @@ #pragma once -#include #include #include diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp index 696c32f46e0c97..324863a948902d 100644 --- a/src/lib/dnssd/platform/tests/TestPlatform.cpp +++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp @@ -46,14 +46,14 @@ test::ExpectedCall operationalCall1 = test::ExpectedCall() .SetInstanceName("BEEFBEEFF00DF00D-1111222233334444") .SetHostName(host) .AddSubtype("_IBEEFBEEFF00DF00D"); -OperationalAdvertisingParameters operationalParams2 = - OperationalAdvertisingParameters() - .SetPeerId(kPeerId2) - .SetMac(ByteSpan(kMac)) - .SetPort(CHIP_PORT) - .EnableIpV4(true) - .SetLocalMRPConfig(std::make_optional(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below - .SetICDModeToAdvertise(ICDModeAdvertise::kSIT); +OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters() + .SetPeerId(kPeerId2) + .SetMac(ByteSpan(kMac)) + .SetPort(CHIP_PORT) + .EnableIpV4(true) + .SetLocalMRPConfig(std::make_optional( + 32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below + .SetICDModeToAdvertise(ICDModeAdvertise::kSIT); test::ExpectedCall operationalCall2 = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp) .SetServiceName("_matter") From da53d9c500a97ef5baeabe658c3a0684080ad688 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:38:07 -0400 Subject: [PATCH 13/30] Add back removed header --- src/lib/dnssd/ActiveResolveAttempts.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/dnssd/ActiveResolveAttempts.h b/src/lib/dnssd/ActiveResolveAttempts.h index 6b1f71a51ea82a..157790b531c3e7 100644 --- a/src/lib/dnssd/ActiveResolveAttempts.h +++ b/src/lib/dnssd/ActiveResolveAttempts.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include From bf49d3e547a7dc85bb8d006daf8533e6978691fb Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 13:58:59 -0400 Subject: [PATCH 14/30] Some compile fixes for IP address --- .../OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp | 4 ++-- src/platform/nxp/common/DnssdImpl.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp index f4548e67829582..f11ffab9697823 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp @@ -1803,9 +1803,9 @@ void GenericThreadStackManagerImpl_OpenThread::DispatchResolve(intptr Dnssd::DnssdService & service = dnsResult->mMdnsService; Span ipAddrs; - if (service.mAddress.HasValue()) + if (service.mAddress.has_value()) { - ipAddrs = Span(&service.mAddress.Value(), 1); + ipAddrs = Span(&*service.mAddress, 1); } ThreadStackMgrImpl().mDnsResolveCallback(dnsResult->context, &service, ipAddrs, dnsResult->error); diff --git a/src/platform/nxp/common/DnssdImpl.cpp b/src/platform/nxp/common/DnssdImpl.cpp index 8050da137f26b8..17116f21fc277f 100644 --- a/src/platform/nxp/common/DnssdImpl.cpp +++ b/src/platform/nxp/common/DnssdImpl.cpp @@ -778,9 +778,9 @@ void DispatchResolve(intptr_t context) Dnssd::DnssdService & service = resolveContext->mMdnsService; Span ipAddrs; - if (service.mAddress.HasValue()) + if (service.mAddress.has_value()) { - ipAddrs = Span(&service.mAddress.Value(), 1); + ipAddrs = Span(&*service.mAddress, 1); } mDnsResolveCallback(resolveContext->matterCtx, &service, ipAddrs, resolveContext->error); From 61967bb007b1ae5789e8c24a8b2d8c8d6682f5d4 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 14:09:08 -0400 Subject: [PATCH 15/30] more compile fixes - tested that qpg thermostat compiles for me now --- .../OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp index f11ffab9697823..e812c0c454e261 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp @@ -1729,7 +1729,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::FromOtDnsRespons if (!otIp6IsAddressUnspecified(&serviceInfo.mHostAddress)) { mdnsService.mAddressType = Inet::IPAddressType::kIPv6; - mdnsService.mAddress = MakeOptional(ToIPAddress(serviceInfo.mHostAddress)); + mdnsService.mAddress = std::make_optional(ToIPAddress(serviceInfo.mHostAddress)); } // Check if TXT record was included in DNS response. @@ -1955,7 +1955,7 @@ void GenericThreadStackManagerImpl_OpenThread::OnDnsAddressResolveRes error = MapOpenThreadError(otDnsAddressResponseGetAddress(aResponse, 0, &address, nullptr)); if (error == CHIP_NO_ERROR) { - dnsResult->mMdnsService.mAddress = MakeOptional(ToIPAddress(address)); + dnsResult->mMdnsService.mAddress = std::make_optional(ToIPAddress(address)); } dnsResult->error = error; From 87ccf15760fc5544e9cf9c658038d6f7269afb36 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 14:16:54 -0400 Subject: [PATCH 16/30] Fix tizen build --- src/platform/Tizen/DnssdImpl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/Tizen/DnssdImpl.cpp b/src/platform/Tizen/DnssdImpl.cpp index cc3e5a1e8f76a6..1b57e1811a1495 100644 --- a/src/platform/Tizen/DnssdImpl.cpp +++ b/src/platform/Tizen/DnssdImpl.cpp @@ -354,7 +354,7 @@ void OnResolve(dnssd_error_e result, dnssd_service_h service, void * userData) VerifyOrExit(ret == DNSSD_ERROR_NONE, ChipLogError(DeviceLayer, "dnssd_service_get_all_txt_record() failed: %s", get_error_message(ret))); - rCtx->mResult.mAddress.SetValue(ipAddr); + rCtx->mResult.mAddress.emplace(ipAddr); { // Before calling the Resolve() callback, we need to lock stack mutex. @@ -467,7 +467,7 @@ void ResolveContext::Finalize(CHIP_ERROR error) mResult.mTextEntries = textEntries.empty() ? nullptr : textEntries.data(); mResult.mTextEntrySize = textEntries.size(); - chip::Inet::IPAddress ipAddr = mResult.mAddress.Value(); + chip::Inet::IPAddress ipAddr = mResult.mAddress.value(); mCallback(mCbContext, &mResult, chip::Span(&ipAddr, 1), CHIP_NO_ERROR); } From b4734e5f2ebfb7af8e4a992543a9decd268f0cc9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 14:23:03 -0400 Subject: [PATCH 17/30] Commissioner passcode fix --- src/app/server/Dnssd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 5ae0859b0c499d..3cdcd35f9a7e98 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -350,7 +350,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_PASSCODE else { - advertiseParameters.SetCommissionerPasscodeSupported(Optional(true)); + advertiseParameters.SetCommissionerPasscodeSupported(std::make_optional(true)); } #endif From d25aebadfa1670a4fb93091e180eea4753e839e9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 15:55:14 -0400 Subject: [PATCH 18/30] make clang-tidy happy --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 03e7f315af81b2..f8e5ac7a4d42d1 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -72,14 +72,16 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser discoveryContext->Retain(); // For some platforms browsed services are already resolved, so verify if resolve is really needed or call resolve callback + auto ipAddress = services[i].mAddress; + // Check if SRV, TXT and AAAA records were received in DNS responses - if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !services[i].mAddress.has_value()) + if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !ipAddress.has_value()) { ChipDnssdResolve(&services[i], services[i].mInterface, HandleNodeResolve, context); } else { - Inet::IPAddress * address = &(*services[i].mAddress); + Inet::IPAddress * address = &(*ipAddress); HandleNodeResolve(context, &services[i], Span(address, 1), error); } } From 84e252346d84c54b83ad97e093567d96ead6536a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 16:46:14 -0400 Subject: [PATCH 19/30] Use references for MRPConfig optionals, as the mrpconfig may be larger --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 2 +- src/lib/dnssd/Discovery_ImplPlatform.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 62722f58421fc7..8fcfc4582d2a2a 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -226,7 +226,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser, CHIP_ERROR AddCommonTxtEntries(const BaseAdvertisingParams & params, CommonTxtEntryStorage & storage, char ** txtFields, size_t & numTxtFields) { - const auto optionalMrp = params.GetLocalMRPConfig(); + const auto & optionalMrp = params.GetLocalMRPConfig(); if (optionalMrp.has_value()) { diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index f8e5ac7a4d42d1..1a2b0308ad6469 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -176,7 +176,7 @@ CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, std::optional optional, +CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, const std::optional & optional, TxtFieldKey key) { VerifyOrReturnError((key == TxtFieldKey::kSessionIdleInterval || key == TxtFieldKey::kSessionActiveInterval || From b670c531395439cdb23aa67a795dad97f641e823 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 16:49:04 -0400 Subject: [PATCH 20/30] Reduce scope of optional usage --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 146 ++++++++++--------- 1 file changed, 81 insertions(+), 65 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 8fcfc4582d2a2a..61f3b4cdd0ae36 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -278,14 +278,16 @@ class AdvertiserMinMdns : public ServiceAdvertiser, txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf; } } - const auto tcpSupported = params.GetTcpSupported(); - if (tcpSupported.has_value()) { - size_t writtenCharactersNumber = - static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); - VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), - CHIP_ERROR_INVALID_STRING_LENGTH); - txtFields[numTxtFields++] = storage.tcpSupportedBuf; + const auto tcpSupported = params.GetTcpSupported(); + if (tcpSupported.has_value()) + { + size_t writtenCharactersNumber = + static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); + VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), + CHIP_ERROR_INVALID_STRING_LENGTH); + txtFields[numTxtFields++] = storage.tcpSupportedBuf; + } } if (params.GetICDModeToAdvertise() != ICDModeAdvertise::kNone) { @@ -671,39 +673,43 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } - const auto vendorId = params.GetVendorId(); - if (vendorId.has_value()) { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); - FullQName vendorServiceName = - allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); - ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); - - if (!allocator->AddResponder(vendorServiceName, instanceName) - .SetReportAdditional(instanceName) - .SetReportInServiceListing(true) - .IsValid()) + const auto vendorId = params.GetVendorId(); + if (vendorId.has_value()) { - ChipLogError(Discovery, "Failed to add vendor PTR record mDNS responder"); - return CHIP_ERROR_NO_MEMORY; + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); + FullQName vendorServiceName = + allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); + ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); + + if (!allocator->AddResponder(vendorServiceName, instanceName) + .SetReportAdditional(instanceName) + .SetReportInServiceListing(true) + .IsValid()) + { + ChipLogError(Discovery, "Failed to add vendor PTR record mDNS responder"); + return CHIP_ERROR_NO_MEMORY; + } } } - const auto deviceType = params.GetDeviceType(); - if (deviceType.has_value()) { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); - FullQName vendorServiceName = - allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); - ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); - - if (!allocator->AddResponder(vendorServiceName, instanceName) - .SetReportAdditional(instanceName) - .SetReportInServiceListing(true) - .IsValid()) + const auto deviceType = params.GetDeviceType(); + if (deviceType.has_value()) { - ChipLogError(Discovery, "Failed to add device type PTR record mDNS responder"); - return CHIP_ERROR_NO_MEMORY; + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); + FullQName vendorServiceName = + allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); + ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); + + if (!allocator->AddResponder(vendorServiceName, instanceName) + .SetReportAdditional(instanceName) + .SetReportInServiceListing(true) + .IsValid()) + { + ChipLogError(Discovery, "Failed to add device type PTR record mDNS responder"); + return CHIP_ERROR_NO_MEMORY; + } } } @@ -813,35 +819,39 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis : &mQueryResponderAllocatorCommissioner; char txtVidPid[chip::Dnssd::kKeyVendorProductMaxLength + 4]; - - const auto productId = params.GetProductId(); - const auto vendorId = params.GetVendorId(); - - if (productId.has_value() && vendorId.has_value()) { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *vendorId, *productId); - txtFields[numTxtFields++] = txtVidPid; - } - else if (vendorId.has_value()) - { - snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", *vendorId); - txtFields[numTxtFields++] = txtVidPid; + const auto productId = params.GetProductId(); + const auto vendorId = params.GetVendorId(); + if (productId.has_value() && vendorId.has_value()) + { + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *vendorId, *productId); + txtFields[numTxtFields++] = txtVidPid; + } + else if (vendorId.has_value()) + { + snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", *vendorId); + txtFields[numTxtFields++] = txtVidPid; + } } char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4]; - const auto deviceType = params.GetDeviceType(); - if (deviceType.has_value()) { - snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); - txtFields[numTxtFields++] = txtDeviceType; + const auto deviceType = params.GetDeviceType(); + if (deviceType.has_value()) + { + snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); + txtFields[numTxtFields++] = txtDeviceType; + } } char txtDeviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 4]; - const auto deviceName = params.GetDeviceName(); - if (deviceName.has_value()) { - snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *deviceName); - txtFields[numTxtFields++] = txtDeviceName; + const auto deviceName = params.GetDeviceName(); + if (deviceName.has_value()) + { + snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *deviceName); + txtFields[numTxtFields++] = txtDeviceName; + } } CommonTxtEntryStorage commonStorage; AddCommonTxtEntries(params, commonStorage, txtFields, numTxtFields); @@ -865,25 +875,31 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", static_cast(params.GetCommissioningMode())); txtFields[numTxtFields++] = txtCommissioningMode; - const auto rotatingDeviceId = params.GetRotatingDeviceId(); - if (rotatingDeviceId.has_value()) { - snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); - txtFields[numTxtFields++] = txtRotatingDeviceId; + const auto rotatingDeviceId = params.GetRotatingDeviceId(); + if (rotatingDeviceId.has_value()) + { + snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); + txtFields[numTxtFields++] = txtRotatingDeviceId; + } } - const auto pairingHint = params.GetPairingHint(); - if (pairingHint.has_value()) { - snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); - txtFields[numTxtFields++] = txtPairingHint; + const auto pairingHint = params.GetPairingHint(); + if (pairingHint.has_value()) + { + snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); + txtFields[numTxtFields++] = txtPairingHint; + } } - const auto pairingInstruction = params.GetPairingInstruction(); - if (pairingInstruction.has_value()) { - snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); - txtFields[numTxtFields++] = txtPairingInstr; + const auto pairingInstruction = params.GetPairingInstruction(); + if (pairingInstruction.has_value()) + { + snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); + txtFields[numTxtFields++] = txtPairingInstr; + } } } else From 903afd968fea6747205ad08f4325b462600b2331 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 17:04:09 -0400 Subject: [PATCH 21/30] Use references everywhere for Advertiser --- src/lib/dnssd/Advertiser.h | 12 ++++++------ src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 02888181f98181..2d443e5dd7db00 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -108,7 +108,7 @@ class BaseAdvertisingParams mTcpSupported = tcpSupported; return *reinterpret_cast(this); } - std::optional GetTcpSupported() const { return mTcpSupported; } + const std::optional & GetTcpSupported() const { return mTcpSupported; } Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode) { @@ -181,14 +181,14 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetVendorId() const { return mVendorId; } + const std::optional & GetVendorId() const { return mVendorId; } CommissionAdvertisingParameters & SetProductId(std::optional productId) { mProductId = productId; return *this; } - std::optional GetProductId() const { return mProductId; } + const std::optional & GetProductId() const { return mProductId; } CommissionAdvertisingParameters & SetCommissioningMode(CommissioningMode mode) { @@ -202,7 +202,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetDeviceType() const { return mDeviceType; } + const std::optional & GetDeviceType() const { return mDeviceType; } CommissionAdvertisingParameters & SetDeviceName(std::optional deviceName) { @@ -263,7 +263,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetPairingHint() const { return mPairingHint; } + const std::optional & GetPairingHint() const { return mPairingHint; } CommissionAdvertisingParameters & SetCommissionAdvertiseMode(CommssionAdvertiseMode mode) { @@ -277,7 +277,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } + const std::optional & GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } private: uint8_t mShortDiscriminator = 0; diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 61f3b4cdd0ae36..1c42f771ea4ea8 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -279,7 +279,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser, } } { - const auto tcpSupported = params.GetTcpSupported(); + const auto & tcpSupported = params.GetTcpSupported(); if (tcpSupported.has_value()) { size_t writtenCharactersNumber = @@ -674,7 +674,7 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } { - const auto vendorId = params.GetVendorId(); + const auto & vendorId = params.GetVendorId(); if (vendorId.has_value()) { MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); @@ -694,7 +694,7 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } { - const auto deviceType = params.GetDeviceType(); + const auto & deviceType = params.GetDeviceType(); if (deviceType.has_value()) { MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); @@ -820,8 +820,8 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis char txtVidPid[chip::Dnssd::kKeyVendorProductMaxLength + 4]; { - const auto productId = params.GetProductId(); - const auto vendorId = params.GetVendorId(); + const auto & productId = params.GetProductId(); + const auto & vendorId = params.GetVendorId(); if (productId.has_value() && vendorId.has_value()) { snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", *vendorId, *productId); @@ -836,7 +836,7 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4]; { - const auto deviceType = params.GetDeviceType(); + const auto & deviceType = params.GetDeviceType(); if (deviceType.has_value()) { snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); @@ -885,7 +885,7 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis } { - const auto pairingHint = params.GetPairingHint(); + const auto & pairingHint = params.GetPairingHint(); if (pairingHint.has_value()) { snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); From ee2f9c84be15d2b97f66714b9ff73b32caf0af45 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 17:08:09 -0400 Subject: [PATCH 22/30] One more use of a reference --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 1a2b0308ad6469..cc9e3872aeee50 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -72,7 +72,7 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser discoveryContext->Retain(); // For some platforms browsed services are already resolved, so verify if resolve is really needed or call resolve callback - auto ipAddress = services[i].mAddress; + auto & ipAddress = services[i].mAddress; // Check if SRV, TXT and AAAA records were received in DNS responses if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !ipAddress.has_value()) From a22f2e937b7ceef3b9730a84710f83c75204fb5d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 17:11:58 -0400 Subject: [PATCH 23/30] One more const reference passing to try to reduce code size --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index cc9e3872aeee50..9d3675e78a3533 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -115,7 +115,7 @@ CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t template CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t & entriesCount, char * buffer, size_t bufferLen, - std::optional value) + const std::optional &value) { VerifyOrReturnError(value.has_value(), CHIP_NO_ERROR); return AddPtrRecord(type, entries, entriesCount, buffer, bufferLen, *value); From d5861b841853865a905574a270f7d0ea090d73f9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 26 Apr 2024 17:15:00 -0400 Subject: [PATCH 24/30] Restyle --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 9d3675e78a3533..037119c38bdbef 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -115,7 +115,7 @@ CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t template CHIP_ERROR AddPtrRecord(DiscoveryFilterType type, const char ** entries, size_t & entriesCount, char * buffer, size_t bufferLen, - const std::optional &value) + const std::optional & value) { VerifyOrReturnError(value.has_value(), CHIP_NO_ERROR); return AddPtrRecord(type, entries, entriesCount, buffer, bufferLen, *value); From 58bcae6dfc4cc7662e701570bccefa831666b5f9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 29 Apr 2024 12:15:59 -0400 Subject: [PATCH 25/30] Attempt to make clang-tidy happy in unit tst --- src/lib/dnssd/tests/TestActiveResolveAttempts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp index 49ea375b774740..3559b735fbf45a 100644 --- a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp +++ b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp @@ -268,7 +268,7 @@ TEST(TestActiveResolveAttempts, TestLRU) std::optional s = attempts.NextScheduled(); while (s.has_value()) { - EXPECT_NE(s->ResolveData().peerId.GetNodeId(), 9999u); + EXPECT_NE(s.value().ResolveData().peerId.GetNodeId(), 9999u); s = attempts.NextScheduled(); } } From 052657cfd668f5076e8cbfcfc87a6863d59c9202 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 29 Apr 2024 12:19:10 -0400 Subject: [PATCH 26/30] Disable the check via a nolint --- src/lib/dnssd/tests/TestActiveResolveAttempts.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp index 3559b735fbf45a..0648c9880c597b 100644 --- a/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp +++ b/src/lib/dnssd/tests/TestActiveResolveAttempts.cpp @@ -268,7 +268,8 @@ TEST(TestActiveResolveAttempts, TestLRU) std::optional s = attempts.NextScheduled(); while (s.has_value()) { - EXPECT_NE(s.value().ResolveData().peerId.GetNodeId(), 9999u); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): this is checked in the while loop + EXPECT_NE(s->ResolveData().peerId.GetNodeId(), 9999u); s = attempts.NextScheduled(); } } From c0f435d9065b5f114673adc7308e88f34e82dae7 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 30 Apr 2024 10:49:38 -0400 Subject: [PATCH 27/30] Code review: use C++17 if initializers to scope out temporary optional values --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 119 ++++++++----------- 1 file changed, 48 insertions(+), 71 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 1c42f771ea4ea8..713bcafbcb7d58 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -226,9 +226,8 @@ class AdvertiserMinMdns : public ServiceAdvertiser, CHIP_ERROR AddCommonTxtEntries(const BaseAdvertisingParams & params, CommonTxtEntryStorage & storage, char ** txtFields, size_t & numTxtFields) { - const auto & optionalMrp = params.GetLocalMRPConfig(); - if (optionalMrp.has_value()) + if (const auto & optionalMrp = params.GetLocalMRPConfig(); optionalMrp.has_value()) { auto mrp = *optionalMrp; @@ -278,17 +277,16 @@ class AdvertiserMinMdns : public ServiceAdvertiser, txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf; } } + + if (const auto & tcpSupported = params.GetTcpSupported(); tcpSupported.has_value()) { - const auto & tcpSupported = params.GetTcpSupported(); - if (tcpSupported.has_value()) - { - size_t writtenCharactersNumber = - static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); - VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), - CHIP_ERROR_INVALID_STRING_LENGTH); - txtFields[numTxtFields++] = storage.tcpSupportedBuf; - } + size_t writtenCharactersNumber = + static_cast(snprintf(storage.tcpSupportedBuf, sizeof(storage.tcpSupportedBuf), "T=%d", *tcpSupported)); + VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.tcpSupportedBuf)), + CHIP_ERROR_INVALID_STRING_LENGTH); + txtFields[numTxtFields++] = storage.tcpSupportedBuf; } + if (params.GetICDModeToAdvertise() != ICDModeAdvertise::kNone) { size_t writtenCharactersNumber = @@ -673,43 +671,37 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & } } + if (const auto & vendorId = params.GetVendorId(); vendorId.has_value()) { - const auto & vendorId = params.GetVendorId(); - if (vendorId.has_value()) - { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); - FullQName vendorServiceName = - allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); - ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kVendorId, *vendorId)); + FullQName vendorServiceName = + allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); + ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); - if (!allocator->AddResponder(vendorServiceName, instanceName) - .SetReportAdditional(instanceName) - .SetReportInServiceListing(true) - .IsValid()) - { - ChipLogError(Discovery, "Failed to add vendor PTR record mDNS responder"); - return CHIP_ERROR_NO_MEMORY; - } + if (!allocator->AddResponder(vendorServiceName, instanceName) + .SetReportAdditional(instanceName) + .SetReportInServiceListing(true) + .IsValid()) + { + ChipLogError(Discovery, "Failed to add vendor PTR record mDNS responder"); + return CHIP_ERROR_NO_MEMORY; } } + if (const auto & deviceType = params.GetDeviceType(); deviceType.has_value()) { - const auto & deviceType = params.GetDeviceType(); - if (deviceType.has_value()) - { - MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); - FullQName vendorServiceName = - allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); - ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); + MakeServiceSubtype(nameBuffer, sizeof(nameBuffer), DiscoveryFilter(DiscoveryFilterType::kDeviceType, *deviceType)); + FullQName vendorServiceName = + allocator->AllocateQName(nameBuffer, kSubtypeServiceNamePart, serviceType, kCommissionProtocol, kLocalDomain); + ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY); - if (!allocator->AddResponder(vendorServiceName, instanceName) - .SetReportAdditional(instanceName) - .SetReportInServiceListing(true) - .IsValid()) - { - ChipLogError(Discovery, "Failed to add device type PTR record mDNS responder"); - return CHIP_ERROR_NO_MEMORY; - } + if (!allocator->AddResponder(vendorServiceName, instanceName) + .SetReportAdditional(instanceName) + .SetReportInServiceListing(true) + .IsValid()) + { + ChipLogError(Discovery, "Failed to add device type PTR record mDNS responder"); + return CHIP_ERROR_NO_MEMORY; } } @@ -835,23 +827,17 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis } char txtDeviceType[chip::Dnssd::kKeyDeviceTypeMaxLength + 4]; + if (const auto & deviceType = params.GetDeviceType(); deviceType.has_value()) { - const auto & deviceType = params.GetDeviceType(); - if (deviceType.has_value()) - { - snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); - txtFields[numTxtFields++] = txtDeviceType; - } + snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%" PRIu32, *deviceType); + txtFields[numTxtFields++] = txtDeviceType; } char txtDeviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 4]; + if (const auto & deviceName = params.GetDeviceName(); deviceName.has_value()) { - const auto deviceName = params.GetDeviceName(); - if (deviceName.has_value()) - { - snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *deviceName); - txtFields[numTxtFields++] = txtDeviceName; - } + snprintf(txtDeviceName, sizeof(txtDeviceName), "DN=%s", *deviceName); + txtFields[numTxtFields++] = txtDeviceName; } CommonTxtEntryStorage commonStorage; AddCommonTxtEntries(params, commonStorage, txtFields, numTxtFields); @@ -875,31 +861,22 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", static_cast(params.GetCommissioningMode())); txtFields[numTxtFields++] = txtCommissioningMode; + if (const auto rotatingDeviceId = params.GetRotatingDeviceId(); rotatingDeviceId.has_value()) { - const auto rotatingDeviceId = params.GetRotatingDeviceId(); - if (rotatingDeviceId.has_value()) - { - snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); - txtFields[numTxtFields++] = txtRotatingDeviceId; - } + snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); + txtFields[numTxtFields++] = txtRotatingDeviceId; } + if (const auto & pairingHint = params.GetPairingHint(); pairingHint.has_value()) { - const auto & pairingHint = params.GetPairingHint(); - if (pairingHint.has_value()) - { - snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); - txtFields[numTxtFields++] = txtPairingHint; - } + snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", *pairingHint); + txtFields[numTxtFields++] = txtPairingHint; } + if (const auto pairingInstruction = params.GetPairingInstruction(); pairingInstruction.has_value()) { - const auto pairingInstruction = params.GetPairingInstruction(); - if (pairingInstruction.has_value()) - { - snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); - txtFields[numTxtFields++] = txtPairingInstr; - } + snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); + txtFields[numTxtFields++] = txtPairingInstr; } } else From b6aa8c260ddc92c88fe31c90fbd2b878f65340d3 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 30 Apr 2024 10:51:01 -0400 Subject: [PATCH 28/30] Use a few more references --- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 713bcafbcb7d58..5ca002673cd4c9 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -861,7 +861,7 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", static_cast(params.GetCommissioningMode())); txtFields[numTxtFields++] = txtCommissioningMode; - if (const auto rotatingDeviceId = params.GetRotatingDeviceId(); rotatingDeviceId.has_value()) + if (const auto & rotatingDeviceId = params.GetRotatingDeviceId(); rotatingDeviceId.has_value()) { snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", *rotatingDeviceId); txtFields[numTxtFields++] = txtRotatingDeviceId; @@ -873,7 +873,7 @@ FullQName AdvertiserMinMdns::GetCommissioningTxtEntries(const CommissionAdvertis txtFields[numTxtFields++] = txtPairingHint; } - if (const auto pairingInstruction = params.GetPairingInstruction(); pairingInstruction.has_value()) + if (const auto & pairingInstruction = params.GetPairingInstruction(); pairingInstruction.has_value()) { snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", *pairingInstruction); txtFields[numTxtFields++] = txtPairingInstr; From d65c6ff080df418741ca99abbbfd43b0ef6533fb Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 30 Apr 2024 10:53:11 -0400 Subject: [PATCH 29/30] Apply code review comment --- src/lib/dnssd/Discovery_ImplPlatform.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 037119c38bdbef..f4a524f2c81f31 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ +#include #include #include @@ -762,7 +763,7 @@ CHIP_ERROR DiscoveryImplPlatform::StartDiscovery(DiscoveryType type, DiscoveryFi CHIP_ERROR DiscoveryImplPlatform::StopDiscovery(DiscoveryContext & context) { - const auto browseIdentifier = context.GetBrowseIdentifier(); + const std::optional browseIdentifier = context.GetBrowseIdentifier(); if (!browseIdentifier.has_value()) { // No discovery going on. From 1a68a9bae06f3cb33a2695b53ecabd25abe45d51 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 30 Apr 2024 10:58:27 -0400 Subject: [PATCH 30/30] Undo return as reference changes in getters for optional --- src/lib/dnssd/Advertiser.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 2d443e5dd7db00..02888181f98181 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -108,7 +108,7 @@ class BaseAdvertisingParams mTcpSupported = tcpSupported; return *reinterpret_cast(this); } - const std::optional & GetTcpSupported() const { return mTcpSupported; } + std::optional GetTcpSupported() const { return mTcpSupported; } Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode) { @@ -181,14 +181,14 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams & GetVendorId() const { return mVendorId; } + std::optional GetVendorId() const { return mVendorId; } CommissionAdvertisingParameters & SetProductId(std::optional productId) { mProductId = productId; return *this; } - const std::optional & GetProductId() const { return mProductId; } + std::optional GetProductId() const { return mProductId; } CommissionAdvertisingParameters & SetCommissioningMode(CommissioningMode mode) { @@ -202,7 +202,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams & GetDeviceType() const { return mDeviceType; } + std::optional GetDeviceType() const { return mDeviceType; } CommissionAdvertisingParameters & SetDeviceName(std::optional deviceName) { @@ -263,7 +263,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams & GetPairingHint() const { return mPairingHint; } + std::optional GetPairingHint() const { return mPairingHint; } CommissionAdvertisingParameters & SetCommissionAdvertiseMode(CommssionAdvertiseMode mode) { @@ -277,7 +277,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams & GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } + std::optional GetCommissionerPasscodeSupported() const { return mCommissionerPasscodeSupported; } private: uint8_t mShortDiscriminator = 0;