Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename messageId into messageCounter #9688

Merged
merged 1 commit into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,8 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
case CHIP_ERROR_RETRANS_TABLE_FULL.AsInteger():
desc = "Retransmit Table is already full";
break;
case CHIP_ERROR_INVALID_ACK_ID.AsInteger():
desc = "Invalid Acknowledgment Id";
case CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER.AsInteger():
desc = "Invalid acknowledged message counter";
break;
case CHIP_ERROR_SEND_THROTTLED.AsInteger():
desc = "Sending to peer is throttled on this Exchange";
Expand Down
6 changes: 3 additions & 3 deletions src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -1316,13 +1316,13 @@ using CHIP_ERROR = ::chip::ChipError;
#define CHIP_ERROR_RETRANS_TABLE_FULL CHIP_CORE_ERROR(0x64)

/**
* @def CHIP_ERROR_INVALID_ACK_ID
* @def CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER
*
* @brief
* An acknowledgment id is invalid.
*
*/
#define CHIP_ERROR_INVALID_ACK_ID CHIP_CORE_ERROR(0x65)
#define CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER CHIP_CORE_ERROR(0x65)

/**
* @def CHIP_ERROR_SEND_THROTTLED
Expand Down Expand Up @@ -2177,7 +2177,7 @@ using CHIP_ERROR = ::chip::ChipError;
* @def CHIP_ERROR_MESSAGE_ID_OUT_OF_WINDOW
*
* @brief
* The message id of the received message is out of receiving window
* The message counter of the received message is out of receiving window
*/
#define CHIP_ERROR_MESSAGE_ID_OUT_OF_WINDOW CHIP_CORE_ERROR(0xc7)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup: Should change the name of this error too?


Expand Down
2 changes: 1 addition & 1 deletion src/lib/core/tests/TestCHIPErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ static const CHIP_ERROR kTestElements[] =
CHIP_ERROR_DEVICE_AUTH_TIMEOUT,
CHIP_ERROR_MESSAGE_NOT_ACKNOWLEDGED,
CHIP_ERROR_RETRANS_TABLE_FULL,
CHIP_ERROR_INVALID_ACK_ID,
CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER,
CHIP_ERROR_SEND_THROTTLED,
CHIP_ERROR_WRONG_MSG_VERSION_FOR_EXCHANGE,
CHIP_ERROR_TRANSACTION_CANCELED,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/support/CHIPFaultInjection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ DLL_EXPORT void FuzzExchangeHeader(uint8_t * p, int32_t arg)
1, // MessageType
2, // ExchangeId
4, // ProfileId
8 // AckMsgId
8 // AckMessageCounter
};
const uint8_t values[CHIP_FAULT_INJECTION_NUM_FUZZ_VALUES] = { 0x1, 0x2, 0xFF };
size_t offsetIndex = 0;
Expand Down
10 changes: 6 additions & 4 deletions src/messaging/ExchangeMessageDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SessionHandle session, uint16_t
// If there is a pending acknowledgment piggyback it on this message.
if (reliableMessageContext->IsAckPending())
{
payloadHeader.SetAckId(reliableMessageContext->TakePendingPeerAckId());
payloadHeader.SetAckMessageCounter(reliableMessageContext->TakePendingPeerAckMessageCounter());

#if !defined(NDEBUG)
if (!payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck))
{
ChipLogDetail(ExchangeManager, "Piggybacking Ack for MsgId:%08" PRIX32 " with msg", payloadHeader.GetAckId().Value());
ChipLogDetail(ExchangeManager, "Piggybacking Ack for MessageCounter:%08" PRIX32 " with msg",
payloadHeader.GetAckMessageCounter().Value());
}
#endif
}
Expand Down Expand Up @@ -103,9 +104,10 @@ CHIP_ERROR ExchangeMessageDispatch::OnMessageReceived(uint32_t messageCounter, c

if (IsReliableTransmissionAllowed())
{
if (!msgFlags.Has(MessageFlagValues::kDuplicateMessage) && payloadHeader.IsAckMsg() && payloadHeader.GetAckId().HasValue())
if (!msgFlags.Has(MessageFlagValues::kDuplicateMessage) && payloadHeader.IsAckMsg() &&
payloadHeader.GetAckMessageCounter().HasValue())
{
ReturnErrorOnFailure(reliableMessageContext->HandleRcvdAck(payloadHeader.GetAckId().Value()));
ReturnErrorOnFailure(reliableMessageContext->HandleRcvdAck(payloadHeader.GetAckMessageCounter().Value()));
}

if (payloadHeader.NeedsAck())
Expand Down
4 changes: 2 additions & 2 deletions src/messaging/ExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
}

// Matched ExchangeContext; send to message handler.
ec->HandleMessage(packetHeader.GetMessageId(), payloadHeader, source, msgFlags, std::move(msgBuf));
ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, source, msgFlags, std::move(msgBuf));
found = true;
return false;
}
Expand Down Expand Up @@ -297,7 +297,7 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
return;
}

