Skip to content

Commit

Permalink
Get rid of the non-spec EMBER_ZCL_STATUS_INVALID_ARGUMENT value. (#12192
Browse files Browse the repository at this point in the history
)

It used to be in the spec, but was then removed in favor of
INVALID_COMMAND or CONSTRAINT_ERROR.  The value we are using for it
collides with the spec-defined NEEDS_TIMED_INTERACTION value.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jan 27, 2022
1 parent 152b12d commit 3e6174b
Show file tree
Hide file tree
Showing 12 changed files with 1,926 additions and 1,923 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ EmberAfStatus StaticSupportedModesManager::getModeOptionByMode(unsigned short en
}
}
emberAfPrintln(EMBER_AF_PRINT_DEBUG, "Cannot find the mode %" PRIu8, mode);
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
return EMBER_ZCL_STATUS_INVALID_VALUE;
}

const ModeSelect::SupportedModesManager * ModeSelect::getSupportedModesManager()
Expand Down
2 changes: 0 additions & 2 deletions examples/common/pigweed/rpc_services/internal/StatusUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ constexpr pw::Status EmberStatusToPwStatus(EmberAfStatus ember_status)
{
case EMBER_ZCL_STATUS_SUCCESS:
return pw::OkStatus();
case EMBER_ZCL_STATUS_INVALID_ARGUMENT:
return pw::Status::InvalidArgument();
case EMBER_ZCL_STATUS_NOT_FOUND:
return pw::Status::NotFound();
case EMBER_ZCL_STATUS_NOT_AUTHORIZED:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ EmberAfStatus ExampleOTARequestor::HandleAnnounceOTAProvider(
auto & providerLocation = commandData.providerLocation;
auto & announcementReason = commandData.announcementReason;

if (commandObj == nullptr || commandObj->GetExchangeContext() == nullptr)
if (commandObj == nullptr)
{
ChipLogError(SoftwareUpdate, "Cannot access ExchangeContext for FabricIndex");
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
ChipLogError(SoftwareUpdate, "Cannot access get FabricIndex");
return EMBER_ZCL_STATUS_INVALID_COMMAND;
}

mProviderNodeId = providerLocation;
mProviderFabricIndex = commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex();
mProviderFabricIndex = commandObj->GetAccessingFabricIndex();

ChipLogProgress(SoftwareUpdate, "OTA Requestor received AnnounceOTAProvider");
ChipLogDetail(SoftwareUpdate, " FabricIndex: %" PRIu8, mProviderFabricIndex);
Expand All @@ -103,7 +103,7 @@ EmberAfStatus ExampleOTARequestor::HandleAnnounceOTAProvider(
break;
default:
ChipLogError(SoftwareUpdate, "Unexpected announcementReason: %" PRIu8, static_cast<uint8_t>(announcementReason));
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
return EMBER_ZCL_STATUS_INVALID_COMMAND;
}

chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(msToStart), StartDelayTimerHandler, this);
Expand Down
8 changes: 4 additions & 4 deletions src/app/clusters/ota-provider/ota-provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback(
if (updateToken.size() > kUpdateTokenMaxLength || updateToken.size() < kUpdateTokenMinLength)
{
ChipLogError(Zcl, "expected size %zu for UpdateToken, got %zu", kUpdateTokenMaxLength, updateToken.size());
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_COMMAND);
return true;
}

Expand Down Expand Up @@ -131,7 +131,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterNotifyUpdateAppliedCallback(
if (updateToken.size() > kUpdateTokenMaxLength || updateToken.size() < kUpdateTokenMinLength)
{
ChipLogError(Zcl, "expected size %zu for UpdateToken, got %zu", kUpdateTokenMaxLength, updateToken.size());
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_COMMAND);
return true;
}

Expand Down Expand Up @@ -215,14 +215,14 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl
if (location.HasValue() && location.Value().size() != kLocationLen)
{
ChipLogError(Zcl, "location param length %zu != expected length %zu", location.Value().size(), kLocationLen);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_COMMAND);
return true;
}

if (metadataForProvider.HasValue() && metadataForProvider.Value().size() > kMaxMetadataLen)
{
ChipLogError(Zcl, "metadata size %zu exceeds max %zu", metadataForProvider.Value().size(), kMaxMetadataLen);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_COMMAND);
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ bool emberAfTestClusterClusterTestAddArgumentsCallback(CommandHandler * apComman
{
if (commandData.arg1 > UINT8_MAX - commandData.arg2)
{
return emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
return emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_COMMAND);
}

TestAddArgumentsResponse::Type responseData;
Expand Down
10 changes: 5 additions & 5 deletions src/app/clusters/thermostat-server/thermostat-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co
else
{
ChipLogError(Zcl, "Error: Could Not adjust heating setpoint to maintain dead band!");
status = EMBER_ZCL_STATUS_INVALID_ARGUMENT;
status = EMBER_ZCL_STATUS_INVALID_COMMAND;
}
}
else
Expand All @@ -710,7 +710,7 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co
ChipLogError(Zcl, "Error: GetOccupiedCoolingSetpoint failed!");
}
else
status = EMBER_ZCL_STATUS_INVALID_ARGUMENT;
status = EMBER_ZCL_STATUS_INVALID_COMMAND;
break;

