Skip to content

Commit

Permalink
Remove duplicated send flag defines and put ExchangeMgr/ExchangeConte… (
Browse files Browse the repository at this point in the history
#3994)

* Remove duplicated send flag defines and put ExchangeMgr/ExchangeContext under the same namespace as CRMP

* Rename kSendFlag_Default to kSendFlag_None
  • Loading branch information
yufengwangca authored Dec 1, 2020
1 parent dedd15f commit 01031e7
Show file tree
Hide file tree
Showing 19 changed files with 89 additions and 69 deletions.
4 changes: 2 additions & 2 deletions src/messaging/ErrorCategory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#include <system/SystemError.h>

namespace chip {
namespace messaging {
namespace Messaging {

bool IsIgnoredMulticastSendError(CHIP_ERROR err)
{
Expand Down Expand Up @@ -81,5 +81,5 @@ bool IsSendErrorNonCritical(CHIP_ERROR err)
err == INET_ERROR_MESSAGE_TOO_LONG || err == INET_ERROR_NO_MEMORY || CHIP_CONFIG_IsPlatformErrorNonCritical(err));
}

} // namespace messaging
} // namespace Messaging
} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/ErrorCategory.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@
#include <support/DLLUtil.h>

namespace chip {
namespace messaging {
namespace Messaging {

CHIP_ERROR FilterUDPSendError(CHIP_ERROR err, bool isMulticast);
bool IsIgnoredMulticastSendError(CHIP_ERROR err);
bool IsSendErrorNonCritical(CHIP_ERROR err);

} // namespace messaging
} // namespace Messaging
} // namespace chip
9 changes: 6 additions & 3 deletions src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ using namespace chip::Inet;
using namespace chip::System;

namespace chip {
namespace Messaging {

static void DefaultOnMessageReceived(ExchangeContext * ec, const PacketHeader & packetHeader, uint32_t protocolId, uint8_t msgType,
PacketBufferHandle payload)
Expand All @@ -68,7 +69,7 @@ void ExchangeContext::SetResponseExpected(bool inResponseExpected)
mFlags.Set(ExFlagValues::kFlagResponseExpected, inResponseExpected);
}

CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, PacketBuffer * msgBuf, uint16_t sendFlags,
CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, PacketBuffer * msgBuf, const SendFlags & sendFlags,
void * msgCtxt)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -92,7 +93,7 @@ CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, Pa
payloadHeader.SetMessageType(msgType);

// If a response message is expected...
if ((sendFlags & kSendFlag_ExpectResponse) != 0)
if (sendFlags.Has(SendMessageFlags::kSendFlag_ExpectResponse))
{
// Only one 'response expected' message can be outstanding at a time.
VerifyOrExit(!IsResponseExpected(), err = CHIP_ERROR_INCORRECT_STATE);
Expand All @@ -119,7 +120,8 @@ CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, Pa
CancelResponseTimer();
SetResponseExpected(false);
}
if (msgBuf != nullptr && (sendFlags & kSendFlag_RetainBuffer) == 0)

if (msgBuf != nullptr && !sendFlags.Has(SendMessageFlags::kSendFlag_RetainBuffer))
{
PacketBuffer::Free(msgBuf);
}
Expand Down Expand Up @@ -332,4 +334,5 @@ CHIP_ERROR ExchangeContext::HandleMessage(const PacketHeader & packetHeader, con
return err;
}

} // namespace Messaging
} // namespace chip
11 changes: 4 additions & 7 deletions src/messaging/ExchangeContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@

#include <lib/core/ReferenceCounted.h>
#include <messaging/ExchangeDelegate.h>
#include <messaging/Flags.h>
#include <support/BitFlags.h>
#include <support/DLLUtil.h>
#include <system/SystemTimer.h>
#include <transport/SecureSessionMgr.h>

