From 311e439067e7e0c8b1ef0a6c91a994a77a5cb6e5 Mon Sep 17 00:00:00 2001 From: Leonard Hansen Date: Wed, 6 Mar 2024 09:31:54 +0100 Subject: [PATCH 1/6] Log nodeID in exchange context --- src/lib/support/logging/TextOnlyLogging.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/lib/support/logging/TextOnlyLogging.h b/src/lib/support/logging/TextOnlyLogging.h index d6b22f7c6c5a6b..d7b6dcf0dad647 100644 --- a/src/lib/support/logging/TextOnlyLogging.h +++ b/src/lib/support/logging/TextOnlyLogging.h @@ -276,8 +276,14 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co */ #define ChipLogFormatExchangeId "%u%c" #define ChipLogValueExchangeId(id, isInitiator) id, ((isInitiator) ? 'i' : 'r') -#define ChipLogFormatExchange ChipLogFormatExchangeId -#define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()) +#define ChipLogFormateExchangeNode "[%u:" ChipLogFormatX64 "]" +#define ChipLogValueExchangeNode(fabric, id) fabric, ChipLogValueX64(id) +#define ChipLogFormatExchange ChipLogFormatExchangeId " to: " ChipLogFormateExchangeNode +#define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()), \ + ChipLogValueExchangeNode( \ + ((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetSubjectDescriptor().fabricIndex : kUndefinedFabricIndex), \ + ((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetSubjectDescriptor().subject : kUndefinedNodeId)) + #define ChipLogValueExchangeIdFromSentHeader(payloadHeader) \ ChipLogValueExchangeId((payloadHeader).GetExchangeID(), (payloadHeader).IsInitiator()) // A received header's initiator boolean is the inverse of the exchange's. From 5cffd1b74f398a5fe98d0b5f196c17491c7c9446 Mon Sep 17 00:00:00 2001 From: Leonard Hansen Date: Tue, 12 Mar 2024 19:58:17 +0100 Subject: [PATCH 2/6] Update comments --- src/lib/support/logging/TextOnlyLogging.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/support/logging/TextOnlyLogging.h b/src/lib/support/logging/TextOnlyLogging.h index d7b6dcf0dad647..1bda7547870c39 100644 --- a/src/lib/support/logging/TextOnlyLogging.h +++ b/src/lib/support/logging/TextOnlyLogging.h @@ -268,9 +268,8 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co #define ChipLogValueMEI(aValue) static_cast(aValue >> 16), static_cast(aValue) /** - * Logging helpers for exchanges. For now just log the exchange id and whether - * it's an initiator or responder, but eventually we may want to log the peer - * node id as well (especially for the responder case). Some callsites only + * Logging helpers for exchanges. Log the exchange id, whether + * it's an initiator or responder and the peer node id. Some callsites only * have the exchange id and initiator/responder boolean, not an actual exchange, * so we want to have a helper for that case too. */ From 01c39749096e84abc3d10785df3e84ccde6d71d1 Mon Sep 17 00:00:00 2001 From: Leonard Hansen Date: Wed, 13 Mar 2024 15:52:26 +0100 Subject: [PATCH 3/6] Use ChipLogValueScopedNodeId with ScopedNodeId instead of SubjectDescriptor --- src/lib/support/logging/TextOnlyLogging.h | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/lib/support/logging/TextOnlyLogging.h b/src/lib/support/logging/TextOnlyLogging.h index 1bda7547870c39..9d431e0838b4e3 100644 --- a/src/lib/support/logging/TextOnlyLogging.h +++ b/src/lib/support/logging/TextOnlyLogging.h @@ -269,19 +269,15 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co /** * Logging helpers for exchanges. Log the exchange id, whether - * it's an initiator or responder and the peer node id. Some callsites only + * it's an initiator or responder and the scoped node. Some callsites only * have the exchange id and initiator/responder boolean, not an actual exchange, * so we want to have a helper for that case too. */ #define ChipLogFormatExchangeId "%u%c" #define ChipLogValueExchangeId(id, isInitiator) id, ((isInitiator) ? 'i' : 'r') -#define ChipLogFormateExchangeNode "[%u:" ChipLogFormatX64 "]" -#define ChipLogValueExchangeNode(fabric, id) fabric, ChipLogValueX64(id) -#define ChipLogFormatExchange ChipLogFormatExchangeId " to: " ChipLogFormateExchangeNode +#define ChipLogFormatExchange ChipLogFormatExchangeId " " ChipLogFormatScopedNodeId #define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()), \ - ChipLogValueExchangeNode( \ - ((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetSubjectDescriptor().fabricIndex : kUndefinedFabricIndex), \ - ((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetSubjectDescriptor().subject : kUndefinedNodeId)) + ChipLogValueScopedNodeId((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetPeer() : ScopedNodeId()) #define ChipLogValueExchangeIdFromSentHeader(payloadHeader) \ ChipLogValueExchangeId((payloadHeader).GetExchangeID(), (payloadHeader).IsInitiator()) From 47cc3389a6d0de0d6ba6f6a7effa8705f676b3af Mon Sep 17 00:00:00 2001 From: Leonard Hansen Date: Wed, 13 Mar 2024 21:17:53 +0100 Subject: [PATCH 4/6] Make node id logging optional --- src/lib/core/CHIPConfig.h | 10 ++++++++++ src/lib/support/logging/TextOnlyLogging.h | 8 +++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 2de6a662ee8c4d..8eb950e1e24e00 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -461,6 +461,16 @@ #define CHIP_CONFIG_ENABLE_CONDITION_LOGGING 0 #endif // CHIP_CONFIG_ENABLE_CONDITION_LOGGING +/** + * @def CHIP_EXCHANGE_NODE_ID_LOGGING + * + * @brief + * If asserted (1), enable logging of node IDs in exchange context. + */ +#ifndef CHIP_EXCHANGE_NODE_ID_LOGGING +#define CHIP_EXCHANGE_NODE_ID_LOGGING 0 +#endif // CHIP_EXCHANGE_NODE_ID_LOGGING + /** * @def CHIP_CONFIG_TEST * diff --git a/src/lib/support/logging/TextOnlyLogging.h b/src/lib/support/logging/TextOnlyLogging.h index 9d431e0838b4e3..4e8fb9fb4752dc 100644 --- a/src/lib/support/logging/TextOnlyLogging.h +++ b/src/lib/support/logging/TextOnlyLogging.h @@ -275,9 +275,15 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co */ #define ChipLogFormatExchangeId "%u%c" #define ChipLogValueExchangeId(id, isInitiator) id, ((isInitiator) ? 'i' : 'r') -#define ChipLogFormatExchange ChipLogFormatExchangeId " " ChipLogFormatScopedNodeId + +#if CHIP_EXCHANGE_NODE_ID_LOGGING +#define ChipLogFormatExchange ChipLogFormatExchangeId " with Node: " ChipLogFormatScopedNodeId #define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()), \ ChipLogValueScopedNodeId((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetPeer() : ScopedNodeId()) +#else // CHIP_EXCHANGE_NODE_ID_LOGGING +#define ChipLogFormatExchange ChipLogFormatExchangeId +#define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()) +#endif // CHIP_EXCHANGE_NODE_ID_LOGGING #define ChipLogValueExchangeIdFromSentHeader(payloadHeader) \ ChipLogValueExchangeId((payloadHeader).GetExchangeID(), (payloadHeader).IsInitiator()) From 3cc87776c5b16f05a35342001bb4303b50befbee Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 13 Mar 2024 20:18:24 +0000 Subject: [PATCH 5/6] Restyled by whitespace --- src/lib/core/CHIPConfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 8eb950e1e24e00..558e0ee08e4605 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -463,7 +463,7 @@ /** * @def CHIP_EXCHANGE_NODE_ID_LOGGING - * + * * @brief * If asserted (1), enable logging of node IDs in exchange context. */ From b7b74763595957acaa18c0c8b510ce56d1b36096 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 13 Mar 2024 20:18:26 +0000 Subject: [PATCH 6/6] Restyled by clang-format --- src/lib/support/logging/TextOnlyLogging.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/support/logging/TextOnlyLogging.h b/src/lib/support/logging/TextOnlyLogging.h index 4e8fb9fb4752dc..b0abef9d00e722 100644 --- a/src/lib/support/logging/TextOnlyLogging.h +++ b/src/lib/support/logging/TextOnlyLogging.h @@ -278,8 +278,9 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co #if CHIP_EXCHANGE_NODE_ID_LOGGING #define ChipLogFormatExchange ChipLogFormatExchangeId " with Node: " ChipLogFormatScopedNodeId -#define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()), \ - ChipLogValueScopedNodeId((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetPeer() : ScopedNodeId()) +#define ChipLogValueExchange(ec) \ + ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator()), \ + ChipLogValueScopedNodeId((ec)->HasSessionHandle() ? (ec)->GetSessionHandle()->GetPeer() : ScopedNodeId()) #else // CHIP_EXCHANGE_NODE_ID_LOGGING #define ChipLogFormatExchange ChipLogFormatExchangeId #define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator())