Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace some usages of chip::Optional with std::optional #33050

Merged
merged 4 commits into from
Apr 19, 2024
Merged

Replace some usages of chip::Optional with std::optional #33050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0302f19
Convert some chip::Optional to std::optional
andreilitvin Apr 18, 2024
d88aafe
Use operator*/-> for optional value access after has_value has been c…
andreilitvin Apr 18, 2024
2237b8c
Slightly more compact code
andreilitvin Apr 18, 2024
e6df82a
Restyle
andreilitvin Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions src/app/CommandHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ CHIP_ERROR CommandHandler::ValidateInvokeRequestMessageAndBuildRegistry(InvokeRe

// Grab the CommandRef if there is one, and validate that it's there when it
// has to be.
Optional<uint16_t> commandRef;
std::optional<uint16_t> commandRef;
uint16_t ref;
err = commandData.GetRef(&ref);
VerifyOrReturnError(err == CHIP_NO_ERROR || err == CHIP_END_OF_TLV, err);
Expand All @@ -168,7 +168,7 @@ CHIP_ERROR CommandHandler::ValidateInvokeRequestMessageAndBuildRegistry(InvokeRe
}
if (err == CHIP_NO_ERROR)
{
commandRef.SetValue(ref);
commandRef.emplace(ref);
}

// Adding can fail if concretePath is not unique, or if commandRef is a value
Expand Down Expand Up @@ -590,10 +590,9 @@ CHIP_ERROR CommandHandler::PrepareInvokeResponseCommand(const ConcreteCommandPat
const CommandHandler::InvokeResponseParameters & aPrepareParameters)
{
auto commandPathRegistryEntry = GetCommandPathRegistry().Find(aPrepareParameters.mRequestCommandPath);
VerifyOrReturnValue(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnValue(commandPathRegistryEntry.has_value(), CHIP_ERROR_INCORRECT_STATE);

return PrepareInvokeResponseCommand(commandPathRegistryEntry.Value(), aResponseCommandPath,
aPrepareParameters.mStartOrEndDataStruct);
return PrepareInvokeResponseCommand(*commandPathRegistryEntry, aResponseCommandPath, aPrepareParameters.mStartOrEndDataStruct);
}

CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aResponseCommandPath, bool aStartDataStruct)
Expand All @@ -610,9 +609,9 @@ CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aResponseC
"Seemingly device supports batch commands, but is calling the deprecated PrepareCommand API");

auto commandPathRegistryEntry = GetCommandPathRegistry().GetFirstEntry();
VerifyOrReturnValue(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnValue(commandPathRegistryEntry.has_value(), CHIP_ERROR_INCORRECT_STATE);

return PrepareInvokeResponseCommand(commandPathRegistryEntry.Value(), aResponseCommandPath, aStartDataStruct);
return PrepareInvokeResponseCommand(*commandPathRegistryEntry, aResponseCommandPath, aStartDataStruct);
}

CHIP_ERROR CommandHandler::PrepareInvokeResponseCommand(const CommandPathRegistryEntry & apCommandPathRegistryEntry,
Expand Down Expand Up @@ -675,9 +674,9 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct)
ReturnErrorOnFailure(commandData.GetWriter()->EndContainer(mDataElementContainerType));
}

if (mRefForResponse.HasValue())
if (mRefForResponse.has_value())
{
ReturnErrorOnFailure(commandData.Ref(mRefForResponse.Value()));
ReturnErrorOnFailure(commandData.Ref(*mRefForResponse));
}

ReturnErrorOnFailure(commandData.EndOfCommandDataIB());
Expand All @@ -699,8 +698,8 @@ CHIP_ERROR CommandHandler::PrepareStatus(const ConcreteCommandPath & aCommandPat
}

auto commandPathRegistryEntry = GetCommandPathRegistry().Find(aCommandPath);
VerifyOrReturnError(commandPathRegistryEntry.HasValue(), CHIP_ERROR_INCORRECT_STATE);
mRefForResponse = commandPathRegistryEntry.Value().ref;
VerifyOrReturnError(commandPathRegistryEntry.has_value(), CHIP_ERROR_INCORRECT_STATE);
mRefForResponse = commandPathRegistryEntry->ref;

MoveToState(State::Preparing);
InvokeResponseIBs::Builder & invokeResponses = mInvokeResponseBuilder.GetInvokeResponses();
Expand All @@ -720,9 +719,9 @@ CHIP_ERROR CommandHandler::FinishStatus()
VerifyOrReturnError(mState == State::AddingCommand, CHIP_ERROR_INCORRECT_STATE);