namespace chip {
namespace Messaging {

class ExchangeManager;
class ExchangeContext;
Expand All @@ -53,12 +55,6 @@ class DLL_EXPORT ExchangeContext : public ReferenceCounted<ExchangeContext, Exch
friend class ExchangeContextDeletor;

public:
enum
{
kSendFlag_ExpectResponse = 0x0001, // Used to indicate that a response is expected within a specified timeout.
kSendFlag_RetainBuffer = 0x0002, // Used to indicate that the message buffer should not be freed after sending.
};

/**
* Determine whether the context is the initiator of the exchange.
*
Expand Down Expand Up @@ -108,7 +104,7 @@ class DLL_EXPORT ExchangeContext : public ReferenceCounted<ExchangeContext, Exch
* @retval #CHIP_NO_ERROR if the CHIP layer successfully sent the message down to the
* network layer.
*/
CHIP_ERROR SendMessage(uint16_t protocolId, uint8_t msgType, System::PacketBuffer * msgPayload, uint16_t sendFlags = 0,
CHIP_ERROR SendMessage(uint16_t protocolId, uint8_t msgType, System::PacketBuffer * msgPayload, const SendFlags & sendFlags,
void * msgCtxt = nullptr);

/**
Expand Down Expand Up @@ -192,4 +188,5 @@ inline void ExchangeContextDeletor::Release(ExchangeContext * obj)
obj->Free();
}

} // namespace Messaging
} // namespace chip
2 changes: 2 additions & 0 deletions src/messaging/ExchangeDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace Messaging {

class ExchangeContext;

Expand Down Expand Up @@ -74,4 +75,5 @@ class DLL_EXPORT ExchangeDelegate
virtual void OnExchangeClosing(ExchangeContext * ec) {}
};

} // namespace Messaging
} // namespace chip
2 changes: 2 additions & 0 deletions src/messaging/ExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ using namespace chip::Inet;
using namespace chip::System;

namespace chip {
namespace Messaging {

/**
* Constructor for the ExchangeManager class.
Expand Down Expand Up @@ -312,4 +313,5 @@ void ExchangeManager::DecrementContextsInUse()
}
}

} // namespace Messaging
} // namespace chip
2 changes: 2 additions & 0 deletions src/messaging/ExchangeMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <transport/SecureSessionMgr.h>

namespace chip {
namespace Messaging {

class ExchangeContext;
class ExchangeDelegate;
Expand Down Expand Up @@ -206,4 +207,5 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate
void OnConnectionExpired(const Transport::PeerConnectionState * state, SecureSessionMgr * mgr) override;
};

} // namespace Messaging
} // namespace chip
30 changes: 18 additions & 12 deletions src/messaging/Flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@
#pragma once

#include <stdint.h>
#include <support/BitFlags.h>

namespace chip {
namespace messaging {
namespace Messaging {

/**
* @brief
Expand All @@ -37,29 +38,32 @@ namespace messaging {
enum class MessageFlagValues : uint32_t
{
/**< Indicates that the existing source node identifier must be reused. */
kChipMessageFlag_ReuseSourceId = 0x00000020,
kMessageFlag_ReuseSourceId = 0x00000020,
/**< Indicates that the sending of the message needs to be delayed. */
kChipMessageFlag_DelaySend = 0x00000040,
kMessageFlag_DelaySend = 0x00000040,
/**< Indicates that the message buffer should not be freed after sending. */
kChipMessageFlag_RetainBuffer = 0x00000080,
kMessageFlag_RetainBuffer = 0x00000080,
/**< Indicates that the CHIP message is already encoded. */
kChipMessageFlag_MessageEncoded = 0x00001000,
kMessageFlag_MessageEncoded = 0x00001000,
/**< Indicates that default IPv6 source address selection should be used when sending IPv6 multicast messages. */
kChipMessageFlag_DefaultMulticastSourceAddress = 0x00002000,
kMessageFlag_DefaultMulticastSourceAddress = 0x00002000,
/**< Indicates that the sender of the message requested an acknowledgment. */
kChipMessageFlag_PeerRequestedAck = 0x00004000,
kMessageFlag_PeerRequestedAck = 0x00004000,
/**< Indicates that the message is a duplicate of a previously received message. */
kChipMessageFlag_DuplicateMessage = 0x00008000,
kMessageFlag_DuplicateMessage = 0x00008000,
/**< Indicates that the peer's group key message counter is not synchronized. */
kChipMessageFlag_PeerGroupMsgIdNotSynchronized = 0x00010000,
kMessageFlag_PeerGroupMsgIdNotSynchronized = 0x00010000,
/**< Indicates that the source of the message is the initiator of the CHIP exchange. */
kChipMessageFlag_FromInitiator = 0x00020000,
kMessageFlag_FromInitiator = 0x00020000,
/**< Indicates that message is being sent/received via the local ephemeral UDP port. */
kChipMessageFlag_ViaEphemeralUDPPort = 0x00040000,
kMessageFlag_ViaEphemeralUDPPort = 0x00040000,
};

using MessageFlags = BitFlags<uint32_t, MessageFlagValues>;

enum class SendMessageFlags : uint16_t
{
kSendFlag_None = 0x0000,
/**< Used to indicate that automatic retransmission is enabled. */
kSendFlag_AutoRetrans = 0x0001,
/**< Used to indicate that a response is expected within a specified timeout. */
Expand All @@ -82,5 +86,7 @@ enum class SendMessageFlags : uint16_t
kSendFlag_NoAutoRequestAck = 0x0800,
};

} // namespace messaging
using SendFlags = BitFlags<uint16_t, SendMessageFlags>;

} // namespace Messaging
} // namespace chip
14 changes: 7 additions & 7 deletions src/messaging/ReliableMessageContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
#include <messaging/ErrorCategory.h>
#include <messaging/Flags.h>
#include <messaging/ReliableMessageManager.h>
#include <protocols/CHIPProtocols.h>
#include <protocols/Protocols.h>
#include <protocols/common/CommonProtocol.h>
#include <support/CodeUtils.h>

