-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add a Transport::TCP class for tcp-based communication. #2689
Changes from 35 commits
a3ece7d
5a64e0f
80c881b
aace8a8
1d6c6b9
57cdb9c
5c98783
4fefbb2
a6c8f9e
a46519e
bc911d6
443add7
8e7f6ac
be7ad2c
1a1425f
ef4a42c
10fc27e
3d7a4c7
517235a
ed3cb92
6b66de4
8b9d993
be3d830
0e4819f
628359f
14cdba1
53d8fd9
6278737
9958256
e3662fb
975c34b
acabcc3
7c019c7
6086423
65b37fd
c67cb08
8083d21
287b5cd
a8637f0
1b60cbc
3b27f33
7f5dfa4
ab3202a
9d9e445
73e12e8
da64988
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,7 @@ enum class Type | |
kUndefined, | ||
kUdp, | ||
kBle, | ||
// More constants to be added later, such as TCP | ||
kTcp, | ||
}; | ||
|
||
/** | ||
|
@@ -57,6 +57,7 @@ enum class Type | |
class PeerAddress | ||
{ | ||
public: | ||
PeerAddress() : mIPAddress(Inet::IPAddress::Any), mTransportType(Type::kUndefined), mInterface(INET_NULL_INTERFACEID) {} | ||
PeerAddress(const Inet::IPAddress & addr, Type type) : mIPAddress(addr), mTransportType(type), mInterface(INET_NULL_INTERFACEID) | ||
{} | ||
PeerAddress(Type type) : mTransportType(type) {} | ||
|
@@ -96,11 +97,15 @@ class PeerAddress | |
|
||
bool IsInitialized() const { return mTransportType != Type::kUndefined; } | ||
|
||
bool operator==(const PeerAddress & other) | ||
bool operator==(const PeerAddress & other) const | ||
{ | ||
return (mTransportType == other.mTransportType) && (mIPAddress == other.mIPAddress) && (mPort == other.mPort); | ||
return (mTransportType == other.mTransportType) && (mIPAddress == other.mIPAddress) && (mPort == other.mPort) && | ||
((mInterface == INET_NULL_INTERFACEID) || (other.mInterface == INET_NULL_INTERFACEID) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we had missed the inteface compare before. But it seems to make sense to me to have null be treated as 'any' for a compare. If we do not like that, we need more thought into how TCP determines 'an active connection already exists'. Likely peer communication still needs some thought - we can likely compare IP address however port can be anything since it is random on connection establishment on one side. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this field should be matched exactly. We shouldn't understand equality of addresses to mean "could these addresses send to the same destination" and we couldn't answer that question in general. It should just mean "these addresses are identical", including zone identifier. Weaker matches can be separate functions, not equality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, updated. |
||
(mInterface == other.mInterface)); | ||
} | ||
|
||
bool operator!=(const PeerAddress & other) const { return !(*this == other); } | ||
|
||
/// Maximum size of an Inet address ToString format, that can hold both IPV6 and IPV4 addresses. | ||
#ifdef INET6_ADDRSTRLEN | ||
static constexpr size_t kInetMaxAddrLen = INET6_ADDRSTRLEN; | ||
|
@@ -129,6 +134,14 @@ class PeerAddress | |
mIPAddress.ToString(ip_addr, sizeof(ip_addr)); | ||
snprintf(buf, bufSize, "UDP:%s:%d", ip_addr, mPort); | ||
break; | ||
case Type::kTcp: | ||
mIPAddress.ToString(ip_addr, sizeof(ip_addr)); | ||
snprintf(buf, bufSize, "TCP:%s:%d", ip_addr, mPort); | ||
break; | ||
case Type::kBle: | ||
// Note that BLE does not currently use any specific address. | ||
snprintf(buf, bufSize, "BLE"); | ||
break; | ||
default: | ||
snprintf(buf, bufSize, "ERROR"); | ||
break; | ||
|
@@ -146,12 +159,14 @@ class PeerAddress | |
{ | ||
return UDP(addr).SetPort(port).SetInterface(interface); | ||
} | ||
static PeerAddress TCP(const Inet::IPAddress & addr) { return PeerAddress(addr, Type::kTcp); } | ||
static PeerAddress TCP(const Inet::IPAddress & addr, uint16_t port) { return TCP(addr).SetPort(port); } | ||
|
||
private: | ||
Inet::IPAddress mIPAddress; | ||
Type mTransportType; | ||
uint16_t mPort = CHIP_PORT; ///< Relevant for UDP data sending. | ||
Inet::InterfaceId mInterface; | ||
uint16_t mPort = CHIP_PORT; ///< Relevant for UDP data sending. | ||
Inet::InterfaceId mInterface = INET_NULL_INTERFACEID; | ||
}; | ||
|
||
} // namespace Transport | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maintain this comment or is TCP the last constant we plan to add?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not aware of anything else we could add as chip spec. I believe a comment would be 'add more stuff here if needed' however that seems redundant as any enum would be 'add more if needed'.