CHIP_ERROR err = ec->HandleMessage(packetHeader.GetMessageId(), payloadHeader, source, msgFlags, std::move(msgBuf));
CHIP_ERROR err = ec->HandleMessage(packetHeader.GetMessageCounter(), payloadHeader, source, msgFlags, std::move(msgBuf));
if (err != CHIP_NO_ERROR)
{
// Using same error message for all errors to reduce code size.
Expand Down
2 changes: 1 addition & 1 deletion src/messaging/Flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ enum class MessageFlagValues : uint32_t
/**< Indicates that the message is a duplicate of a previously received message. */
kDuplicateMessage = 0x00004000,
/**< Indicates that the peer's group key message counter is not synchronized. */
kPeerGroupMsgIdNotSynchronized = 0x00008000,
kPeerGroupMessageCounterNotSynchronized = 0x00008000,
/**< Indicates that the source of the message is the initiator of the CHIP exchange. */
kFromInitiator = 0x00010000,
/**< Indicates that message is being sent/received via the local ephemeral UDP port. */
Expand Down
63 changes: 32 additions & 31 deletions src/messaging/ReliableMessageContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace chip {
namespace Messaging {

ReliableMessageContext::ReliableMessageContext() :
mConfig(gDefaultReliableMessageProtocolConfig), mNextAckTimeTick(0), mPendingPeerAckId(0)
mConfig(gDefaultReliableMessageProtocolConfig), mNextAckTimeTick(0), mPendingPeerAckMessageCounter(0)
{}

void ReliableMessageContext::RetainContext()
Expand Down Expand Up @@ -125,7 +125,7 @@ CHIP_ERROR ReliableMessageContext::FlushAcks()
if (err == CHIP_NO_ERROR)
{
#if !defined(NDEBUG)
ChipLogDetail(ExchangeManager, "Flushed pending ack for MsgId:%08" PRIX32, mPendingPeerAckId);
ChipLogDetail(ExchangeManager, "Flushed pending ack for MessageCounter:%08" PRIX32, mPendingPeerAckMessageCounter);
#endif
}
}
Expand All @@ -150,36 +150,37 @@ uint64_t ReliableMessageContext::GetActiveRetransmitTimeoutTick()
* @note
* This message is part of the CHIP Reliable Messaging protocol.
*
* @param[in] AckMsgId The msgId of incoming Ack message.
* @param[in] ackMessageCounter The messageCounter of incoming Ack message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the "Acknowledged Message Counter" of the incoming message, not the "Message Counter", right?

Pre-existing; please either do a followup to fix this documentation to be less confusing or let me know when this merges and I will.

*
* @retval #CHIP_ERROR_INVALID_ACK_ID if the msgId of received Ack is not in the RetransTable.
* @retval #CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER if the messageCounter of received Ack is not in the
* RetransTable.
* @retval #CHIP_NO_ERROR if the context was removed.
*
*/
CHIP_ERROR ReliableMessageContext::HandleRcvdAck(uint32_t AckMsgId)
CHIP_ERROR ReliableMessageContext::HandleRcvdAck(uint32_t ackMessageCounter)
{
CHIP_ERROR err = CHIP_NO_ERROR;

// Msg is an Ack; Check Retrans Table and remove message context
if (!GetReliableMessageMgr()->CheckAndRemRetransTable(this, AckMsgId))
if (!GetReliableMessageMgr()->CheckAndRemRetransTable(this, ackMessageCounter))
{
#if !defined(NDEBUG)
ChipLogError(ExchangeManager, "CHIP MsgId:%08" PRIX32 " not in RetransTable", AckMsgId);
ChipLogError(ExchangeManager, "CHIP MessageCounter:%08" PRIX32 " not in RetransTable", ackMessageCounter);
#endif
err = CHIP_ERROR_INVALID_ACK_ID;
err = CHIP_ERROR_INVALID_ACK_MESSAGE_COUNTER;
// Optionally call an application callback with this error.
}
else
{
#if !defined(NDEBUG)
ChipLogDetail(ExchangeManager, "Removed CHIP MsgId:%08" PRIX32 " from RetransTable", AckMsgId);
ChipLogDetail(ExchangeManager, "Removed CHIP MessageCounter:%08" PRIX32 " from RetransTable", ackMessageCounter);
#endif
}

return err;
}

CHIP_ERROR ReliableMessageContext::HandleNeedsAck(uint32_t messageId, BitFlags<MessageFlagValues> messageFlags)
CHIP_ERROR ReliableMessageContext::HandleNeedsAck(uint32_t messageCounter, BitFlags<MessageFlagValues> messageFlags)

{
// Skip processing ack if drop ack debug is enabled.
Expand All @@ -189,41 +190,41 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAck(uint32_t messageId, BitFlags<M
// Expire any virtual ticks that have expired so all wakeup sources reflect the current time
GetReliableMessageMgr()->ExpireTicks();

CHIP_ERROR err = HandleNeedsAckInner(messageId, messageFlags);
CHIP_ERROR err = HandleNeedsAckInner(messageCounter, messageFlags);

// Schedule next physical wakeup on function exit
GetReliableMessageMgr()->StartTimer();

return err;
}

CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageId, BitFlags<MessageFlagValues> messageFlags)
CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageCounter, BitFlags<MessageFlagValues> messageFlags)

{
// If the message IS a duplicate there will never be a response to it, so we
// should not wait for one and just immediately send a standalone ack.
if (messageFlags.Has(MessageFlagValues::kDuplicateMessage))
{
#if !defined(NDEBUG)
ChipLogDetail(ExchangeManager, "Forcing tx of solitary ack for duplicate MsgId:%08" PRIX32, messageId);
ChipLogDetail(ExchangeManager, "Forcing tx of solitary ack for duplicate MessageCounter:%08" PRIX32, messageCounter);
#endif
// Is there pending ack for a different message id.
bool wasAckPending = IsAckPending() && mPendingPeerAckId != messageId;
// Is there pending ack for a different message counter.
bool wasAckPending = IsAckPending() && mPendingPeerAckMessageCounter != messageCounter;

// Temporary store currently pending ack id (even if there is none).
uint32_t tempAckId = mPendingPeerAckId;
// Temporary store currently pending ack message counter (even if there is none).
uint32_t tempAckMessageCounter = mPendingPeerAckMessageCounter;

// Set the pending ack id.
SetPendingPeerAckId(messageId);
// Set the pending ack message counter.
SetPendingPeerAckMessageCounter(messageCounter);

// Send the Ack for the duplication message in a SecureChannel::StandaloneAck message.
CHIP_ERROR err = SendStandaloneAckMessage();

// If there was pending ack for a different message id.
// If there was pending ack for a different message counter.
if (wasAckPending)
{
// Restore previously pending ack id.
SetPendingPeerAckId(tempAckId);
// Restore previously pending ack message counter.
SetPendingPeerAckMessageCounter(tempAckMessageCounter);
}

return err;
Expand All @@ -234,15 +235,15 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageId, BitFl
if (IsAckPending())
{
#if !defined(NDEBUG)
ChipLogDetail(ExchangeManager, "Pending ack queue full; forcing tx of solitary ack for MsgId:%08" PRIX32,
mPendingPeerAckId);
ChipLogDetail(ExchangeManager, "Pending ack queue full; forcing tx of solitary ack for MessageCounter:%08" PRIX32,
mPendingPeerAckMessageCounter);
#endif
// Send the Ack for the currently pending Ack in a SecureChannel::StandaloneAck message.
ReturnErrorOnFailure(SendStandaloneAckMessage());
}

// Replace the Pending ack id.
SetPendingPeerAckId(messageId);
// Replace the Pending ack message counter.
SetPendingPeerAckMessageCounter(messageCounter);
mNextAckTimeTick =
static_cast<uint16_t>(CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK +
GetReliableMessageMgr()->GetTickCounterFromTimeDelta(System::Clock::GetMonotonicMilliseconds()));
Expand All @@ -261,7 +262,7 @@ CHIP_ERROR ReliableMessageContext::SendStandaloneAckMessage()

// Send the null message
#if !defined(NDEBUG)
ChipLogDetail(ExchangeManager, "Sending Standalone Ack for MsgId:%08" PRIX32, mPendingPeerAckId);
ChipLogDetail(ExchangeManager, "Sending Standalone Ack for MessageCounter:%08" PRIX32, mPendingPeerAckMessageCounter);
#endif

CHIP_ERROR err = GetExchangeContext()->SendMessage(Protocols::SecureChannel::MsgType::StandaloneAck, std::move(msgBuf),
Expand All @@ -273,16 +274,16 @@ CHIP_ERROR ReliableMessageContext::SendStandaloneAckMessage()
}
if (err != CHIP_NO_ERROR)
{
ChipLogError(ExchangeManager, "Failed to send Solitary ack for MsgId:%08" PRIX32 ":%" CHIP_ERROR_FORMAT, mPendingPeerAckId,
err.Format());
ChipLogError(ExchangeManager, "Failed to send Solitary ack for MessageCounter:%08" PRIX32 ":%" CHIP_ERROR_FORMAT,
mPendingPeerAckMessageCounter, err.Format());
}

return err;
}

void ReliableMessageContext::SetPendingPeerAckId(uint32_t aPeerAckId)
void ReliableMessageContext::SetPendingPeerAckMessageCounter(uint32_t aPeerAckMessageCounter)
{
mPendingPeerAckId = aPeerAckId;
mPendingPeerAckMessageCounter = aPeerAckMessageCounter;
SetAckPending(true);
}

Expand Down
18 changes: 9 additions & 9 deletions src/messaging/ReliableMessageContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ class ReliableMessageContext
CHIP_ERROR FlushAcks();

/**
* Take the pending peer ack id from the context. This must only be called
* Take the pending peer ack message counter from the context. This must only be called
* when IsAckPending() is true. After this call, IsAckPending() will be
* false; it's the caller's responsibility to send the ack.
*/
uint32_t TakePendingPeerAckId()
uint32_t TakePendingPeerAckMessageCounter()
{
SetAckPending(false);
return mPendingPeerAckId;
return mPendingPeerAckMessageCounter;
}

/**
Expand Down Expand Up @@ -217,9 +217,9 @@ class ReliableMessageContext
private:
void RetainContext();
void ReleaseContext();
CHIP_ERROR HandleRcvdAck(uint32_t AckMsgId);
CHIP_ERROR HandleNeedsAck(uint32_t messageId, BitFlags<MessageFlagValues> messageFlags);
CHIP_ERROR HandleNeedsAckInner(uint32_t messageId, BitFlags<MessageFlagValues> messageFlags);
CHIP_ERROR HandleRcvdAck(uint32_t ackMessageCounter);
CHIP_ERROR HandleNeedsAck(uint32_t messageCounter, BitFlags<MessageFlagValues> messageFlags);
CHIP_ERROR HandleNeedsAckInner(uint32_t messageCounter, BitFlags<MessageFlagValues> messageFlags);
ExchangeContext * GetExchangeContext();

/**
Expand All @@ -231,9 +231,9 @@ class ReliableMessageContext
*/
void SetAckPending(bool inAckPending);

// Set our pending peer ack id and any other state needed to ensure that we
// Set our pending peer ack message counter and any other state needed to ensure that we
// will send that ack at some point.
void SetPendingPeerAckId(uint32_t aPeerAckId);
void SetPendingPeerAckMessageCounter(uint32_t aPeerAckMessageCounter);

private:
friend class ReliableMessageMgr;
Expand All @@ -242,7 +242,7 @@ class ReliableMessageContext

ReliableMessageProtocolConfig mConfig;
uint16_t mNextAckTimeTick; // Next time for triggering Solo Ack
uint32_t mPendingPeerAckId;
uint32_t mPendingPeerAckMessageCounter;
};

} // namespace Messaging
Expand Down
Loading