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

Remove duplicated send flag defines and put ExchangeMgr/ExchangeConte… #3994

Merged
merged 2 commits into from
Dec 1, 2020
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/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 @@ -246,11 +248,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