Skip to content

Commit

Permalink
[chore] Replace all usages of chip::Optional with std::optional in sr…
Browse files Browse the repository at this point in the history 
…c/lib/dnssd (#33200)

* std optional in recordwriter

* Fix a call

* Change some advertiser.h to std::optional

* Fix usage in advertiser.cpp

* Make more things compile

* Restyle

* make clang-tidy happy

* Replace chip optional from active resolveattempts

* Fix platform dns

* More test fixes

* Fix one more compile to not have optional at all in dnssd

* Restyle

* Add back removed header

* Some compile fixes for IP address

* more compile fixes - tested that qpg thermostat compiles for me now

* Fix tizen build

* Commissioner passcode fix

* make clang-tidy happy

* Use references for MRPConfig optionals, as the mrpconfig may be larger

* Reduce scope of optional usage

* Use references everywhere for Advertiser

* One more use of a reference

* One more const reference passing to try to reduce code size

* Restyle

* Attempt to make clang-tidy happy in unit tst

* Disable the check via a nolint

* Code review: use C++17 if initializers to scope out temporary optional values

* Use a few more references

* Apply code review comment

* Undo return as reference changes in getters for optional

---------

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
@andy31415 @andreilitvin
andy31415 and andreilitvin authored Apr 30, 2024
1 parent f6bbfee commit 9ed9c72
Show file tree
Hide file tree
Showing 18 changed files with 304 additions and 292 deletions.
29 changes: 15 additions & 14 deletions examples/minimal-mdns/advertiser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
#include <cstdio>
#include <memory>
#include <optional>

#include <arpa/inet.h>
#include <strings.h>
Expand Down Expand Up @@ -45,18 +46,18 @@ struct Options
AdvertisingMode advertisingMode = AdvertisingMode::kCommissionableNode;

// commissionable node / commissioner params
Optional<uint16_t> vendorId;
Optional<uint16_t> productId;
Optional<uint32_t> deviceType;
Optional<const char *> deviceName;
std::optional<uint16_t> vendorId;
std::optional<uint16_t> productId;
std::optional<uint32_t> deviceType;
std::optional<const char *> deviceName;

// commissionable node params
uint8_t shortDiscriminator = 52;
uint16_t longDiscriminator = 840;
Dnssd::CommissioningMode commissioningMode = Dnssd::CommissioningMode::kDisabled;
Optional<const char *> rotatingId;
Optional<const char *> pairingInstr;
Optional<uint16_t> pairingHint;
std::optional<const char *> rotatingId;
std::optional<const char *> pairingInstr;
std::optional<uint16_t> pairingHint;

// operational params
uint64_t fabricId = 12345;
Expand Down Expand Up @@ -130,10 +131,10 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
gOptions.longDiscriminator = static_cast<uint16_t>(atoi(aValue));
return true;
case kOptionCommissioningVendorId:
gOptions.vendorId = Optional<uint16_t>::Value(static_cast<uint16_t>(atoi(aValue)));
gOptions.vendorId = std::make_optional<uint16_t>(static_cast<uint16_t>(atoi(aValue)));
return true;
case kOptionCommissioningProductId:
gOptions.productId = Optional<uint16_t>::Value(static_cast<uint16_t>(atoi(aValue)));
gOptions.productId = std::make_optional<uint16_t>(static_cast<uint16_t>(atoi(aValue)));
return true;
case kOptionCommissioningMode:
cm = static_cast<uint8_t>(atoi(aValue));
Expand All @@ -147,19 +148,19 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
}
return true;
case kOptionCommissioningDeviceType:
gOptions.deviceType = Optional<uint32_t>::Value(static_cast<uint32_t>(atoi(aValue)));
gOptions.deviceType = std::make_optional<uint32_t>(static_cast<uint32_t>(atoi(aValue)));
return true;
case kOptionCommissioningDeviceName:
gOptions.deviceName = Optional<const char *>::Value(static_cast<const char *>(aValue));
gOptions.deviceName = std::make_optional<const char *>(static_cast<const char *>(aValue));
return true;
case kOptionCommissioningRotatingId:
gOptions.rotatingId = Optional<const char *>::Value(static_cast<const char *>(aValue));
gOptions.rotatingId = std::make_optional<const char *>(static_cast<const char *>(aValue));
return true;
case kOptionCommissioningPairingInstr:
gOptions.pairingInstr = Optional<const char *>::Value(static_cast<const char *>(aValue));
gOptions.pairingInstr = std::make_optional<const char *>(static_cast<const char *>(aValue));
return true;
case kOptionCommissioningPairingHint:
gOptions.pairingHint = Optional<uint16_t>::Value(static_cast<uint16_t>(atoi(aValue)));
gOptions.pairingHint = std::make_optional<uint16_t>(static_cast<uint16_t>(atoi(aValue)));
return true;
case kOptionOperationalFabricId:
if (sscanf(aValue, "%" SCNx64, &gOptions.fabricId) != 1)
Expand Down
28 changes: 14 additions & 14 deletions src/app/server/Dnssd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -251,7 +251,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetVendorId(chip::Optional<uint16_t>::Value(value));
advertiseParameters.SetVendorId(std::make_optional<uint16_t>(value));
}

if (DeviceLayer::GetDeviceInstanceInfoProvider()->GetProductId(value) != CHIP_NO_ERROR)
Expand All @@ -260,23 +260,23 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetProductId(chip::Optional<uint16_t>::Value(value));
advertiseParameters.SetProductId(std::make_optional<uint16_t>(value));
}