case EMBER_ZCL_SETPOINT_ADJUST_MODE_HEAT_SETPOINT:
Expand Down Expand Up @@ -745,7 +745,7 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co
else
{
ChipLogError(Zcl, "Error: Could Not adjust cooling setpoint to maintain dead band!");
status = EMBER_ZCL_STATUS_INVALID_ARGUMENT;
status = EMBER_ZCL_STATUS_INVALID_COMMAND;
}
}
else
Expand All @@ -763,11 +763,11 @@ bool emberAfThermostatClusterSetpointRaiseLowerCallback(app::CommandHandler * co
ChipLogError(Zcl, "Error: GetOccupiedHeatingSetpoint failed!");
}
else
status = EMBER_ZCL_STATUS_INVALID_ARGUMENT;
status = EMBER_ZCL_STATUS_INVALID_COMMAND;
break;

default:
status = EMBER_ZCL_STATUS_INVALID_ARGUMENT;
status = EMBER_ZCL_STATUS_INVALID_COMMAND;
break;
}

Expand Down
8 changes: 5 additions & 3 deletions src/app/util/af-enums.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ enum EmberAfStatus : uint8_t
EMBER_ZCL_STATUS_UNSUP_GENERAL_COMMAND = 0x82,
EMBER_ZCL_STATUS_UNSUP_MANUF_CLUSTER_COMMAND = 0x83,
EMBER_ZCL_STATUS_UNSUP_MANUF_GENERAL_COMMAND = 0x84,
EMBER_ZCL_STATUS_INVALID_FIELD = 0x85,
EMBER_ZCL_STATUS_INVALID_COMMAND = 0x85,
EMBER_ZCL_STATUS_INVALID_FIELD = 0x85, // Same as INVALID_COMMAND
EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE = 0x86,
EMBER_ZCL_STATUS_INVALID_VALUE = 0x87,
EMBER_ZCL_STATUS_CONSTRAINT_ERROR = 0x87,
EMBER_ZCL_STATUS_INVALID_VALUE = 0x87, // Same as CONSTRAINT_ERROR
EMBER_ZCL_STATUS_READ_ONLY = 0x88,
EMBER_ZCL_STATUS_INSUFFICIENT_SPACE = 0x89,
EMBER_ZCL_STATUS_DUPLICATE_EXISTS = 0x8A,
Expand All @@ -57,5 +59,5 @@ enum EmberAfStatus : uint8_t
EMBER_ZCL_STATUS_SOFTWARE_FAILURE = 0xC1,
EMBER_ZCL_STATUS_UNSUPPORTED_CLUSTER = 0xC3,
EMBER_ZCL_STATUS_LIMIT_REACHED = 0xC4,
EMBER_ZCL_STATUS_INVALID_ARGUMENT = 0xC6,
EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION = 0xC6,
};
8 changes: 4 additions & 4 deletions src/app/util/error-mapping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ EmberAfStatus ToEmberAfStatus(Protocols::InteractionModel::Status code)
return EMBER_ZCL_STATUS_SUCCESS;
case imcode::NoUpstreamSubscription: // 0xc5
return EMBER_ZCL_STATUS_FAILURE;
case imcode::InvalidArgument: // 0xc6
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
case imcode::NeedsTimedInteraction: // 0xc6
return EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION;
// Default case is omitted intentionally so we can guarantee that we exhaust all of the error codes.
}
return EMBER_ZCL_STATUS_FAILURE;
Expand Down Expand Up @@ -183,8 +183,8 @@ Protocols::InteractionModel::Status ToInteractionModelStatus(EmberAfStatus code)
return imcode::UnsupportedCluster;
case EMBER_ZCL_STATUS_LIMIT_REACHED: // 0xC4
return imcode::Success;
case EMBER_ZCL_STATUS_INVALID_ARGUMENT: // 0xC6
return imcode::InvalidArgument;
case EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION: // 0xC6
return imcode::NeedsTimedInteraction;
default:
return imcode::Failure;
}
Expand Down
7 changes: 5 additions & 2 deletions src/app/util/im-client-callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ void LogStatus(uint8_t status)
case EMBER_ZCL_STATUS_LIMIT_REACHED:
ChipLogProgress(Zcl, " status: EMBER_ZCL_STATUS_LIMIT_REACHED (0x%02x)", status);
break;
case EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION:
ChipLogProgress(Zcl, " status: EMBER_ZCL_STATUS_NEEDS_TIMED_INTERACTION (0x%02x)", status);
break;
default:
ChipLogError(Zcl, "Unknow status: 0x%02x", status);
break;
Expand Down Expand Up @@ -264,8 +267,8 @@ static void LogIMStatus(Protocols::InteractionModel::Status status)
case Protocols::InteractionModel::Status::NoUpstreamSubscription:
ChipLogProgress(Zcl, " status: NoUpstreamSubscription (0x%04" PRIx16 ")", to_underlying(status));
break;
case Protocols::InteractionModel::Status::InvalidArgument:
ChipLogProgress(Zcl, " status: InvalidArgument (0x%04" PRIx16 ")", to_underlying(status));
case Protocols::InteractionModel::Status::NeedsTimedInteraction:
ChipLogProgress(Zcl, " status: NeedsTimedInteraction (0x%04" PRIx16 ")", to_underlying(status));
break;
default:
ChipLogError(Zcl, "Unknown status: 0x%04" PRIx16, to_underlying(status));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
value.SetNull();
return EMBER_ZCL_STATUS_SUCCESS;
{{else}}
return EMBER_ZCL_STATUS_INVALID_VALUE;
return EMBER_ZCL_STATUS_CONSTRAINT_ERROR;
{{/if}}
}
{{#if isNullable}}
auto & span = value.SetNonNull();
{{/if}}
{{~#*inline "value"}}{{#if isNullable}}span{{else}}value{{/if}}{{/inline}}
VerifyOrReturnError({{>value}}.size() == {{maxLength}}, EMBER_ZCL_STATUS_INVALID_ARGUMENT);
VerifyOrReturnError({{>value}}.size() == {{maxLength}}, EMBER_ZCL_STATUS_INVALID_DATA_TYPE);
memcpy({{>value}}.data(), &zclString[{{>sizingBytes}}], {{maxLength}});
{{>value}}.reduce_size(length);
return status;
Expand All @@ -75,7 +75,7 @@ EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
{{else}}
if (!NumericAttributeTraits<{{accessorTraitType type}}>::CanRepresentValue(/* isNullable = */ {{isNullable}}, temp))
{
return EMBER_ZCL_STATUS_INVALID_VALUE;
return EMBER_ZCL_STATUS_CONSTRAINT_ERROR;
}
*value = NumericAttributeTraits<{{accessorTraitType type}}>::StorageToWorking(temp);
{{/if}}
Expand All @@ -88,15 +88,15 @@ EmberAfStatus Set(chip::EndpointId endpoint, {{asUnderlyingZclType type}} value)
{{~#*inline "lengthType"}}uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t{{/inline}}
static_assert({{maxLength}} < NumericAttributeTraits<{{>lengthType}}>::kNullValue,
"value.size() might be too big");
VerifyOrReturnError(value.size() <= {{maxLength}}, EMBER_ZCL_STATUS_INVALID_ARGUMENT);
VerifyOrReturnError(value.size() <= {{maxLength}}, EMBER_ZCL_STATUS_CONSTRAINT_ERROR);
uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
emberAfCopyInt{{#if (isShortString type)}}8{{else}}16{{/if}}u(zclString, 0, static_cast<{{>lengthType}}>(value.size()));
memcpy(&zclString[{{>sizingBytes}}], value.data(), value.size());
return emberAfWriteServerAttribute(endpoint, {{>clusterId}}, Id, zclString, ZCL_{{asDelimitedMacro type}}_ATTRIBUTE_TYPE);
{{else}}
if (!NumericAttributeTraits<{{accessorTraitType type}}>::CanRepresentValue(/* isNullable = */ {{isNullable}}, value))
{
return EMBER_ZCL_STATUS_INVALID_ARGUMENT;
return EMBER_ZCL_STATUS_CONSTRAINT_ERROR;
}
NumericAttributeTraits<{{accessorTraitType type}}>::StorageType storageValue;
NumericAttributeTraits<{{accessorTraitType type}}>::WorkingToStorage(value, storageValue);
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/interaction_model/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ enum class Status : uint16_t
UnsupportedCluster = 0xc3,
Deprecatedc4 = 0xc4,
NoUpstreamSubscription = 0xc5,
InvalidArgument = 0xc6,
NeedsTimedInteraction = 0xc6,
};
} // namespace InteractionModel

Expand Down
Loading

0 comments on commit 3e6174b

Please sign in to comment.