Android - implement ChipDnssdStopBrowse (#32801)
* Android - implement ChipDnssdStopBrowse in the core SDK
Fixing style issues
cleanup log statement
* Addressed comments by bzbarsky-apple and sharadb-amazon
* Addressed comments by bzbarsky-apple now using browseIdentifier to stop DNS-SD browse
* Addressed comments by sharadb-amazon - debug logging
diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java
index b847e17..46401af 100644
--- a/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java
+++ b/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java
@@ -38,9 +38,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
public class DiscoveryExampleFragment extends Fragment {
private static final String TAG = DiscoveryExampleFragment.class.getSimpleName();
@@ -48,9 +45,7 @@
private static final Long DISCOVERY_TARGET_DEVICE_TYPE = 35L;
private static final int DISCOVERY_RUNTIME_SEC = 15;
private TextView matterDiscoveryMessageTextView;
- private static final ScheduledExecutorService executorService =
- Executors.newSingleThreadScheduledExecutor();
- private ScheduledFuture scheduledFutureTask;
+ private TextView matterDiscoveryErrorMessageTextView;
private static final List<CastingPlayer> castingPlayerList = new ArrayList<>();
private static ArrayAdapter<CastingPlayer> arrayAdapter;
@@ -164,15 +159,17 @@
matterDiscoveryMessageTextView.setText(
getString(R.string.matter_discovery_message_initializing_text));
+ matterDiscoveryErrorMessageTextView =
+ getActivity().findViewById(R.id.matterDiscoveryErrorTextView);
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_initial));
+
arrayAdapter = new CastingPlayerArrayAdapter(getActivity(), castingPlayerList);
final ListView list = getActivity().findViewById(R.id.castingPlayerList);
list.setAdapter(arrayAdapter);
Log.d(TAG, "onViewCreated() creating callbacks");
- // TODO: In following PRs. Enable startDiscoveryButton and stopDiscoveryButton when
- // stopDiscovery is implemented in the core Matter SDK DNS-SD API. Enable in
- // fragment_matter_discovery_example.xml
Button startDiscoveryButton = getView().findViewById(R.id.startDiscoveryButton);
startDiscoveryButton.setOnClickListener(
v -> {
@@ -188,8 +185,6 @@
v -> {
Log.i(TAG, "onViewCreated() stopDiscoveryButton button clicked. Calling stopDiscovery()");
stopDiscovery();
- Log.i(TAG, "onViewCreated() stopDiscoveryButton button clicked. Canceling future task");
- scheduledFutureTask.cancel(true);
});
Button clearDiscoveryResultsButton = getView().findViewById(R.id.clearDiscoveryResultsButton);
@@ -198,6 +193,8 @@
Log.i(
TAG, "onViewCreated() clearDiscoveryResultsButton button clicked. Clearing results");
arrayAdapter.clear();
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_initial));
});
}
@@ -219,11 +216,7 @@
public void onPause() {
super.onPause();
Log.i(TAG, "onPause() called");
- // stopDiscovery();
- // Don't crash the app
- if (scheduledFutureTask != null) {
- scheduledFutureTask.cancel(true);
- }
+ stopDiscovery();
}
/** Interface for notifying the host. */
@@ -235,6 +228,8 @@
private boolean startDiscovery() {
Log.i(TAG, "startDiscovery() called");
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_initial));
arrayAdapter.clear();
@@ -244,6 +239,8 @@
matterCastingPlayerDiscovery.addCastingPlayerChangeListener(castingPlayerChangeListener);
if (err.hasError()) {
Log.e(TAG, "startDiscovery() addCastingPlayerChangeListener() called, err Add: " + err);
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_stop_before_starting) + err);
return false;
}
// Start discovery
@@ -251,6 +248,8 @@
err = matterCastingPlayerDiscovery.startDiscovery(DISCOVERY_TARGET_DEVICE_TYPE);
if (err.hasError()) {
Log.e(TAG, "startDiscovery() startDiscovery() called, err Start: " + err);
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_start_error) + err);
return false;
}
@@ -259,28 +258,13 @@
matterDiscoveryMessageTextView.setText(
getString(R.string.matter_discovery_message_discovering_text));
- // TODO: In following PRs. Enable this to auto-stop discovery after stopDiscovery is
- // implemented in the core Matter SDK DNS-SD API.
- // Schedule a service to stop discovery and remove the CastingPlayerChangeListener
- // Safe to call if discovery is not running
- // scheduledFutureTask =
- // executorService.schedule(
- // () -> {
- // Log.i(
- // TAG,
- // "startDiscovery() executorService "
- // + DISCOVERY_RUNTIME_SEC
- // + " seconds timer expired. Auto-calling stopDiscovery()");
- // stopDiscovery();
- // },
- // DISCOVERY_RUNTIME_SEC,
- // TimeUnit.SECONDS);
-
return true;
}
private void stopDiscovery() {
Log.i(TAG, "stopDiscovery() called");
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_initial));
// Stop discovery
MatterError err = matterCastingPlayerDiscovery.stopDiscovery();
@@ -288,8 +272,9 @@
Log.e(
TAG,
"stopDiscovery() MatterCastingPlayerDiscovery.stopDiscovery() called, err Stop: " + err);
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_stop_error) + err);
} else {
- // TODO: In following PRs. Implement stop discovery in the Android core API.
Log.d(TAG, "stopDiscovery() MatterCastingPlayerDiscovery.stopDiscovery() success");
}
@@ -309,6 +294,8 @@
TAG,
"stopDiscovery() matterCastingPlayerDiscovery.removeCastingPlayerChangeListener() called, err Remove: "
+ err);
+ matterDiscoveryErrorMessageTextView.setText(
+ getString(R.string.matter_discovery_error_message_stop_error) + err);
}
}
}
diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayerDiscovery-JNI.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayerDiscovery-JNI.cpp
index 5981ab8..df79e60 100644
--- a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayerDiscovery-JNI.cpp
+++ b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayerDiscovery-JNI.cpp
@@ -73,17 +73,17 @@
VerifyOrReturn(castingPlayerChangeListenerJavaObject.HasValidObjectRef(),
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Warning: Not set, "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Not set, "
"CastingPlayerChangeListener == nullptr"));
VerifyOrReturn(onAddedCallbackJavaMethodID != nullptr,
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Warning: Not set, "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Not set, "
"onAddedCallbackJavaMethodID == nullptr"));
jobject matterCastingPlayerJavaObject = support::convertCastingPlayerFromCppToJava(player);
VerifyOrReturn(matterCastingPlayerJavaObject != nullptr,
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Warning: Could not create "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnAdded() Could not create "
"CastingPlayer jobject"));
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
@@ -100,17 +100,17 @@
VerifyOrReturn(castingPlayerChangeListenerJavaObject.HasValidObjectRef(),
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Warning: Not set, "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Not set, "
"CastingPlayerChangeListener == nullptr"));
VerifyOrReturn(onChangedCallbackJavaMethodID != nullptr,
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Warning: Not set, "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Not set, "
"onChangedCallbackJavaMethodID == nullptr"));
jobject matterCastingPlayerJavaObject = support::convertCastingPlayerFromCppToJava(player);
VerifyOrReturn(matterCastingPlayerJavaObject != nullptr,
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Warning: Could not "
+ "CastingPlayerDiscovery-JNI::DiscoveryDelegateImpl::HandleOnUpdated() Could not "
"create CastingPlayer jobject"));
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
@@ -191,7 +191,7 @@
if (DiscoveryDelegateImpl::GetInstance()->castingPlayerChangeListenerJavaObject.HasValidObjectRef())
{
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::addCastingPlayerChangeListener() Warning: Call removeCastingPlayerChangeListener "
+ "CastingPlayerDiscovery-JNI::addCastingPlayerChangeListener() Call removeCastingPlayerChangeListener "
"before adding a new one");
return support::convertMatterErrorFromCppToJava(CHIP_ERROR_INCORRECT_STATE);
}
@@ -246,11 +246,18 @@
return support::convertMatterErrorFromCppToJava(CHIP_NO_ERROR);
}
+ else if (DiscoveryDelegateImpl::GetInstance()->castingPlayerChangeListenerJavaObject.ObjectRef() == nullptr)
+ {
+ ChipLogError(
+ AppServer,
+ "CastingPlayerDiscovery-JNI::removeCastingPlayerChangeListener() Cannot remove listener. No listener was added");
+ return support::convertMatterErrorFromCppToJava(CHIP_NO_ERROR);
+ }
else
{
ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::removeCastingPlayerChangeListener() Warning: Cannot remove listener. Received a "
- "different CastingPlayerChangeListener object");
+ "CastingPlayerDiscovery-JNI::removeCastingPlayerChangeListener() Cannot remove listener. Received a different "
+ "CastingPlayerChangeListener object");
return support::convertMatterErrorFromCppToJava(CHIP_ERROR_INCORRECT_STATE);
}
}
@@ -276,8 +283,7 @@
jboolean added = env->CallBooleanMethod(arrayList, addMethod, matterCastingPlayerJavaObject);
if (!((bool) added))
{
- ChipLogError(AppServer,
- "CastingPlayerDiscovery-JNI::getCastingPlayers() Warning: Unable to add CastingPlayer with ID: %s",
+ ChipLogError(AppServer, "CastingPlayerDiscovery-JNI::getCastingPlayers() Unable to add CastingPlayer with ID: %s",
player->GetId());
}
}
diff --git a/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_discovery_example.xml b/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_discovery_example.xml
index 1c3eb13..3f3e3d9 100644
--- a/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_discovery_example.xml
+++ b/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_discovery_example.xml
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
-<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
@@ -15,14 +15,14 @@
android:padding="10sp">
<Button
- android:enabled="false"
+ android:enabled="true"
android:id="@+id/startDiscoveryButton"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="@string/matter_start_discovery_button_text" />
<Button
- android:enabled="false"
+ android:enabled="true"
android:id="@+id/stopDiscoveryButton"
android:layout_width="match_parent"
android:layout_height="wrap_content"
@@ -49,5 +49,13 @@
android:fadeScrollbars = "false" />
</LinearLayout>
+ <TextView
+ android:id="@+id/matterDiscoveryErrorTextView"
+ android:layout_width="match_parent"
+ android:layout_height = "wrap_content"
+ android:text="string/matter_discovery_error_default_text"
+ android:textAlignment="center"
+ android:textAppearance="@style/TextAppearance.AppCompat.Small"
+ android:layout_alignParentBottom="true"/>
-</FrameLayout>
\ No newline at end of file
+</RelativeLayout>
\ No newline at end of file
diff --git a/examples/tv-casting-app/android/App/app/src/main/res/values/strings.xml b/examples/tv-casting-app/android/App/app/src/main/res/values/strings.xml
index de7849c..d0cc982 100644
--- a/examples/tv-casting-app/android/App/app/src/main/res/values/strings.xml
+++ b/examples/tv-casting-app/android/App/app/src/main/res/values/strings.xml
@@ -31,8 +31,12 @@
<string name="matter_stop_discovery_button_text">Stop Discovery"</string>
<string name="matter_clear_results_button_text">Clear Results"</string>
<string name="matter_discovery_message_initializing_text">Initializing</string>
- <string name="matter_discovery_message_discovering_text">Casting Players on-network:</string>
+ <string name="matter_discovery_message_discovering_text">Discovering Casting Players on-network:</string>
<string name="matter_discovery_message_stopped_text">Discovery Stopped</string>
+ <string name="matter_discovery_error_message_initial">No errors to report</string>
+ <string name="matter_discovery_error_message_start_error">Start discovery error. -</string>
+ <string name="matter_discovery_error_message_stop_before_starting">Start discovery error. Discovery ongoing, stop before starting. -</string>
+ <string name="matter_discovery_error_message_stop_error">Stop discovery error. -</string>
<string name="matter_connection_next_button_text">Next</string>
<string name="matter_action_selector_text">Select an action</string>
<string name="matter_select_content_launcher_launch_url_button_text">Content Launcher Launch URL</string>
diff --git a/src/platform/android/DnssdImpl.cpp b/src/platform/android/DnssdImpl.cpp
index 5f2e79b..5a27b76 100644
--- a/src/platform/android/DnssdImpl.cpp
+++ b/src/platform/android/DnssdImpl.cpp
@@ -44,6 +44,7 @@
JniGlobalReference sMdnsCallbackObject;
jmethodID sResolveMethod = nullptr;
jmethodID sBrowseMethod = nullptr;
+jmethodID sStopBrowseMethod = nullptr;
jmethodID sGetTextEntryKeysMethod = nullptr;
jmethodID sGetTextEntryDataMethod = nullptr;
jclass sMdnsCallbackClass = nullptr;
@@ -203,13 +204,31 @@
return CHIP_JNI_ERROR_EXCEPTION_THROWN;
}
- *browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
+ auto sdCtx = chip::Platform::New<BrowseContext>(callback);
+ *browseIdentifier = reinterpret_cast<intptr_t>(sdCtx);
+
return CHIP_NO_ERROR;
}
CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
{
- return CHIP_ERROR_NOT_IMPLEMENTED;
+ VerifyOrReturnError(sBrowserObject.HasValidObjectRef() && sStopBrowseMethod != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
+ JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+ auto ctx = reinterpret_cast<BrowseContext *>(browseIdentifier);
+
+ env->CallVoidMethod(sBrowserObject.ObjectRef(), sStopBrowseMethod, reinterpret_cast<jlong>(ctx->callback));
+
+ if (env->ExceptionCheck())
+ {
+ ChipLogError(Discovery, "Java exception in ChipDnssdStopBrowse");
+ env->ExceptionDescribe();
+ env->ExceptionClear();
+ return CHIP_JNI_ERROR_EXCEPTION_THROWN;
+ }
+ chip::Platform::Delete(ctx);
+
+ return CHIP_NO_ERROR;
}
template <size_t N>
@@ -306,6 +325,8 @@
sBrowseMethod = env->GetMethodID(browserClass, "browse", "(Ljava/lang/String;JJLchip/platform/ChipMdnsCallback;)V");
+ sStopBrowseMethod = env->GetMethodID(browserClass, "stopDiscover", "(J)V");
+
if (sResolveMethod == nullptr)
{
ChipLogError(Discovery, "Failed to access Resolver 'resolve' method");
diff --git a/src/platform/android/DnssdImpl.h b/src/platform/android/DnssdImpl.h
index a160157..f106ec6 100644
--- a/src/platform/android/DnssdImpl.h
+++ b/src/platform/android/DnssdImpl.h
@@ -18,6 +18,7 @@
#pragma once
#include <jni.h>
+#include <lib/dnssd/platform/Dnssd.h>
namespace chip {
namespace Dnssd {
@@ -37,5 +38,12 @@
void HandleBrowse(jobjectArray instanceName, jstring serviceType, jlong callbackHandle, jlong contextHandle);
+struct BrowseContext
+{
+ DnssdBrowseCallback callback;
+
+ BrowseContext(DnssdBrowseCallback cb) { callback = cb; };
+};
+
} // namespace Dnssd
} // namespace chip
diff --git a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
index f98acdf..28fc698 100644
--- a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
+++ b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
@@ -24,6 +24,7 @@
import android.net.wifi.WifiManager.MulticastLock;
import android.os.Handler;
import android.os.Looper;
+import android.os.MessageQueue;
import android.util.Log;
import java.util.ArrayList;
import java.util.HashMap;
@@ -68,7 +69,14 @@
new Runnable() {
@Override
public void run() {
- stopDiscover(callbackHandle, chipMdnsCallback);
+ Log.i(
+ TAG,
+ "Browse for service '"
+ + serviceType
+ + "' expired after timeout: "
+ + timeout
+ + " ms");
+ stopDiscover(callbackHandle);
}
};
startDiscover(serviceType, callbackHandle, contextHandle, chipMdnsCallback);
@@ -81,23 +89,30 @@
final long contextHandle,
final ChipMdnsCallback chipMdnsCallback) {
if (callbackMap.containsKey(callbackHandle)) {
- Log.d(TAG, "Invalid callbackHandle");
+ Log.w(TAG, "Starting service discovery failed. Invalid callbackHandle: " + callbackHandle);
return;
}
NsdManagerDiscovery discovery =
new NsdManagerDiscovery(serviceType, callbackHandle, contextHandle);
multicastLock.acquire();
+ discovery.setChipMdnsCallback(chipMdnsCallback);
- Log.d(TAG, "Starting service discovering for '" + serviceType + "'");
+ Log.d(
+ TAG,
+ "Starting service discovery for '"
+ + serviceType
+ + "' with callbackHandle: "
+ + callbackHandle);
this.nsdManager.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discovery);
callbackMap.put(callbackHandle, discovery);
}
- public void stopDiscover(final long callbackHandle, final ChipMdnsCallback chipMdnsCallback) {
+ public void stopDiscover(final long callbackHandle) {
+ Log.d(TAG, "Stopping service discovery with callbackHandle: " + callbackHandle);
if (!callbackMap.containsKey(callbackHandle)) {
- Log.d(TAG, "Invalid callbackHandle");
+ Log.w(TAG, "Stopping service discovery failed. Callback handle not found.");
return;
}
@@ -106,8 +121,13 @@
multicastLock.release();
}
+ MessageQueue queue = mainThreadHandler.getLooper().getQueue();
+ if (!queue.isIdle()) {
+ Log.d(TAG, "Canceling scheduled browse timeout runnable for '" + discovery.serviceType + "'");
+ mainThreadHandler.removeCallbacksAndMessages(null);
+ }
+
this.nsdManager.stopServiceDiscovery(discovery);
- discovery.handleServiceBrowse(chipMdnsCallback);
}
public class NsdManagerDiscovery implements NsdManager.DiscoveryListener {
@@ -115,6 +135,7 @@
private long callbackHandle;
private long contextHandle;
private ArrayList<String> serviceNameList = new ArrayList<>();
+ private ChipMdnsCallback chipMdnsCallback;
public NsdManagerDiscovery(String serviceType, long callbackHandle, long contextHandle) {
this.serviceType = serviceType;
@@ -150,7 +171,8 @@
@Override
public void onDiscoveryStopped(String serviceType) {
- Log.w(TAG, "Succeed to stop discovery service '" + serviceType);
+ Log.w(TAG, "Successfully stopped discovery service '" + serviceType);
+ this.handleServiceBrowse(chipMdnsCallback);
}
public void handleServiceBrowse(ChipMdnsCallback chipMdnsCallback) {
@@ -160,5 +182,9 @@
callbackHandle,
contextHandle);
}
+
+ public void setChipMdnsCallback(final ChipMdnsCallback chipMdnsCallback) {
+ this.chipMdnsCallback = chipMdnsCallback;
+ }
}
}
