[Android] Fix discover Commissionable Device API issue when discovered multiple devices (#32459)
* Fix Android Commissionable Device API when searched multiple devices
* Fix Android Commissionable Device API when searched multiple devices
* Fix from comment, fix edge case issue
* Restyled by google-java-format
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
index 6372b14..f8686ce 100644
--- a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
+++ b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
@@ -74,7 +74,10 @@
AndroidBleManager(context),
PreferencesKeyValueStoreManager(context),
PreferencesConfigurationManager(context),
- NsdManagerServiceResolver(context),
+ NsdManagerServiceResolver(
+ context,
+ NsdManagerServiceResolver.NsdManagerResolverAvailState()
+ ),
NsdManagerServiceBrowser(context),
ChipMdnsCallbackImpl(),
DiagnosticDataProviderImpl(context)
diff --git a/src/controller/AbstractDnssdDiscoveryController.cpp b/src/controller/AbstractDnssdDiscoveryController.cpp
index d0f6d5d..2443c5f 100644
--- a/src/controller/AbstractDnssdDiscoveryController.cpp
+++ b/src/controller/AbstractDnssdDiscoveryController.cpp
@@ -35,7 +35,8 @@
continue;
}
if (strcmp(discoveredNode.resolutionData.hostName, nodeData.resolutionData.hostName) == 0 &&
- discoveredNode.resolutionData.port == nodeData.resolutionData.port)
+ discoveredNode.resolutionData.port == nodeData.resolutionData.port &&
+ discoveredNode.resolutionData.ipAddress == nodeData.resolutionData.ipAddress)
{
discoveredNode = nodeData;
if (mDeviceDiscoveryDelegate != nullptr)
diff --git a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
index 3532182..f98acdf 100644
--- a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
+++ b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
@@ -34,10 +34,19 @@
private final NsdManager nsdManager;
private MulticastLock multicastLock;
private Handler mainThreadHandler;
+ private final long timeout;
private HashMap<Long, NsdManagerDiscovery> callbackMap;
public NsdManagerServiceBrowser(Context context) {
+ this(context, BROWSE_SERVICE_TIMEOUT);
+ }
+
+ /**
+ * @param context application context
+ * @param timeout Timeout value in case there is no response after calling browse
+ */
+ public NsdManagerServiceBrowser(Context context, long timeout) {
this.nsdManager = (NsdManager) context.getSystemService(Context.NSD_SERVICE);
this.mainThreadHandler = new Handler(Looper.getMainLooper());
@@ -46,6 +55,7 @@
.createMulticastLock("chipBrowseMulticastLock");
this.multicastLock.setReferenceCounted(true);
callbackMap = new HashMap<>();
+ this.timeout = timeout;
}
@Override
@@ -62,7 +72,7 @@
}
};
startDiscover(serviceType, callbackHandle, contextHandle, chipMdnsCallback);
- mainThreadHandler.postDelayed(timeoutRunnable, BROWSE_SERVICE_TIMEOUT);
+ mainThreadHandler.postDelayed(timeoutRunnable, timeout);
}
public void startDiscover(
diff --git a/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java b/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java
index c018551..1f1a9ef 100644
--- a/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java
+++ b/src/platform/android/java/chip/platform/NsdManagerServiceResolver.java
@@ -22,13 +22,14 @@
import android.net.nsd.NsdServiceInfo;
import android.net.wifi.WifiManager;
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.ArrayList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -38,30 +39,38 @@
private static final long RESOLVE_SERVICE_TIMEOUT = 30000;
private final NsdManager nsdManager;
private MulticastLock multicastLock;
- private Handler mainThreadHandler;
private List<NsdManager.RegistrationListener> registrationListeners = new ArrayList<>();
private final CopyOnWriteArrayList<String> mMFServiceName = new CopyOnWriteArrayList<>();
@Nullable private final NsdManagerResolverAvailState nsdManagerResolverAvailState;
+ private final long timeout;
/**
* @param context application context
* @param nsdManagerResolverAvailState Passing NsdManagerResolverAvailState allows
* NsdManagerServiceResolver to synchronize on the usage of NsdManager's resolveService() API
+ * @param timeout Timeout value in case there is no response after calling resolve
*/
public NsdManagerServiceResolver(
- Context context, @Nullable NsdManagerResolverAvailState nsdManagerResolverAvailState) {
+ Context context,
+ @Nullable NsdManagerResolverAvailState nsdManagerResolverAvailState,
+ long timeout) {
this.nsdManager = (NsdManager) context.getSystemService(Context.NSD_SERVICE);
- this.mainThreadHandler = new Handler(Looper.getMainLooper());
this.multicastLock =
((WifiManager) context.getSystemService(Context.WIFI_SERVICE))
.createMulticastLock("chipMulticastLock");
this.multicastLock.setReferenceCounted(true);
this.nsdManagerResolverAvailState = nsdManagerResolverAvailState;
+ this.timeout = timeout;
}
public NsdManagerServiceResolver(Context context) {
- this(context, null);
+ this(context, null, RESOLVE_SERVICE_TIMEOUT);
+ }
+
+ public NsdManagerServiceResolver(
+ Context context, @Nullable NsdManagerResolverAvailState nsdManagerResolverAvailState) {
+ this(context, nsdManagerResolverAvailState, RESOLVE_SERVICE_TIMEOUT);
}
@Override
@@ -82,6 +91,10 @@
+ serviceType
+ "'");
+ if (nsdManagerResolverAvailState != null) {
+ nsdManagerResolverAvailState.acquireResolver();
+ }
+
Runnable timeoutRunnable =
new Runnable() {
@Override
@@ -92,14 +105,18 @@
Log.d(TAG, "resolve: Timing out");
if (multicastLock.isHeld()) {
multicastLock.release();
+ }
- if (nsdManagerResolverAvailState != null) {
- nsdManagerResolverAvailState.signalFree();
- }
+ if (nsdManagerResolverAvailState != null) {
+ nsdManagerResolverAvailState.signalFree();
}
}
};
+ ScheduledFuture<?> resolveTimeoutExecutor =
+ Executors.newSingleThreadScheduledExecutor()
+ .schedule(timeoutRunnable, timeout, TimeUnit.MILLISECONDS);
+
NsdServiceFinderAndResolver serviceFinderResolver =
new NsdServiceFinderAndResolver(
this.nsdManager,
@@ -107,13 +124,10 @@
callbackHandle,
contextHandle,
chipMdnsCallback,
- timeoutRunnable,
multicastLock,
- mainThreadHandler,
+ resolveTimeoutExecutor,
nsdManagerResolverAvailState);
serviceFinderResolver.start();
-
- mainThreadHandler.postDelayed(timeoutRunnable, RESOLVE_SERVICE_TIMEOUT);
}
@Override
diff --git a/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java b/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java
index fc3425f..70ffdbc 100644
--- a/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java
+++ b/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java
@@ -21,7 +21,6 @@
import android.net.nsd.NsdManager;
import android.net.nsd.NsdServiceInfo;
import android.net.wifi.WifiManager.MulticastLock;
-import android.os.Handler;
import android.util.Log;
import androidx.annotation.Nullable;
import java.util.concurrent.Executors;
@@ -38,9 +37,8 @@
private final long callbackHandle;
private final long contextHandle;
private final ChipMdnsCallback chipMdnsCallback;
- private final Runnable timeoutRunnable;
private final MulticastLock multicastLock;
- private final Handler mainThreadHandler;
+ private final ScheduledFuture<?> resolveTimeoutExecutor;
@Nullable
private final NsdManagerServiceResolver.NsdManagerResolverAvailState nsdManagerResolverAvailState;
@@ -53,18 +51,16 @@
final long callbackHandle,
final long contextHandle,
final ChipMdnsCallback chipMdnsCallback,
- final Runnable timeoutRunnable,
final MulticastLock multicastLock,
- final Handler mainThreadHandler,
+ final ScheduledFuture<?> resolveTimeoutExecutor,
final NsdManagerServiceResolver.NsdManagerResolverAvailState nsdManagerResolverAvailState) {
this.nsdManager = nsdManager;
this.targetServiceInfo = targetServiceInfo;
this.callbackHandle = callbackHandle;
this.contextHandle = contextHandle;
this.chipMdnsCallback = chipMdnsCallback;
- this.timeoutRunnable = timeoutRunnable;
this.multicastLock = multicastLock;
- this.mainThreadHandler = mainThreadHandler;
+ this.resolveTimeoutExecutor = resolveTimeoutExecutor;
this.nsdManagerResolverAvailState = nsdManagerResolverAvailState;
}
@@ -101,16 +97,9 @@
if (stopDiscoveryRunnable.cancel(false)) {
nsdManager.stopServiceDiscovery(this);
- if (multicastLock.isHeld()) {
- multicastLock.release();
- }
}
- if (nsdManagerResolverAvailState != null) {
- nsdManagerResolverAvailState.acquireResolver();
- }
-
- resolveService(service, callbackHandle, contextHandle, chipMdnsCallback, timeoutRunnable);
+ resolveService(service, callbackHandle, contextHandle, chipMdnsCallback);
} else {
Log.d(TAG, "onServiceFound: found service not a target for resolution, ignoring " + service);
}
@@ -120,8 +109,7 @@
NsdServiceInfo serviceInfo,
final long callbackHandle,
final long contextHandle,
- final ChipMdnsCallback chipMdnsCallback,
- Runnable timeoutRunnable) {
+ final ChipMdnsCallback chipMdnsCallback) {
this.nsdManager.resolveService(
serviceInfo,
new NsdManager.ResolveListener() {
@@ -152,7 +140,7 @@
nsdManagerResolverAvailState.signalFree();
}
}
- mainThreadHandler.removeCallbacks(timeoutRunnable);
+ resolveTimeoutExecutor.cancel(false);
}
@Override
@@ -188,7 +176,7 @@
nsdManagerResolverAvailState.signalFree();
}
}
- mainThreadHandler.removeCallbacks(timeoutRunnable);
+ resolveTimeoutExecutor.cancel(false);
}
});
}
