From 10673301e4c38ff8644301e86f2586d3be76d937 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 11 Jan 2022 17:00:30 -0500 Subject: [PATCH] Change InteractionModel::Status to be 8-bit. (#13463) This change is happening on the spec side in https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/4814 --- src/app/CommandSender.cpp | 2 +- src/app/StatusResponse.cpp | 2 +- src/app/tests/TestCommandInteraction.cpp | 2 +- src/app/util/im-client-callbacks.cpp | 84 +++++++++---------- .../python/chip/interaction_model/Delegate.h | 6 +- .../python/chip/interaction_model/delegate.py | 5 +- src/protocols/interaction_model/Constants.h | 2 +- 7 files changed, 53 insertions(+), 50 deletions(-) diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index c9174d78b62eaa..e3b82bfa09cd60 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -282,7 +282,7 @@ CHIP_ERROR CommandSender::ProcessInvokeResponseIB(InvokeResponseIB::Parser & aIn { ChipLogProgress(DataManagement, "Received Command Response Status for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI - " Command=" ChipLogFormatMEI " Status=0x%" PRIx16, + " Command=" ChipLogFormatMEI " Status=0x%" PRIx8, endpointId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId), to_underlying(statusIB.mStatus)); } diff --git a/src/app/StatusResponse.cpp b/src/app/StatusResponse.cpp index f6f5335b50de79..3ae368137a90a7 100644 --- a/src/app/StatusResponse.cpp +++ b/src/app/StatusResponse.cpp @@ -54,7 +54,7 @@ CHIP_ERROR StatusResponse::ProcessStatusResponse(System::PacketBufferHandle && a ReturnErrorOnFailure(response.CheckSchemaValidity()); #endif ReturnErrorOnFailure(response.GetStatus(aStatus.mStatus)); - ChipLogProgress(InteractionModel, "Received status response, status is %" PRIu16, to_underlying(aStatus.mStatus)); + ChipLogProgress(InteractionModel, "Received status response, status is %" PRIu8, to_underlying(aStatus.mStatus)); if (aStatus.mStatus == Protocols::InteractionModel::Status::Success) { diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 23a54ecb993aca..2af13c597ce36e 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -126,7 +126,7 @@ class MockCommandSenderCallback : public CommandSender::Callback } void OnError(const chip::app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus, CHIP_ERROR aError) override { - ChipLogError(Controller, "OnError happens with %" PRIx16 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus.mStatus), + ChipLogError(Controller, "OnError happens with %" PRIx8 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus.mStatus), aError.Format()); onErrorCalledTimes++; } diff --git a/src/app/util/im-client-callbacks.cpp b/src/app/util/im-client-callbacks.cpp index ec50dd0fb31e05..c9e6985722962a 100644 --- a/src/app/util/im-client-callbacks.cpp +++ b/src/app/util/im-client-callbacks.cpp @@ -145,130 +145,130 @@ static void LogIMStatus(Protocols::InteractionModel::Status status) switch (status) { case Protocols::InteractionModel::Status::Success: - ChipLogProgress(Zcl, " status: Success (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Success (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Failure: - ChipLogProgress(Zcl, " status: Failure (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Failure (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::InvalidSubscription: - ChipLogProgress(Zcl, " status: InvalidSubscription (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: InvalidSubscription (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedAccess: - ChipLogProgress(Zcl, " status: UnsupportedAccess (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedAccess (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedEndpoint: - ChipLogProgress(Zcl, " status: UnsupportedEndpoint (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedEndpoint (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::InvalidAction: - ChipLogProgress(Zcl, " status: InvalidAction (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: InvalidAction (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedCommand: - ChipLogProgress(Zcl, " status: UnsupportedCommand (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedCommand (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated82: - ChipLogProgress(Zcl, " status: Deprecated82 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated82 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated83: - ChipLogProgress(Zcl, " status: Deprecated83 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated83 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated84: - ChipLogProgress(Zcl, " status: Deprecated84 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated84 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::InvalidCommand: - ChipLogProgress(Zcl, " status: InvalidCommand (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: InvalidCommand (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedAttribute: - ChipLogProgress(Zcl, " status: UnsupportedAttribute (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedAttribute (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::InvalidValue: - ChipLogProgress(Zcl, " status: InvalidValue (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: InvalidValue (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedWrite: - ChipLogProgress(Zcl, " status: UnsupportedWrite (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedWrite (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::ResourceExhausted: - ChipLogProgress(Zcl, " status: ResourceExhausted (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: ResourceExhausted (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated8a: - ChipLogProgress(Zcl, " status: Deprecated8a (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated8a (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::NotFound: - ChipLogProgress(Zcl, " status: NotFound (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: NotFound (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnreportableAttribute: - ChipLogProgress(Zcl, " status: UnreportableAttribute (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnreportableAttribute (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::InvalidDataType: - ChipLogProgress(Zcl, " status: InvalidDataType (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: InvalidDataType (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated8e: - ChipLogProgress(Zcl, " status: Deprecated8e (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated8e (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedRead: - ChipLogProgress(Zcl, " status: UnsupportedRead (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedRead (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated90: - ChipLogProgress(Zcl, " status: Deprecated90 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated90 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated91: - ChipLogProgress(Zcl, " status: Deprecated91 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated91 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved92: - ChipLogProgress(Zcl, " status: Reserved92 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved92 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecated93: - ChipLogProgress(Zcl, " status: Deprecated93 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecated93 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Timeout: - ChipLogProgress(Zcl, " status: Timeout (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Timeout (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved95: - ChipLogProgress(Zcl, " status: Reserved95 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved95 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved96: - ChipLogProgress(Zcl, " status: Reserved96 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved96 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved97: - ChipLogProgress(Zcl, " status: Reserved97 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved97 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved98: - ChipLogProgress(Zcl, " status: Reserved98 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved98 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved99: - ChipLogProgress(Zcl, " status: Reserved99 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved99 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Reserved9a: - ChipLogProgress(Zcl, " status: Reserved9a (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Reserved9a (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::ConstraintError: - ChipLogProgress(Zcl, " status: ConstraintError (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: ConstraintError (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Busy: - ChipLogProgress(Zcl, " status: Busy (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Busy (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecatedc0: - ChipLogProgress(Zcl, " status: Deprecatedc0 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecatedc0 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecatedc1: - ChipLogProgress(Zcl, " status: Deprecatedc1 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecatedc1 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecatedc2: - ChipLogProgress(Zcl, " status: Deprecatedc2 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecatedc2 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::UnsupportedCluster: - ChipLogProgress(Zcl, " status: UnsupportedCluster (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: UnsupportedCluster (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::Deprecatedc4: - ChipLogProgress(Zcl, " status: Deprecatedc4 (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: Deprecatedc4 (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::NoUpstreamSubscription: - ChipLogProgress(Zcl, " status: NoUpstreamSubscription (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: NoUpstreamSubscription (0x%02" PRIx8 ")", to_underlying(status)); break; case Protocols::InteractionModel::Status::NeedsTimedInteraction: - ChipLogProgress(Zcl, " status: NeedsTimedInteraction (0x%04" PRIx16 ")", to_underlying(status)); + ChipLogProgress(Zcl, " status: NeedsTimedInteraction (0x%02" PRIx8 ")", to_underlying(status)); break; default: - ChipLogError(Zcl, "Unknown status: 0x%04" PRIx16, to_underlying(status)); + ChipLogError(Zcl, "Unknown status: 0x%02" PRIx8, to_underlying(status)); break; } } diff --git a/src/controller/python/chip/interaction_model/Delegate.h b/src/controller/python/chip/interaction_model/Delegate.h index 4f878e764680e6..9807e320562a38 100644 --- a/src/controller/python/chip/interaction_model/Delegate.h +++ b/src/controller/python/chip/interaction_model/Delegate.h @@ -30,6 +30,7 @@ namespace Controller { // The command status will be used for python script. // use packed attribute so we can unpack it from python and no need to worry about padding. +// This struct needs to match the IMCommandStatus definition in delegate.py struct __attribute__((packed)) CommandStatus { Protocols::InteractionModel::Status status; @@ -43,7 +44,7 @@ struct __attribute__((packed)) CommandStatus static_assert(std::is_same::value && std::is_same::value && std::is_same::value, "Members in CommandStatus does not match interaction_model/delegate.py"); -static_assert(sizeof(CommandStatus) == 2 + 1 + 2 + 4 + 4 + 1, "Size of CommandStatus might contain padding"); +static_assert(sizeof(CommandStatus) == 1 + 1 + 2 + 4 + 4 + 1, "Size of CommandStatus might contain padding"); struct __attribute__((packed)) AttributePath { @@ -57,6 +58,7 @@ static_assert(std::is_same::value && std::is_same::value && std::is_same::value && std::is_same::value, "Members in AttributeWriteStatus does not match interaction_model/delegate.py"); -static_assert(sizeof(AttributeWriteStatus) == 8 + 8 + 2 + 2 + 4 + 4, "Size of AttributeWriteStatus might contain padding"); +static_assert(sizeof(AttributeWriteStatus) == 8 + 8 + 1 + 2 + 4 + 4, "Size of AttributeWriteStatus might contain padding"); extern "C" { typedef void (*PythonInteractionModelDelegate_OnCommandResponseStatusCodeReceivedFunct)(uint64_t commandSenderPtr, diff --git a/src/controller/python/chip/interaction_model/delegate.py b/src/controller/python/chip/interaction_model/delegate.py index 106c1a4a310888..6ad6bd5a5785d4 100644 --- a/src/controller/python/chip/interaction_model/delegate.py +++ b/src/controller/python/chip/interaction_model/delegate.py @@ -28,7 +28,7 @@ # The type should match CommandStatus in interaction_model/Delegate.h # CommandStatus should not contain padding IMCommandStatus = Struct( - "Status" / Int16ul, + "Status" / Int8ul, "ClusterStatus" / Int8ul, "EndpointId" / Int16ul, "ClusterId" / Int32ul, @@ -36,10 +36,11 @@ "CommandIndex" / Int8ul, ) +# The type should match WriteStatus in interaction_model/Delegate.h IMWriteStatus = Struct( "NodeId" / Int64ul, "AppIdentifier" / Int64ul, - "Status" / Int16ul, + "Status" / Int8ul, "EndpointId" / Int16ul, "ClusterId" / Int32ul, "AttributeId" / Int32ul, diff --git a/src/protocols/interaction_model/Constants.h b/src/protocols/interaction_model/Constants.h index b662b47324a876..5be5cb36a7df7f 100644 --- a/src/protocols/interaction_model/Constants.h +++ b/src/protocols/interaction_model/Constants.h @@ -66,7 +66,7 @@ enum class MsgType : uint8_t }; // This table comes from the IM's "Status Code Table" section from the Interaction Model spec. -enum class Status : uint16_t +enum class Status : uint8_t { Success = 0x0, Failure = 0x01,