Skip to content

Commit

Permalink
Replace some usages of chip::Optional with std::optional (#33050)
Browse files Browse the repository at this point in the history 
* Convert some chip::Optional to std::optional

* Use operator*/-> for optional value access after has_value has been called

* Slightly more compact code

* Restyle

---------

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
@andy31415 @andreilitvin
andy31415 and andreilitvin authored Apr 19, 2024
1 parent cb8eace commit 9c06a05
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 49 deletions.
25 changes: 12 additions & 13 deletions src/app/CommandHandler.cpp
View file
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
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
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
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
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
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
{
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

0 comments on commit 9c06a05

Please sign in to comment.