Skip to content

Commit

Permalink
Add nicer logging for InteractionModel::Status. (project-chip#16855)
Browse files Browse the repository at this point in the history
* Add nicer logging for InteractionModel::Status.

After this change we get things like:

    Error: IM Error 0x000005C6: General error: 0xc6 (NEEDS_TIMED_INTERACTION)

instead of just

    Error: IM Error 0x000005C6: General error: 0xc6

* Address review comment
  • Loading branch information
bzbarsky-apple authored Mar 31, 2022
1 parent 98294c9 commit 7dce058
Show file tree
Hide file tree
Showing 10 changed files with 206 additions and 51 deletions.
21 changes: 16 additions & 5 deletions src/app/MessageDef/StatusIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ CHIP_ERROR StatusIB::Parser::CheckSchemaValidity() const
{
uint16_t status;
ReturnErrorOnFailure(reader.Get(status));
PRETTY_PRINT("\tstatus = 0x%" PRIx16 ",", status);
PRETTY_PRINT("\tstatus = " ChipLogFormatIMStatus ",", ChipLogValueIMStatus(static_cast<Status>(status)));
}
#endif // CHIP_DETAIL_LOGGING
}
Expand Down Expand Up @@ -195,13 +195,24 @@ bool FormatStatusIBError(char * buf, uint16_t bufSize, CHIP_ERROR err)

const char * desc = nullptr;
#if !CHIP_CONFIG_SHORT_ERROR_STR
constexpr char generalFormat[] = "General error: 0x%02x";
constexpr char generalFormat[] = "General error: " ChipLogFormatIMStatus;
constexpr char clusterFormat[] = "Cluster-specific error: 0x%02x";

// Formatting an 8-bit int will take at most 2 chars, and replace the '%02x'
// so a buffer big enough to hold our format string will also hold our
// formatted string.
constexpr size_t formattedSize = max(sizeof(generalFormat), sizeof(clusterFormat));
// formatted string, as long as we account for the possible string formats.
constexpr size_t statusNameMaxLength =
#define CHIP_IM_STATUS_CODE(name, spec_name, value) \
max(sizeof(#spec_name),
#include <protocols/interaction_model/StatusCodeList.h>
#undef CHIP_IM_STATUS_CODE
static_cast<size_t>(0)
#define CHIP_IM_STATUS_CODE(name, spec_name, value) \
)
#include <protocols/interaction_model/StatusCodeList.h>
#undef CHIP_IM_STATUS_CODE
;
constexpr size_t formattedSize = max(sizeof(generalFormat) + statusNameMaxLength, sizeof(clusterFormat));
char formattedString[formattedSize];

StatusIB status;
Expand All @@ -212,7 +223,7 @@ bool FormatStatusIBError(char * buf, uint16_t bufSize, CHIP_ERROR err)
}
else
{
snprintf(formattedString, formattedSize, generalFormat, to_underlying(status.mStatus));
snprintf(formattedString, formattedSize, generalFormat, ChipLogValueIMStatus(status.mStatus));
}
desc = formattedString;
#endif // !CHIP_CONFIG_SHORT_ERROR_STR
Expand Down
6 changes: 6 additions & 0 deletions src/app/tests/TestStatusIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <app/AppBuildConfig.h>
#include <app/MessageDef/StatusIB.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPError.h>
#include <lib/support/CHIPMem.h>
#include <lib/support/ErrorStr.h>
Expand Down Expand Up @@ -96,7 +97,12 @@ void TestStatusIBErrorToString(nlTestSuite * aSuite, void * aContext)
status.mStatus = Status::InvalidAction;
CHIP_ERROR err = status.ToChipError();
const char * str = ErrorStr(err);

#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
NL_TEST_ASSERT(aSuite, strcmp(str, "IM Error 0x00000580: General error: 0x80 (INVALID_ACTION)") == 0);
#else // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
NL_TEST_ASSERT(aSuite, strcmp(str, "IM Error 0x00000580: General error: 0x80") == 0);
#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT

status.mStatus = Status::Failure;
status.mClusterStatus = MakeOptional(static_cast<ClusterStatus>(5));
Expand Down
14 changes: 14 additions & 0 deletions src/lib/core/CHIPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -1799,6 +1799,20 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
#define CHIP_CONFIG_NETWORK_COMMISSIONING_DEBUG_TEXT_BUFFER_SIZE 64
#endif // CHIP_CONFIG_NETWORK_COMMISSIONING_DEBUG_TEXT_BUFFER_SIZE

/**
* @def CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
*
* If 1, IM status codes, when logged, will be formatted as "0xNN (NameOfCode)"
* If 0, IM status codes, when logged, will be formatted as "0xNN" In either
* case, the macro ChipLogFormatIMStatus expands to a suitable printf format
* string, which already includes the '%' in it, to be used with
* ChipLogValueIMStatus(status).
*/

#ifndef CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
#define CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT 0
#endif // CHIP_CONFIG_ERROR_FORMAT_AS_STRING