CommandStatusIB::Builder & commandStatus = mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus();
if (mRefForResponse.HasValue())
if (mRefForResponse.has_value())
{
ReturnErrorOnFailure(commandStatus.Ref(mRefForResponse.Value()));
ReturnErrorOnFailure(commandStatus.Ref(*mRefForResponse));
}

ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus().EndOfCommandStatusIB());
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ class CommandHandler
// TODO Allow flexibility in registration.
BasicCommandPathRegistry<CHIP_CONFIG_MAX_PATHS_PER_INVOKE> mBasicCommandPathRegistry;
CommandPathRegistry * mCommandPathRegistry = &mBasicCommandPathRegistry;
Optional<uint16_t> mRefForResponse;
std::optional<uint16_t> mRefForResponse;

CommandHandlerExchangeInterface * mpResponder = nullptr;

Expand Down
28 changes: 15 additions & 13 deletions src/app/CommandPathRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,27 @@
#include <lib/core/CHIPError.h>
#include <lib/core/Optional.h>

#include <optional>

namespace chip {
namespace app {

struct CommandPathRegistryEntry
{
ConcreteCommandPath requestPath = ConcreteCommandPath(0, 0, 0);
Optional<uint16_t> ref;
std::optional<uint16_t> ref;
};

class CommandPathRegistry
{
public:
virtual ~CommandPathRegistry() = default;

virtual Optional<CommandPathRegistryEntry> Find(const ConcreteCommandPath & requestPath) const = 0;
virtual Optional<CommandPathRegistryEntry> GetFirstEntry() const = 0;
virtual CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const Optional<uint16_t> & ref) = 0;
virtual size_t Count() const = 0;
virtual size_t MaxSize() const = 0;
virtual std::optional<CommandPathRegistryEntry> Find(const ConcreteCommandPath & requestPath) const = 0;
virtual std::optional<CommandPathRegistryEntry> GetFirstEntry() const = 0;
virtual CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const std::optional<uint16_t> & ref) = 0;
virtual size_t Count() const = 0;
virtual size_t MaxSize() const = 0;
};