namespace chip {
namespace messaging {
namespace Messaging {

void ReliableMessageContextDeletor::Release(ReliableMessageContext * obj)
{
Expand Down Expand Up @@ -221,7 +221,7 @@ CHIP_ERROR ReliableMessageContext::SendThrottleFlow(uint32_t pauseTimeMillis)
// Send a Throttle Flow message to the peer. Throttle Flow messages must never request
// acknowledgment, so suppress the auto-request ACK feature on the exchange in case it has been
// enabled by the application.
err = mManager->SendMessage(this, Protocols::kChipProtocol_Common, Protocols::Common::kMsgType_RMP_Throttle_Flow,
err = mManager->SendMessage(this, Protocols::kProtocol_Protocol_Common, Protocols::Common::kMsgType_RMP_Throttle_Flow,
msgBuf.Release_ForNow(),
BitFlags<uint16_t, SendMessageFlags>(SendMessageFlags::kSendFlag_NoAutoRequestAck));

Expand Down Expand Up @@ -277,7 +277,7 @@ CHIP_ERROR ReliableMessageContext::SendDelayedDelivery(uint32_t pauseTimeMillis,
// Send a Delayed Delivery message to the peer. Delayed Delivery messages must never request
// acknowledgment, so suppress the auto-request ACK feature on the exchange in case it has been
// enabled by the application.
err = mManager->SendMessage(this, Protocols::kChipProtocol_Common, Protocols::Common::kMsgType_RMP_Delayed_Delivery,
err = mManager->SendMessage(this, Protocols::kProtocol_Protocol_Common, Protocols::Common::kMsgType_RMP_Delayed_Delivery,
msgBuf.Release_ForNow(),
BitFlags<uint16_t, SendMessageFlags>{ SendMessageFlags::kSendFlag_NoAutoRequestAck });

Expand Down Expand Up @@ -351,7 +351,7 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAck(uint32_t MessageId, BitFlags<u
mManager->ExpireTicks();

// If the message IS a duplicate.
if (MsgFlags.Has(MessageFlagValues::kChipMessageFlag_DuplicateMessage))
if (MsgFlags.Has(MessageFlagValues::kMessageFlag_DuplicateMessage))
{
#if !defined(NDEBUG)
ChipLogProgress(ExchangeManager, "Forcing tx of solitary ack for duplicate MsgId:%08" PRIX32, MessageId);
Expand Down Expand Up @@ -454,7 +454,7 @@ CHIP_ERROR ReliableMessageContext::SendCommonNullMessage()
VerifyOrExit(!msgBuf.IsNull(), err = CHIP_ERROR_NO_MEMORY);

// Send the null message
err = mManager->SendMessage(this, chip::Protocols::kChipProtocol_Common, chip::Protocols::Common::kMsgType_Null,
err = mManager->SendMessage(this, chip::Protocols::kProtocol_Protocol_Common, chip::Protocols::Common::kMsgType_Null,
msgBuf.Release_ForNow(),
BitFlags<uint16_t, SendMessageFlags>{ SendMessageFlags::kSendFlag_NoAutoRequestAck });

Expand All @@ -472,5 +472,5 @@ CHIP_ERROR ReliableMessageContext::SendCommonNullMessage()
return err;
}

} // namespace messaging
} // namespace Messaging
} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/ReliableMessageContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace messaging {
namespace Messaging {

class ChipMessageInfo;
enum class MessageFlagValues : uint32_t;
Expand Down Expand Up @@ -131,5 +131,5 @@ class ReliableMessageContext : public ReferenceCounted<ReliableMessageContext, R
ReliableMessageDelegate * mDelegate;
};

} // namespace messaging
} // namespace Messaging
} // namespace chip
6 changes: 3 additions & 3 deletions src/messaging/ReliableMessageManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#include <support/logging/CHIPLogging.h>

namespace chip {
namespace messaging {
namespace Messaging {

ReliableMessageManager::RetransTableEntry::RetransTableEntry() :
rc(nullptr), msgBuf(nullptr), msgId(0), msgSendFlags(0), nextRetransTimeTick(0), sendCount(0)
Expand Down Expand Up @@ -419,7 +419,7 @@ CHIP_ERROR ReliableMessageManager::SendFromRetransTable(RetransTableEntry * entr

// Send the message through
uint16_t msgSendFlags = entry->msgSendFlags;
SetFlag(msgSendFlags, MessageFlagValues::kChipMessageFlag_RetainBuffer);
SetFlag(msgSendFlags, MessageFlagValues::kMessageFlag_RetainBuffer);
err = SendMessage(rc, entry->msgBuf, msgSendFlags);

// Reset the msgBuf start pointer and data length after sending
Expand Down Expand Up @@ -642,5 +642,5 @@ int ReliableMessageManager::TestGetCountRetransTable()
return count;
}

} // namespace messaging
} // namespace Messaging
} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/ReliableMessageManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace messaging {
namespace Messaging {

enum class SendMessageFlags : uint16_t;
class ReliableMessageContext;
Expand Down Expand Up @@ -122,5 +122,5 @@ class ReliableMessageManager
RetransTableEntry RetransTable[CHIP_CONFIG_RMP_RETRANS_TABLE_SIZE];
};

} // namespace messaging
} // namespace Messaging
} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/ReliableMessageProtocolConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include <system/SystemConfig.h>

namespace chip {
namespace messaging {
namespace Messaging {

/**
* @def CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT
Expand Down Expand Up @@ -121,5 +121,5 @@ const ReliableMessageProtocolConfig gDefaultReliableMessageProtocolConfig = { CH

// clang-format on

} // namespace messaging
} // namespace Messaging
} // namespace chip
8 changes: 6 additions & 2 deletions src/messaging/tests/TestExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <core/CHIPCore.h>
#include <messaging/ExchangeContext.h>
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <protocols/Protocols.h>
#include <support/CodeUtils.h>
#include <transport/SecureSessionMgr.h>
Expand All @@ -42,6 +43,7 @@ namespace {
using namespace chip;
using namespace chip::Inet;
using namespace chip::Transport;
using namespace chip::Messaging;

using TestContext = chip::Test::IOContext;

Expand Down Expand Up @@ -245,11 +247,13 @@ void CheckExchangeMessages(nlTestSuite * inSuite, void * inContext)
err = exchangeMgr.RegisterUnsolicitedMessageHandler(0x0001, 0x0001, &mockUnsolicitedAppDelegate);

// send a malicious packet
ec1->SendMessage(0x0001, 0x0002, System::PacketBuffer::New().Release_ForNow());
ec1->SendMessage(0x0001, 0x0002, System::PacketBuffer::New().Release_ForNow(),
SendFlags(Messaging::SendMessageFlags::kSendFlag_None));
NL_TEST_ASSERT(inSuite, !mockUnsolicitedAppDelegate.IsOnMessageReceivedCalled);

// send a good packet
ec1->SendMessage(0x0001, 0x0001, System::PacketBuffer::New().Release_ForNow());
ec1->SendMessage(0x0001, 0x0001, System::PacketBuffer::New().Release_ForNow(),
SendFlags(Messaging::SendMessageFlags::kSendFlag_None));
NL_TEST_ASSERT(inSuite, mockUnsolicitedAppDelegate.IsOnMessageReceivedCalled);
}

Expand Down
Loading

0 comments on commit 01031e7

Please sign in to comment.