Skip to content

Commit

Permalink
Clean up optional arguments properly in chip-tool. (#20190)
Browse files Browse the repository at this point in the history
Otherwise in interactive mode we can end up with noise left over from
a previous command invocation.

Fixes #20151
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 20, 2023
1 parent 6bc26ea commit 2083113
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 27 deletions.
3 changes: 2 additions & 1 deletion examples/chip-tool/commands/clusters/ModelCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ void ModelCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CH

void ModelCommand::Shutdown()
{
ResetArguments();
mOnDeviceConnectedCallback.Cancel();
mOnDeviceConnectionFailureCallback.Cancel();

CHIPCommand::Shutdown();
}
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class CHIPCommand : public Command

// Shut down the command. After a Shutdown call the command object is ready
// to be used for another command invocation.
virtual void Shutdown() {}
virtual void Shutdown() { ResetArguments(); }

// Clean up any resources allocated by the command. Some commands may hold
// on to resources after Shutdown(), but Cleanup() will guarantee those are
Expand Down
160 changes: 135 additions & 25 deletions examples/chip-tool/commands/common/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -854,44 +854,154 @@ size_t Command::AddArgumentToList(Argument && argument)
return 0;
}

namespace {
template <typename T>
void ResetOptionalArg(const Argument & arg)
{
VerifyOrDie(arg.isOptional());

if (arg.isNullable())
{
reinterpret_cast<chip::Optional<chip::app::DataModel::Nullable<T>> *>(arg.value)->ClearValue();
}
else
{
reinterpret_cast<chip::Optional<T> *>(arg.value)->ClearValue();
}
}
} // anonymous namespace

void Command::ResetArguments()
{
for (size_t i = 0; i < mArgs.size(); i++)
{
const Argument arg = mArgs[i];
const ArgumentType type = arg.type;
const uint8_t flags = arg.flags;
if (type == ArgumentType::VectorBool && flags == Argument::kOptional)
if (arg.isOptional())
{
auto vectorArgument = static_cast<std::vector<uint16_t> *>(arg.value);
vectorArgument->clear();
}
else if (type == ArgumentType::Vector16 && flags != Argument::kOptional)
{
auto vectorArgument = static_cast<std::vector<uint16_t> *>(arg.value);
vectorArgument->clear();
}
else if (type == ArgumentType::Vector32 && flags != Argument::kOptional)
{
auto vectorArgument = static_cast<std::vector<uint32_t> *>(arg.value);
vectorArgument->clear();
}
else if (type == ArgumentType::Vector32 && flags == Argument::kOptional)
{
auto optionalArgument = static_cast<chip::Optional<std::vector<uint32_t>> *>(arg.value);
if (optionalArgument->HasValue())
// Must always clean these up so they don't carry over to the next
// command invocation in interactive mode.
switch (type)
{
optionalArgument->Value().clear();
case ArgumentType::Complex: {
// No optional complex arguments so far.
VerifyOrDie(false);
break;
}
case ArgumentType::Custom: {
// No optional custom arguments so far.
VerifyOrDie(false);
break;
}
case ArgumentType::VectorBool: {
auto vectorArgument = static_cast<std::vector<bool> *>(arg.value);
vectorArgument->clear();
break;
}
case ArgumentType::Vector16: {
// No optional Vector16 arguments so far.
VerifyOrDie(false);
break;
}
case ArgumentType::Vector32: {
ResetOptionalArg<std::vector<uint32_t>>(arg);
break;
}
case ArgumentType::VectorCustom: {
// No optional VectorCustom arguments so far.
VerifyOrDie(false);
break;
}
case ArgumentType::Attribute: {
// No optional Attribute arguments so far.
VerifyOrDie(false);
break;
}
case ArgumentType::String: {
ResetOptionalArg<char *>(arg);
break;
}
case ArgumentType::CharString: {
ResetOptionalArg<chip::CharSpan>(arg);
break;
}
case ArgumentType::OctetString: {
ResetOptionalArg<chip::ByteSpan>(arg);
break;
}
case ArgumentType::Bool: {
ResetOptionalArg<bool>(arg);
break;
}
case ArgumentType::Number_uint8: {
ResetOptionalArg<uint8_t>(arg);
break;
}
case ArgumentType::Number_uint16: {
ResetOptionalArg<uint16_t>(arg);
break;
}
case ArgumentType::Number_uint32: {
ResetOptionalArg<uint32_t>(arg);
break;
}
case ArgumentType::Number_uint64: {
ResetOptionalArg<uint64_t>(arg);
break;
}
case ArgumentType::Number_int8: {
ResetOptionalArg<int8_t>(arg);
break;
}
case ArgumentType::Number_int16: {
ResetOptionalArg<int16_t>(arg);
break;
}
case ArgumentType::Number_int32: {
ResetOptionalArg<int32_t>(arg);
break;
}
case ArgumentType::Number_int64: {
ResetOptionalArg<int64_t>(arg);
break;
}
case ArgumentType::Float: {
ResetOptionalArg<float>(arg);
break;
}
case ArgumentType::Double: {
ResetOptionalArg<double>(arg);
break;
}
case ArgumentType::Address: {
ResetOptionalArg<AddressWithInterface>(arg);
break;
}
}
}
else if (type == ArgumentType::VectorCustom && flags != Argument::kOptional)
else
{
auto vectorArgument = static_cast<std::vector<CustomArgument *> *>(arg.value);
for (auto & customArgument : *vectorArgument)
// Some non-optional arguments have state that needs to be cleaned
// up too.
if (type == ArgumentType::Vector16)
{
delete customArgument;
auto vectorArgument = static_cast<std::vector<uint16_t> *>(arg.value);
vectorArgument->clear();
}
else if (type == ArgumentType::Vector32)
{
auto vectorArgument = static_cast<std::vector<uint32_t> *>(arg.value);
vectorArgument->clear();
}
else if (type == ArgumentType::VectorCustom)
{
auto vectorArgument = static_cast<std::vector<CustomArgument *> *>(arg.value);
for (auto & customArgument : *vectorArgument)
{
delete customArgument;
}
vectorArgument->clear();
}
vectorArgument->clear();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@ void DiscoverCommissionersCommand::Shutdown()

ChipLogProgress(chipTool, "Total of %d commissioner(s) discovered in %u sec", commissionerCount,
std::chrono::duration_cast<System::Clock::Seconds16>(GetWaitDuration()).count());

CHIPCommand::Shutdown();
}

0 comments on commit 2083113

Please sign in to comment.