if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceTypeEnabled() &&
DeviceLayer::ConfigurationMgr().GetDeviceTypeId(val32) == CHIP_NO_ERROR)
{
advertiseParameters.SetDeviceType(chip::Optional<uint32_t>::Value(val32));
advertiseParameters.SetDeviceType(std::make_optional<uint32_t>(val32));
}

char deviceName[chip::Dnssd::kKeyDeviceNameMaxLength + 1];
if (DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled() &&
DeviceLayer::ConfigurationMgr().GetCommissionableDeviceName(deviceName, sizeof(deviceName)) == CHIP_NO_ERROR)
{
advertiseParameters.SetDeviceName(chip::Optional<const char *>::Value(deviceName));
advertiseParameters.SetDeviceName(std::make_optional<const char *>(deviceName));
}

advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig());
advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig().std_optional());

#if CHIP_CONFIG_ENABLE_ICD_SERVER
AddICDKeyToAdvertisement(advertiseParameters);
Expand All @@ -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<const char *>::Value(rotatingDeviceIdHexBuffer));
advertiseParameters.SetRotatingDeviceId(std::make_optional<const char *>(rotatingDeviceIdHexBuffer));
#endif

if (!HaveOperationalCredentials())
Expand All @@ -314,7 +314,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
advertiseParameters.SetPairingHint(std::make_optional<uint16_t>(value));
}

if (DeviceLayer::ConfigurationMgr().GetInitialPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
Expand All @@ -323,7 +323,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
advertiseParameters.SetPairingInstruction(std::make_optional<const char *>(pairingInst));
}
}
else
Expand All @@ -334,7 +334,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetPairingHint(chip::Optional<uint16_t>::Value(value));
advertiseParameters.SetPairingHint(std::make_optional<uint16_t>(value));
}

if (DeviceLayer::ConfigurationMgr().GetSecondaryPairingInstruction(pairingInst, sizeof(pairingInst)) != CHIP_NO_ERROR)
Expand All @@ -343,24 +343,24 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
}
else
{
advertiseParameters.SetPairingInstruction(chip::Optional<const char *>::Value(pairingInst));
advertiseParameters.SetPairingInstruction(std::make_optional<const char *>(pairingInst));
}
}
}
#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_PASSCODE
else
{
advertiseParameters.SetCommissionerPasscodeSupported(Optional<bool>(true));
advertiseParameters.SetCommissionerPasscodeSupported(std::make_optional<bool>(true));
}
#endif

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

Expand Down
18 changes: 9 additions & 9 deletions src/lib/dnssd/ActiveResolveAttempts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ void ActiveResolveAttempts::MarkPending(ScheduledAttempt && attempt)
entryToUse->nextRetryDelay = System::Clock::Seconds16(1);
}

Optional<System::Clock::Timeout> ActiveResolveAttempts::GetTimeUntilNextExpectedResponse() const
std::optional<System::Clock::Timeout> ActiveResolveAttempts::GetTimeUntilNextExpectedResponse() const
{
Optional<System::Clock::Timeout> minDelay = Optional<System::Clock::Timeout>::Missing();
std::optional<System::Clock::Timeout> minDelay = std::nullopt;

chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp();

Expand All @@ -234,20 +234,20 @@ Optional<System::Clock::Timeout> ActiveResolveAttempts::GetTimeUntilNextExpected
if (now >= entry.queryDueTime)
{
// found an entry that needs processing right now
return Optional<System::Clock::Timeout>::Value(0);
return std::make_optional<System::Clock::Timeout>(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::ScheduledAttempt> ActiveResolveAttempts::NextScheduled()
std::optional<ActiveResolveAttempts::ScheduledAttempt> ActiveResolveAttempts::NextScheduled()
{
chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp();

Expand All @@ -273,13 +273,13 @@ Optional<ActiveResolveAttempts::ScheduledAttempt> ActiveResolveAttempts::NextSch
entry.queryDueTime = now + entry.nextRetryDelay;
entry.nextRetryDelay *= 2;

Optional<ScheduledAttempt> attempt = MakeOptional(entry.attempt);
entry.attempt.firstSend = false;
std::optional<ScheduledAttempt> attempt = std::make_optional(entry.attempt);
entry.attempt.firstSend = false;

return attempt;
}

return Optional<ScheduledAttempt>::Missing();
return std::nullopt;
}

bool ActiveResolveAttempts::ShouldResolveIpAddress(PeerId peerId) const
Expand Down
6 changes: 3 additions & 3 deletions src/lib/dnssd/ActiveResolveAttempts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

#include <cstddef>
#include <cstdint>
#include <optional>

#include <lib/core/Optional.h>
#include <lib/core/PeerId.h>
#include <lib/dnssd/Resolver.h>
#include <lib/dnssd/minimal_mdns/core/HeapQName.h>
Expand Down Expand Up @@ -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<chip::System::Clock::Timeout> GetTimeUntilNextExpectedResponse() const;
std::optional<chip::System::Clock::Timeout> GetTimeUntilNextExpectedResponse() const;

// Get the peer Id that needs scheduling for a query
//
Expand All @@ -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<ScheduledAttempt> NextScheduled();
std::optional<ScheduledAttempt> NextScheduled();

/// Check if any of the pending queries are for the given host name for
/// IP resolution.
Expand Down
Loading

0 comments on commit 9ed9c72

Please sign in to comment.