/**
* @}
*/
2 changes: 2 additions & 0 deletions src/platform/Darwin/CHIPPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

#define CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE 1

#define CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT 1

// ==================== Security Adaptations ====================

// ==================== General Configuration Overrides ====================
Expand Down
2 changes: 2 additions & 0 deletions src/platform/Linux/CHIPPlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ using CHIP_CONFIG_PERSISTED_STORAGE_KEY_TYPE = const char *;

#define CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE 1

#define CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT 1

// ==================== Security Adaptations ====================

// ==================== General Configuration Overrides ====================
Expand Down
2 changes: 2 additions & 0 deletions src/protocols/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ static_library("protocols") {
"echo/Echo.h",
"echo/EchoClient.cpp",
"echo/EchoServer.cpp",
"interaction_model/StatusCode.cpp",
"interaction_model/StatusCode.h",
"secure_channel/MessageCounterManager.cpp",
"secure_channel/MessageCounterManager.h",
"user_directed_commissioning/UDCClientState.h",
Expand Down
47 changes: 1 addition & 46 deletions src/protocols/interaction_model/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <type_traits>

#include <protocols/Protocols.h>
#include <protocols/interaction_model/StatusCode.h>

/**
* @namespace chip::Protocols::InteractionModel
Expand Down Expand Up @@ -65,52 +66,6 @@ enum class MsgType : uint8_t
TimedRequest = 0x0a,
};

// This table comes from the IM's "Status Code Table" section from the Interaction Model spec.
enum class Status : uint8_t
{
Success = 0x0,
Failure = 0x01,
InvalidSubscription = 0x7d,
UnsupportedAccess = 0x7e,
UnsupportedEndpoint = 0x7f,
InvalidAction = 0x80,
UnsupportedCommand = 0x81,
Deprecated82 = 0x82,
Deprecated83 = 0x83,
Deprecated84 = 0x84,
InvalidCommand = 0x85,
UnsupportedAttribute = 0x86,
ConstraintError = 0x87,
InvalidValue = ConstraintError, // Deprecated
UnsupportedWrite = 0x88,
ResourceExhausted = 0x89,
Deprecated8a = 0x8a,
NotFound = 0x8b,
UnreportableAttribute = 0x8c,
InvalidDataType = 0x8d,
Deprecated8e = 0x8e,
UnsupportedRead = 0x8f,
Deprecated90 = 0x90,
Deprecated91 = 0x91,
DataVersionMismatch = 0x92,
Deprecated93 = 0x93,
Timeout = 0x94,
Reserved95 = 0x95,
Reserved96 = 0x96,
Reserved97 = 0x97,
Reserved98 = 0x98,
Reserved99 = 0x99,
Reserved9a = 0x9a,
Busy = 0x9c,
Deprecatedc0 = 0xc0,
Deprecatedc1 = 0xc1,
Deprecatedc2 = 0xc2,
UnsupportedCluster = 0xc3,
Deprecatedc4 = 0xc4,
NoUpstreamSubscription = 0xc5,
NeedsTimedInteraction = 0xc6,
UnsupportedEvent = 0xc7,
};
} // namespace InteractionModel

template <>
Expand Down
43 changes: 43 additions & 0 deletions src/protocols/interaction_model/StatusCode.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <lib/core/CHIPConfig.h>
#include <protocols/interaction_model/StatusCode.h>

namespace chip {
namespace Protocols {
namespace InteractionModel {

#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
const char * StatusName(Status status)
{
switch (status)
{
#define CHIP_IM_STATUS_CODE(name, spec_name, value) \
case Status(value): \
return #spec_name;
#include <protocols/interaction_model/StatusCodeList.h>
#undef CHIP_IM_STATUS_CODE
}

return "Unallocated";
}
#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT

} // namespace InteractionModel
} // namespace Protocols
} // namespace chip
53 changes: 53 additions & 0 deletions src/protocols/interaction_model/StatusCode.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <stdint.h>

#include <lib/core/CHIPConfig.h>
#include <lib/support/TypeTraits.h>

#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
#define ChipLogFormatIMStatus "0x%02x (%s)"
#define ChipLogValueIMStatus(status) chip::to_underlying(status), chip::Protocols::InteractionModel::StatusName(status)
#else // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
#define ChipLogFormatIMStatus "0x%02x"
#define ChipLogValueIMStatus(status) chip::to_underlying(status)
#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT

namespace chip {
namespace Protocols {
namespace InteractionModel {

// This table comes from the IM's "Status Code Table" section from the Interaction Model spec.
enum class Status : uint8_t
{
#define CHIP_IM_STATUS_CODE(name, spec_name, value) name = value,
#include <protocols/interaction_model/StatusCodeList.h>
#undef CHIP_IM_STATUS_CODE

InvalidValue = ConstraintError, // Deprecated
};

#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
const char * StatusName(Status status);
#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT

} // namespace InteractionModel
} // namespace Protocols
} // namespace chip
67 changes: 67 additions & 0 deletions src/protocols/interaction_model/StatusCodeList.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* This file is designed to be included multiple times, hence does not use
* include guards or #pragma once. Consumers must define the
* CHIP_IM_STATUS_CODE(name, spec_name, value) macro to do whatever they want,
* include this file, then undefine the macro.
*/

// clang-format off
CHIP_IM_STATUS_CODE(Success , SUCCESS , 0x0)
CHIP_IM_STATUS_CODE(Failure , FAILURE , 0x01)
CHIP_IM_STATUS_CODE(InvalidSubscription , INVALID_SUBSCRIPTION , 0x7d)
CHIP_IM_STATUS_CODE(UnsupportedAccess , UNSUPPORTED_ACCESS , 0x7e)
CHIP_IM_STATUS_CODE(UnsupportedEndpoint , UNSUPPORTED_ENDPOINT , 0x7f)
CHIP_IM_STATUS_CODE(InvalidAction , INVALID_ACTION , 0x80)
CHIP_IM_STATUS_CODE(UnsupportedCommand , UNSUPPORTED_COMMAND , 0x81)
CHIP_IM_STATUS_CODE(Deprecated82 , Deprecated82 , 0x82)
CHIP_IM_STATUS_CODE(Deprecated83 , Deprecated83 , 0x83)
CHIP_IM_STATUS_CODE(Deprecated84 , Deprecated84 , 0x84)
CHIP_IM_STATUS_CODE(InvalidCommand , INVALID_COMMAND , 0x85)
CHIP_IM_STATUS_CODE(UnsupportedAttribute , UNSUPPORTED_ATTRIBUTE , 0x86)
CHIP_IM_STATUS_CODE(ConstraintError , CONSTRAINT_ERROR , 0x87)
CHIP_IM_STATUS_CODE(UnsupportedWrite , UNSUPPORTED_WRITE , 0x88)
CHIP_IM_STATUS_CODE(ResourceExhausted , RESOURCE_EXHAUSTED , 0x89)
CHIP_IM_STATUS_CODE(Deprecated8a , Deprecated8a , 0x8a)
CHIP_IM_STATUS_CODE(NotFound , NOT_FOUND , 0x8b)
CHIP_IM_STATUS_CODE(UnreportableAttribute , UNREPORTABLE_ATTRIBUTE , 0x8c)
CHIP_IM_STATUS_CODE(InvalidDataType , INVALID_DATA_TYPE , 0x8d)
CHIP_IM_STATUS_CODE(Deprecated8e , Deprecated8e , 0x8e)
CHIP_IM_STATUS_CODE(UnsupportedRead , UNSUPPORTED_READ , 0x8f)
CHIP_IM_STATUS_CODE(Deprecated90 , Deprecated90 , 0x90)
CHIP_IM_STATUS_CODE(Deprecated91 , Deprecated91 , 0x91)
CHIP_IM_STATUS_CODE(DataVersionMismatch , DATA_VERSION_MISMATCH , 0x92)
CHIP_IM_STATUS_CODE(Deprecated93 , Deprecated93 , 0x93)
CHIP_IM_STATUS_CODE(Timeout , TIMEOUT , 0x94)
CHIP_IM_STATUS_CODE(Reserved95 , Reserved95 , 0x95)
CHIP_IM_STATUS_CODE(Reserved96 , Reserved96 , 0x96)
CHIP_IM_STATUS_CODE(Reserved97 , Reserved97 , 0x97)
CHIP_IM_STATUS_CODE(Reserved98 , Reserved98 , 0x98)
CHIP_IM_STATUS_CODE(Reserved99 , Reserved99 , 0x99)
CHIP_IM_STATUS_CODE(Reserved9a , Reserved9a , 0x9a)
CHIP_IM_STATUS_CODE(Busy , BUSY , 0x9c)
CHIP_IM_STATUS_CODE(Deprecatedc0 , Deprecatedc0 , 0xc0)
CHIP_IM_STATUS_CODE(Deprecatedc1 , Deprecatedc1 , 0xc1)
CHIP_IM_STATUS_CODE(Deprecatedc2 , Deprecatedc2 , 0xc2)
CHIP_IM_STATUS_CODE(UnsupportedCluster , UNSUPPORTED_CLUSTER , 0xc3)
CHIP_IM_STATUS_CODE(Deprecatedc4 , Deprecatedc4 , 0xc4)
CHIP_IM_STATUS_CODE(NoUpstreamSubscription, NO_UPSTREAM_SUBSCRIPTION, 0xc5)
CHIP_IM_STATUS_CODE(NeedsTimedInteraction , NEEDS_TIMED_INTERACTION , 0xc6)
CHIP_IM_STATUS_CODE(UnsupportedEvent , UNSUPPORTED_EVENT , 0xc7)
// clang-format on

0 comments on commit 7dce058

Please sign in to comment.