/**
Expand All @@ -59,28 +61,28 @@ template <size_t N>
class BasicCommandPathRegistry : public CommandPathRegistry
{
public:
Optional<CommandPathRegistryEntry> Find(const ConcreteCommandPath & requestPath) const override
std::optional<CommandPathRegistryEntry> Find(const ConcreteCommandPath & requestPath) const override
{
for (size_t i = 0; i < mCount; i++)
{
if (mTable[i].requestPath == requestPath)
{
return MakeOptional(mTable[i]);
return std::make_optional(mTable[i]);
}
}
return NullOptional;
return std::nullopt;
}

Optional<CommandPathRegistryEntry> GetFirstEntry() const override
std::optional<CommandPathRegistryEntry> GetFirstEntry() const override
{
if (mCount > 0)
{
return MakeOptional(mTable[0]);
return std::make_optional(mTable[0]);
}
return NullOptional;
return std::nullopt;
}

CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const Optional<uint16_t> & ref) override
CHIP_ERROR Add(const ConcreteCommandPath & requestPath, const std::optional<uint16_t> & ref) override
{
if (mCount >= N)
{
Expand Down
16 changes: 8 additions & 8 deletions src/app/tests/TestBasicCommandPathRegistry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ void TestAddingSameConcretePath(nlTestSuite * apSuite, void * apContext)
BasicCommandPathRegistry<kQuickTestSize> basicCommandPathRegistry;

ConcreteCommandPath concretePath(0, 0, 0);
Optional<uint16_t> commandRef;
std::optional<uint16_t> commandRef;
uint16_t commandRefValue = 0;

size_t idx = 0;
for (idx = 0; idx < kQuickTestSize && err == CHIP_NO_ERROR; idx++)
{
commandRef.SetValue(commandRefValue);
commandRef.emplace(commandRefValue);
commandRefValue++;
err = basicCommandPathRegistry.Add(concretePath, commandRef);
}
Expand All @@ -56,8 +56,8 @@ void TestAddingSameCommandRef(nlTestSuite * apSuite, void * apContext)
CHIP_ERROR err = CHIP_NO_ERROR;
BasicCommandPathRegistry<kQuickTestSize> basicCommandPathRegistry;

Optional<uint16_t> commandRef;
commandRef.SetValue(0);
std::optional<uint16_t> commandRef;
commandRef.emplace(0);

uint16_t endpointValue = 0;

Expand All @@ -78,14 +78,14 @@ void TestAddingMaxNumberOfEntries(nlTestSuite * apSuite, void * apContext)
CHIP_ERROR err = CHIP_NO_ERROR;
BasicCommandPathRegistry<kQuickTestSize> basicCommandPathRegistry;

Optional<uint16_t> commandRef;
std::optional<uint16_t> commandRef;
uint16_t commandRefAndEndpointValue = 0;

size_t idx = 0;
for (idx = 0; idx < kQuickTestSize && err == CHIP_NO_ERROR; idx++)
{
ConcreteCommandPath concretePath(commandRefAndEndpointValue, 0, 0);
commandRef.SetValue(commandRefAndEndpointValue);
commandRef.emplace(commandRefAndEndpointValue);
commandRefAndEndpointValue++;
err = basicCommandPathRegistry.Add(concretePath, commandRef);
}
Expand All @@ -100,14 +100,14 @@ void TestAddingTooManyEntries(nlTestSuite * apSuite, void * apContext)
BasicCommandPathRegistry<kQuickTestSize> basicCommandPathRegistry;
size_t maxPlusOne = kQuickTestSize + 1;

Optional<uint16_t> commandRef;
std::optional<uint16_t> commandRef;
uint16_t commandRefAndEndpointValue = 0;

size_t idx = 0;
for (idx = 0; idx < maxPlusOne && err == CHIP_NO_ERROR; idx++)
{
ConcreteCommandPath concretePath(commandRefAndEndpointValue, 0, 0);
commandRef.SetValue(commandRefAndEndpointValue);
commandRef.emplace(commandRefAndEndpointValue);
commandRefAndEndpointValue++;
err = basicCommandPathRegistry.Add(concretePath, commandRef);
}
Expand Down
30 changes: 16 additions & 14 deletions src/app/tests/TestCommandInteraction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <optional>
#include <platform/CHIPDeviceLayer.h>
#include <protocols/interaction_model/Constants.h>
#include <system/SystemPacketBuffer.h>
Expand Down Expand Up @@ -392,7 +393,7 @@ class TestCommandInteraction
const Optional<uint16_t> & aRef) :
CommandHandler(apCallback)
{
GetCommandPathRegistry().Add(aRequestCommandPath, aRef);
GetCommandPathRegistry().Add(aRequestCommandPath, aRef.std_optional());
SetExchangeInterface(&mMockCommandResponder);
}
MockCommandResponder mMockCommandResponder;
Expand All @@ -407,7 +408,8 @@ class TestCommandInteraction
// payload will be included. Otherwise no payload will be included.
static void GenerateInvokeResponse(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
CommandId aCommandId, ClusterId aClusterId = kTestClusterId,
EndpointId aEndpointId = kTestEndpointId, Optional<uint16_t> aCommandRef = NullOptional);
EndpointId aEndpointId = kTestEndpointId,
std::optional<uint16_t> aCommandRef = std::nullopt);
static void AddInvokeRequestData(nlTestSuite * apSuite, void * apContext, CommandSender * apCommandSender,
CommandId aCommandId = kTestCommandIdWithData);
static void AddInvalidInvokeRequestData(nlTestSuite * apSuite, void * apContext, CommandSender * apCommandSender,
Expand Down Expand Up @@ -494,7 +496,7 @@ void TestCommandInteraction::GenerateInvokeRequest(nlTestSuite * apSuite, void *

void TestCommandInteraction::GenerateInvokeResponse(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
CommandId aCommandId, ClusterId aClusterId, EndpointId aEndpointId,
Optional<uint16_t> aCommandRef)
std::optional<uint16_t> aCommandRef)

{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -536,9 +538,9 @@ void TestCommandInteraction::GenerateInvokeResponse(nlTestSuite * apSuite, void
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
}

if (aCommandRef.HasValue())
if (aCommandRef.has_value())
{
NL_TEST_ASSERT(apSuite, commandDataIBBuilder.Ref(aCommandRef.Value()) == CHIP_NO_ERROR);
NL_TEST_ASSERT(apSuite, commandDataIBBuilder.Ref(*aCommandRef) == CHIP_NO_ERROR);
}

commandDataIBBuilder.EndOfCommandDataIB();
Expand Down Expand Up @@ -633,9 +635,9 @@ uint32_t TestCommandInteraction::GetAddResponseDataOverheadSizeForPath(nlTestSui
ConcreteCommandPath requestCommandPath1 = { kTestEndpointId, kTestClusterId, kTestCommandIdFillResponseMessage };
ConcreteCommandPath requestCommandPath2 = { kTestEndpointId, kTestClusterId, kTestCommandIdCommandSpecificResponse };

CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, MakeOptional<uint16_t>(static_cast<uint16_t>(1)));
CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, std::make_optional<uint16_t>(static_cast<uint16_t>(1)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = basicCommandPathRegistry.Add(requestCommandPath2, MakeOptional<uint16_t>(static_cast<uint16_t>(2)));
err = basicCommandPathRegistry.Add(requestCommandPath2, std::make_optional<uint16_t>(static_cast<uint16_t>(2)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

err = commandHandler.AllocateBuffer();
Expand Down Expand Up @@ -823,7 +825,7 @@ void TestCommandInteraction::TestCommandSenderExtendableApiWithProcessReceivedMs

uint16_t invalidResponseCommandRef = 2;
GenerateInvokeResponse(apSuite, apContext, buf, kTestCommandIdWithData, kTestClusterId, kTestEndpointId,
MakeOptional(invalidResponseCommandRef));
std::make_optional(invalidResponseCommandRef));
bool moreChunkedMessages = false;
err = commandSender.ProcessInvokeResponse(std::move(buf), moreChunkedMessages);
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_KEY_NOT_FOUND);
Expand Down Expand Up @@ -1948,9 +1950,9 @@ void TestCommandInteraction::TestCommandHandler_FillUpInvokeResponseMessageWhere
ConcreteCommandPath requestCommandPath1 = { kTestEndpointId, kTestClusterId, kTestCommandIdFillResponseMessage };
ConcreteCommandPath requestCommandPath2 = { kTestEndpointId, kTestClusterId, kTestCommandIdCommandSpecificResponse };

CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, MakeOptional<uint16_t>(static_cast<uint16_t>(1)));
CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, std::make_optional<uint16_t>(static_cast<uint16_t>(1)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = basicCommandPathRegistry.Add(requestCommandPath2, MakeOptional<uint16_t>(static_cast<uint16_t>(2)));
err = basicCommandPathRegistry.Add(requestCommandPath2, std::make_optional<uint16_t>(static_cast<uint16_t>(2)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

uint32_t sizeToLeave = 0;
Expand All @@ -1977,9 +1979,9 @@ void TestCommandInteraction::TestCommandHandler_FillUpInvokeResponseMessageWhere
ConcreteCommandPath requestCommandPath1 = { kTestEndpointId, kTestClusterId, kTestCommandIdFillResponseMessage };
ConcreteCommandPath requestCommandPath2 = { kTestEndpointId, kTestClusterId, kTestCommandIdCommandSpecificResponse };

CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, MakeOptional<uint16_t>(static_cast<uint16_t>(1)));
CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, std::make_optional<uint16_t>(static_cast<uint16_t>(1)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = basicCommandPathRegistry.Add(requestCommandPath2, MakeOptional<uint16_t>(static_cast<uint16_t>(2)));
err = basicCommandPathRegistry.Add(requestCommandPath2, std::make_optional<uint16_t>(static_cast<uint16_t>(2)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

uint32_t sizeToLeave = 0;
Expand All @@ -2005,9 +2007,9 @@ void TestCommandInteraction::TestCommandHandler_FillUpInvokeResponseMessageWhere
ConcreteCommandPath requestCommandPath1 = { kTestEndpointId, kTestClusterId, kTestCommandIdFillResponseMessage };
ConcreteCommandPath requestCommandPath2 = { kTestEndpointId, kTestClusterId, kTestCommandIdCommandSpecificResponse };

CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, MakeOptional<uint16_t>(static_cast<uint16_t>(1)));
CHIP_ERROR err = basicCommandPathRegistry.Add(requestCommandPath1, std::make_optional<uint16_t>(static_cast<uint16_t>(1)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = basicCommandPathRegistry.Add(requestCommandPath2, MakeOptional<uint16_t>(static_cast<uint16_t>(2)));
err = basicCommandPathRegistry.Add(requestCommandPath2, std::make_optional<uint16_t>(static_cast<uint16_t>(2)));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

uint32_t sizeToLeave = 0;
Expand Down
7 changes: 7 additions & 0 deletions src/lib/core/Optional.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#pragma once

#include <new>
#include <optional>
#include <type_traits>
#include <utility>

Expand Down Expand Up @@ -211,6 +212,12 @@ class Optional
bool operator==(const T & other) const { return HasValue() && Value() == other; }
bool operator!=(const T & other) const { return !(*this == other); }

std::optional<T> std_optional() const
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
{
VerifyOrReturnValue(HasValue(), std::nullopt);
return std::make_optional(Value());
}

/** Convenience method to create an optional without a valid value. */
static Optional<T> Missing() { return Optional<T>(); }

Expand Down
Loading