Skip to content

Commit

Permalink
Add Compare IPAddress in discoveredNode
Browse files Browse the repository at this point in the history
  • Loading branch information
@joonhaengHeo
joonhaengHeo committed May 24, 2024
1 parent c3ef110 commit f3736b9
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 34 deletions.
29 changes: 26 additions & 3 deletions src/controller/AbstractDnssdDiscoveryController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,31 @@
namespace chip {
namespace Controller {

bool compareIpAddresses(const size_t sourceNumIPs, const size_t destinationNumIPs, const Inet::IPAddress *source, const Inet::IPAddress *destination)
{
size_t sameIpAddress = 0;
bool addressUsed[chip::Dnssd::CommonResolutionData::kMaxIPAddresses] = {false};
if (sourceNumIPs != destinationNumIPs)
{
return false;
}

for (size_t s = 0 ; s < sourceNumIPs ; s++)
{
for (size_t d = 0 ; d < destinationNumIPs ; d++)
{
if (!addressUsed[d] && source[s] == destination[d])
{
// Change the user flag so that the compared target is no longer used
addressUsed[d] = true;
sameIpAddress++;
break;
}
}
}
return sameIpAddress == destinationNumIPs;
}

void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & discNodeData)
{
VerifyOrReturn(discNodeData.Is<chip::Dnssd::CommissionNodeData>());
Expand All @@ -38,10 +63,8 @@ void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::Disco
{
continue;
}
// TODO(#32576) Check if IP address are the same. Must account for `numIPs` in the list of `ipAddress`.
// Additionally, must NOT assume that the ordering is consistent.
if (strcmp(discoveredNode.hostName, nodeData.hostName) == 0 && discoveredNode.port == nodeData.port &&
discoveredNode.numIPs == nodeData.numIPs)
compareIpAddresses(discoveredNode.numIPs, nodeData.numIPs, discoveredNode.ipAddress, nodeData.ipAddress))
{
discoveredNode = nodeData;
if (mDeviceDiscoveryDelegate != nullptr)
Expand Down
102 changes: 102 additions & 0 deletions src/controller/tests/TestCommissionableNodeController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,106 @@ TEST_F(TestCommissionableNodeController, TestDiscoverCommissioners_DiscoverCommi
EXPECT_NE(controller.DiscoverCommissioners(), CHIP_NO_ERROR);
}

TEST_F(TestCommissionableNodeController, TestGetDiscoveredCommissioner_MultipleIPAddressDiscover)
{
MockResolver resolver;
CommissionableNodeController controller(&resolver);

// example 1
chip::Dnssd::DiscoveredNodeData discNodeData1;
discNodeData1.Set<chip::Dnssd::CommissionNodeData>();
chip::Dnssd::CommissionNodeData & inNodeData1 = discNodeData1.Get<chip::Dnssd::CommissionNodeData>();
Platform::CopyString(inNodeData1.hostName, "mockHostName");
Inet::IPAddress::FromString("fd11:1111:1122:2222:1111:2222:3333:4444", inNodeData1.ipAddress[0]);
inNodeData1.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:5555", inNodeData1.ipAddress[1]);
inNodeData1.numIPs++;
inNodeData1.port = 5540;

controller.OnNodeDiscovered(discNodeData1);

// example 5 - exactly same as example 1
chip::Dnssd::DiscoveredNodeData discNodeData5;
discNodeData5.Set<chip::Dnssd::CommissionNodeData>();
chip::Dnssd::CommissionNodeData & inNodeData5 = discNodeData5.Get<chip::Dnssd::CommissionNodeData>();
Platform::CopyString(inNodeData1.hostName, "mockHostName");
Inet::IPAddress::FromString("fd11:1111:1122:2222:1111:2222:3333:4444", inNodeData5.ipAddress[0]);
inNodeData5.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:5555", inNodeData5.ipAddress[1]);
inNodeData5.numIPs++;
inNodeData5.port = 5540;

controller.OnNodeDiscovered(discNodeData5);

// example 2 - same as example 1 (IPAdress sequence is only different.)
chip::Dnssd::DiscoveredNodeData discNodeData2;
discNodeData2.Set<chip::Dnssd::CommissionNodeData>();
chip::Dnssd::CommissionNodeData & inNodeData2 = discNodeData2.Get<chip::Dnssd::CommissionNodeData>();
Platform::CopyString(inNodeData2.hostName, "mockHostName");
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:5555", inNodeData2.ipAddress[0]);
inNodeData2.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:1111:2222:3333:4444", inNodeData2.ipAddress[1]);
inNodeData2.numIPs++;
inNodeData2.port = 5540;

controller.OnNodeDiscovered(discNodeData2);

// example 3 - different example
chip::Dnssd::DiscoveredNodeData discNodeData3;
discNodeData3.Set<chip::Dnssd::CommissionNodeData>();
chip::Dnssd::CommissionNodeData & inNodeData3 = discNodeData3.Get<chip::Dnssd::CommissionNodeData>();
Platform::CopyString(inNodeData3.hostName, "mockHostName");
Inet::IPAddress::FromString("fd11:1111:1122:2222:1111:2222:3333:4444", inNodeData3.ipAddress[0]);
inNodeData3.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:6666", inNodeData3.ipAddress[1]);
inNodeData3.numIPs++;
inNodeData3.port = 5540;

controller.OnNodeDiscovered(discNodeData3);

// example 4 - different example (Different IP count)
chip::Dnssd::DiscoveredNodeData discNodeData4;
discNodeData4.Set<chip::Dnssd::CommissionNodeData>();
chip::Dnssd::CommissionNodeData & inNodeData4 = discNodeData4.Get<chip::Dnssd::CommissionNodeData>();
Platform::CopyString(inNodeData4.hostName, "mockHostName");
Inet::IPAddress::FromString("fd11:1111:1122:2222:1111:2222:3333:4444", inNodeData4.ipAddress[0]);
inNodeData4.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:6666", inNodeData4.ipAddress[1]);
inNodeData4.numIPs++;
Inet::IPAddress::FromString("fd11:1111:1122:2222:2222:3333:4444:7777", inNodeData4.ipAddress[2]);
inNodeData4.numIPs++;
inNodeData4.port = 5540;

controller.OnNodeDiscovered(discNodeData4);


// Example 2 result - example 1 is removed. (reason : duplicate)
ASSERT_NE(controller.GetDiscoveredCommissioner(0), nullptr);
EXPECT_STREQ(inNodeData1.hostName, controller.GetDiscoveredCommissioner(0)->hostName);
EXPECT_EQ(inNodeData2.ipAddress[0], controller.GetDiscoveredCommissioner(0)->ipAddress[0]);
EXPECT_EQ(inNodeData2.ipAddress[1], controller.GetDiscoveredCommissioner(0)->ipAddress[1]);
EXPECT_EQ(controller.GetDiscoveredCommissioner(0)->port, 5540);
EXPECT_EQ(controller.GetDiscoveredCommissioner(0)->numIPs, 2u);

// Example 3 result
ASSERT_NE(controller.GetDiscoveredCommissioner(1), nullptr);
EXPECT_STREQ(inNodeData3.hostName, controller.GetDiscoveredCommissioner(1)->hostName);
EXPECT_EQ(inNodeData3.ipAddress[0], controller.GetDiscoveredCommissioner(1)->ipAddress[0]);
EXPECT_EQ(inNodeData3.ipAddress[1], controller.GetDiscoveredCommissioner(1)->ipAddress[1]);
EXPECT_EQ(controller.GetDiscoveredCommissioner(1)->port, 5540);
EXPECT_EQ(controller.GetDiscoveredCommissioner(1)->numIPs, 2u);

// Example 3 result
ASSERT_NE(controller.GetDiscoveredCommissioner(2), nullptr);
EXPECT_STREQ(inNodeData4.hostName, controller.GetDiscoveredCommissioner(2)->hostName);
EXPECT_EQ(inNodeData4.ipAddress[0], controller.GetDiscoveredCommissioner(2)->ipAddress[0]);
EXPECT_EQ(inNodeData4.ipAddress[1], controller.GetDiscoveredCommissioner(2)->ipAddress[1]);
EXPECT_EQ(inNodeData4.ipAddress[2], controller.GetDiscoveredCommissioner(2)->ipAddress[2]);
EXPECT_EQ(controller.GetDiscoveredCommissioner(2)->port, 5540);
EXPECT_EQ(controller.GetDiscoveredCommissioner(2)->numIPs, 3u);

// Total is 3. (Not 4)
ASSERT_EQ(controller.GetDiscoveredCommissioner(3), nullptr);
}

} // namespace
4 changes: 3 additions & 1 deletion src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ public void onStopDiscoveryFailed(String serviceType, int errorCode) {
@Override
public void onDiscoveryStopped(String serviceType) {
Log.w(TAG, "Successfully stopped discovery service '" + serviceType);
this.handleServiceBrowse(chipMdnsCallback);
new Handler(Looper.getMainLooper()).post(() -> {
this.handleServiceBrowse(chipMdnsCallback);
});
}

public void handleServiceBrowse(ChipMdnsCallback chipMdnsCallback) {
Expand Down
68 changes: 38 additions & 30 deletions src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import android.net.nsd.NsdManager;
import android.net.nsd.NsdServiceInfo;
import android.net.wifi.WifiManager.MulticastLock;
import android.os.Handler;
import android.os.Looper;
import android.util.Log;
import androidx.annotation.Nullable;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -118,21 +120,22 @@ public void onResolveFailed(NsdServiceInfo serviceInfo, int errorCode) {
Log.w(
TAG,
"Failed to resolve service '" + serviceInfo.getServiceName() + "': " + errorCode);
chipMdnsCallback.handleServiceResolve(
serviceInfo.getServiceName(),
// Use the target service info since the resolved service info sometimes appends a
// "." at the front likely because it is trying to strip the service name out of it
// and something is missed.
// The target service info service type should be effectively the same as the
// resolved service info.
NsdServiceFinderAndResolver.this.targetServiceInfo.getServiceType(),
null,
null,
0,
null,
callbackHandle,
contextHandle);

new Handler(Looper.getMainLooper()).post(() -> {
chipMdnsCallback.handleServiceResolve(
serviceInfo.getServiceName(),
// Use the target service info since the resolved service info sometimes appends a
// "." at the front likely because it is trying to strip the service name out of it
// and something is missed.
// The target service info service type should be effectively the same as the
// resolved service info.
NsdServiceFinderAndResolver.this.targetServiceInfo.getServiceType(),
null,
null,
0,
null,
callbackHandle,
contextHandle);
});
if (multicastLock.isHeld()) {
multicastLock.release();

Expand All @@ -153,22 +156,27 @@ public void onServiceResolved(NsdServiceInfo serviceInfo) {
+ serviceInfo.getHost()
+ ", type : "
+ serviceInfo.getServiceType());
// TODO: Find out if DNS-SD results for Android should contain interface ID
chipMdnsCallback.handleServiceResolve(
serviceInfo.getServiceName(),
// Use the target service info since the resolved service info sometimes appends a
// "." at the front likely because it is trying to strip the service name out of it
// and something is missed.
// The target service info service type should be effectively the same as the
// resolved service info.
NsdServiceFinderAndResolver.this.targetServiceInfo.getServiceType(),
serviceInfo.getHost().getHostName(),
serviceInfo.getHost().getHostAddress(),
serviceInfo.getPort(),
serviceInfo.getAttributes(),
callbackHandle,
contextHandle);

final String hostName = serviceInfo.getHost().getHostName();
final String address = serviceInfo.getHost().getHostAddress();
final int port = serviceInfo.getPort();
new Handler(Looper.getMainLooper()).post(() -> {
// TODO: Find out if DNS-SD results for Android should contain interface ID
chipMdnsCallback.handleServiceResolve(
serviceInfo.getServiceName(),
// Use the target service info since the resolved service info sometimes appends a
// "." at the front likely because it is trying to strip the service name out of it
// and something is missed.
// The target service info service type should be effectively the same as the
// resolved service info.
NsdServiceFinderAndResolver.this.targetServiceInfo.getServiceType(),
hostName,
address,
port,
serviceInfo.getAttributes(),
callbackHandle,
contextHandle);
});
if (multicastLock.isHeld()) {
multicastLock.release();

Expand Down

0 comments on commit f3736b9

Please sign in to comment.