From 3310412c1f4892bc1b533663f5fb327e319459dc Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 5 May 2022 22:41:55 -0400 Subject: [PATCH] Add support for "true" and "false" for boolean args in chip-tool. (#18091) * Add support for "true" and "false" for boolean args in chip-tool. This continues allowing 0 and 1 for now for backwards compat, as aliases for false and true. Other numeric values continue not being allowed. * Address review comments --- .../chip-tool/commands/common/Command.cpp | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/examples/chip-tool/commands/common/Command.cpp b/examples/chip-tool/commands/common/Command.cpp index 7c6846c5c5380c..7962d314f3fe49 100644 --- a/examples/chip-tool/commands/common/Command.cpp +++ b/examples/chip-tool/commands/common/Command.cpp @@ -182,6 +182,27 @@ bool Command::InitArgument(size_t argIndex, char * argValue) bool isHexNotation = strncmp(argValue, "0x", 2) == 0 || strncmp(argValue, "0X", 2) == 0; Argument arg = mArgs.at(argIndex); + + // We have two places where we handle uint8_t-typed args (actual int8u and + // bool args), so declare the handler function here so it can be reused. + auto uint8Handler = [&](uint8_t * value) { + // stringstream treats uint8_t as char, which is not what we want here. + uint16_t tmpValue; + std::stringstream ss; + isHexNotation ? (ss << std::hex << argValue) : (ss << argValue); + ss >> tmpValue; + if (chip::CanCastTo(tmpValue)) + { + *value = static_cast(tmpValue); + + uint64_t min = chip::CanCastTo(arg.min) ? static_cast(arg.min) : 0; + uint64_t max = arg.max; + return (!ss.fail() && ss.eof() && *value >= min && *value <= max); + } + + return false; + }; + switch (arg.type) { case ArgumentType::Complex: { @@ -330,28 +351,41 @@ bool Command::InitArgument(size_t argIndex, char * argValue) break; } - case ArgumentType::Bool: - case ArgumentType::Number_uint8: { - isValidArgument = HandleNullableOptional(arg, argValue, [&](auto * value) { - // stringstream treats uint8_t as char, which is not what we want here. - uint16_t tmpValue; - std::stringstream ss; - isHexNotation ? ss << std::hex << argValue : ss << argValue; - ss >> tmpValue; - if (chip::CanCastTo(tmpValue)) + case ArgumentType::Bool: { + isValidArgument = HandleNullableOptional(arg, argValue, [&](auto * value) { + // Start with checking for actual boolean values. + if (strcasecmp(argValue, "true") == 0) { - *value = static_cast(tmpValue); + *value = true; + return true; + } - uint64_t min = chip::CanCastTo(arg.min) ? static_cast(arg.min) : 0; - uint64_t max = arg.max; - return (!ss.fail() && ss.eof() && *value >= min && *value <= max); + if (strcasecmp(argValue, "false") == 0) + { + *value = false; + return true; } - return false; + // For backwards compat, keep accepting 0 and 1 for now as synonyms + // for false and true. Since we set our min to 0 and max to 1 for + // booleans, calling uint8Handler does the right thing in terms of + // only allowing those two values. + uint8_t temp = 0; + if (!uint8Handler(&temp)) + { + return false; + } + *value = (temp == 1); + return true; }); break; } + case ArgumentType::Number_uint8: { + isValidArgument = HandleNullableOptional(arg, argValue, uint8Handler); + break; + } + case ArgumentType::Number_uint16: { isValidArgument = HandleNullableOptional(arg, argValue, [&](auto * value) { std::stringstream ss;