From 1f2f8820073f96c842869f693248dc2acba17dd0 Mon Sep 17 00:00:00 2001 From: Cliff Chung <116232729+cliffamzn@users.noreply.github.com> Date: Wed, 8 Mar 2023 12:52:46 -0800 Subject: [PATCH] Respect updated DNS SD broadcast names in TV app (#25355) * Respect updated DNS SD broadcast names When a device is advertising over dns-sd and the TX bits change (device name) a message is sent out to indicate that the old adverisment should be expired and the new one should be used. In the current implementation of the casting app, we stop listening to updates after the `discoveryDurationSeconds` timeout. This causes us to miss messages and Android seems to fall back to the cached value of the DNS advertisment. We need to change the usage pattern here so that clients stop the discovery manually after it is started and keep it active for the lifetime they need it for. I also updated the test apps to behave a bit nicer with discovered commissioners. --- .../android/App/.idea/gradle.xml | 1 - .../app/CommissionerDiscoveryFragment.java | 164 ++++++++++++------ .../chip/casting/app/ConnectionFragment.java | 6 +- .../com/chip/casting/DiscoveredNodeData.java | 19 ++ .../jni/com/chip/casting/MatterError.java | 3 + .../chip/casting/NsdDiscoveryListener.java | 1 + .../jni/com/chip/casting/TvCastingApp.java | 50 +++--- .../commissionable_player_list_item.xml | 12 ++ .../fragment_commissioner_discovery.xml | 10 +- .../CommissionerDiscoveryViewModel.swift | 8 +- 10 files changed, 191 insertions(+), 83 deletions(-) create mode 100644 examples/tv-casting-app/android/App/app/src/main/res/layout/commissionable_player_list_item.xml diff --git a/examples/tv-casting-app/android/App/.idea/gradle.xml b/examples/tv-casting-app/android/App/.idea/gradle.xml index 526b4c25c6813e..a2d7c21338e98a 100644 --- a/examples/tv-casting-app/android/App/.idea/gradle.xml +++ b/examples/tv-casting-app/android/App/.idea/gradle.xml @@ -13,7 +13,6 @@ - diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CommissionerDiscoveryFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CommissionerDiscoveryFragment.java index 5ff612685c515b..0f0529a6c70cfa 100644 --- a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CommissionerDiscoveryFragment.java +++ b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CommissionerDiscoveryFragment.java @@ -8,25 +8,38 @@ import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; +import android.widget.ArrayAdapter; import android.widget.Button; -import android.widget.LinearLayout; +import android.widget.ListView; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; import androidx.fragment.app.Fragment; import com.chip.casting.DiscoveredNodeData; import com.chip.casting.FailureCallback; import com.chip.casting.MatterError; import com.chip.casting.SuccessCallback; import com.chip.casting.TvCastingApp; +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; +import java.util.concurrent.TimeUnit; /** A {@link Fragment} to discover commissioners on the network */ public class CommissionerDiscoveryFragment extends Fragment { private static final String TAG = CommissionerDiscoveryFragment.class.getSimpleName(); - private static final long DISCOVERY_DURATION_SECS = 10; + private static final long DISCOVERY_POLL_INTERVAL_MS = 15000; + private static final List commissionerVideoPlayerList = new ArrayList<>(); + private FailureCallback failureCallback; + private SuccessCallback successCallback; + private ScheduledFuture poller; private final TvCastingApp tvCastingApp; + private ScheduledExecutorService executor; public CommissionerDiscoveryFragment(TvCastingApp tvCastingApp) { this.tvCastingApp = tvCastingApp; + this.executor = Executors.newSingleThreadScheduledExecutor(); } /** @@ -56,75 +69,126 @@ public void onViewCreated(View view, @Nullable Bundle savedInstanceState) { super.onViewCreated(view, savedInstanceState); Button manualCommissioningButton = getView().findViewById(R.id.manualCommissioningButton); + // In the ideal case we wouldn't rely on the host activity to maintain this object since + // the lifecycle of the context isn't tied to this callback. Since this is an example app + // this should work fine. Callback callback = (Callback) this.getActivity(); View.OnClickListener manualCommissioningButtonOnClickListener = - new View.OnClickListener() { - @Override - public void onClick(View v) { - callback.handleCommissioningButtonClicked(null); - } - }; + v -> callback.handleCommissioningButtonClicked(null); manualCommissioningButton.setOnClickListener(manualCommissioningButtonOnClickListener); - Context context = this.getContext(); - SuccessCallback successCallback = + ArrayAdapter arrayAdapter = + new VideoPlayerCommissionerAdapter(getActivity(), commissionerVideoPlayerList); + final ListView list = getActivity().findViewById(R.id.commissionerList); + list.setAdapter(arrayAdapter); + + this.successCallback = new SuccessCallback() { @Override public void handle(DiscoveredNodeData discoveredNodeData) { Log.d(TAG, "Discovered a Video Player Commissioner: " + discoveredNodeData); - String buttonText = getCommissionerButtonText(discoveredNodeData); - - if (!buttonText.isEmpty()) { - Button commissionerButton = new Button(context); - commissionerButton.setText(buttonText); - CommissionerDiscoveryFragment.Callback callback = - (CommissionerDiscoveryFragment.Callback) getActivity(); - commissionerButton.setOnClickListener( - new View.OnClickListener() { - @Override - public void onClick(View v) { - Log.d( - TAG, - "CommissionerResolveListener.onServiceResolved.OnClickListener.onClick called for " - + discoveredNodeData); - callback.handleCommissioningButtonClicked(discoveredNodeData); - } - }); - new Handler(Looper.getMainLooper()) - .post( - () -> - ((LinearLayout) getActivity().findViewById(R.id.castingCommissioners)) - .addView(commissionerButton)); - } + new Handler(Looper.getMainLooper()) + .post( + () -> { + final Optional discoveredNodeInList = + commissionerVideoPlayerList + .stream() + .filter(node -> discoveredNodeData.discoveredNodeHasSameSource(node)) + .findFirst(); + if (discoveredNodeInList.isPresent()) { + Log.d( + TAG, + "Replacing existing entry " + + discoveredNodeInList.get().getDeviceName() + + " in players list"); + arrayAdapter.remove(discoveredNodeInList.get()); + } + arrayAdapter.add(discoveredNodeData); + }); } }; - FailureCallback failureCallback = + this.failureCallback = new FailureCallback() { @Override public void handle(MatterError matterError) { Log.e(TAG, "Error occurred during video player commissioner discovery: " + matterError); + if (MatterError.DISCOVERY_SERVICE_LOST == matterError) { + Log.d(TAG, "Attempting to restart service"); + tvCastingApp.discoverVideoPlayerCommissioners(successCallback, this); + } } }; Button discoverButton = getView().findViewById(R.id.discoverButton); discoverButton.setOnClickListener( - new View.OnClickListener() { - @Override - public void onClick(View v) { - Log.d(TAG, "Discovering on button click"); - tvCastingApp.discoverVideoPlayerCommissioners( - DISCOVERY_DURATION_SECS, successCallback, failureCallback); - } + v -> { + Log.d(TAG, "Discovering on button click"); + tvCastingApp.discoverVideoPlayerCommissioners(successCallback, failureCallback); }); + } + @Override + public void onResume() { + super.onResume(); Log.d(TAG, "Auto discovering"); - tvCastingApp.discoverVideoPlayerCommissioners( - DISCOVERY_DURATION_SECS, successCallback, failureCallback); + + poller = + executor.scheduleAtFixedRate( + () -> { + tvCastingApp.discoverVideoPlayerCommissioners(successCallback, failureCallback); + }, + 0, + DISCOVERY_POLL_INTERVAL_MS, + TimeUnit.MILLISECONDS); + } + + @Override + public void onPause() { + super.onPause(); + tvCastingApp.stopVideoPlayerDiscovery(); + poller.cancel(true); } - @VisibleForTesting - public String getCommissionerButtonText(DiscoveredNodeData commissioner) { + /** Interface for notifying the host. */ + public interface Callback { + /** Notifies listener of Commissioning Button click. */ + void handleCommissioningButtonClicked(DiscoveredNodeData selectedCommissioner); + } +} + +class VideoPlayerCommissionerAdapter extends ArrayAdapter { + private final List playerList; + private final Context context; + private LayoutInflater inflater; + private static final String TAG = VideoPlayerCommissionerAdapter.class.getSimpleName(); + + public VideoPlayerCommissionerAdapter(Context context, List playerList) { + super(context, 0, playerList); + this.context = context; + this.playerList = playerList; + inflater = (LayoutInflater.from(context)); + } + + @Override + public View getView(int i, View view, ViewGroup viewGroup) { + view = inflater.inflate(R.layout.commissionable_player_list_item, null); + String buttonText = getCommissionerButtonText(playerList.get(i)); + Button playerDescription = view.findViewById(R.id.commissionable_player_description); + playerDescription.setText(buttonText); + View.OnClickListener clickListener = + v -> { + DiscoveredNodeData discoveredNodeData = playerList.get(i); + Log.d(TAG, "OnItemClickListener.onClick called for " + discoveredNodeData); + CommissionerDiscoveryFragment.Callback callback1 = + (CommissionerDiscoveryFragment.Callback) context; + callback1.handleCommissioningButtonClicked(discoveredNodeData); + }; + playerDescription.setOnClickListener(clickListener); + return view; + } + + private String getCommissionerButtonText(DiscoveredNodeData commissioner) { String main = commissioner.getDeviceName() != null ? commissioner.getDeviceName() : ""; String aux = "" + (commissioner.getProductId() > 0 ? "Product ID: " + commissioner.getProductId() : ""); @@ -141,10 +205,4 @@ public String getCommissionerButtonText(DiscoveredNodeData commissioner) { String preCommissioned = commissioner.isPreCommissioned() ? " (Pre-commissioned)" : ""; return main + aux + preCommissioned; } - - /** Interface for notifying the host. */ - public interface Callback { - /** Notifies listener of Commissioning Button click. */ - void handleCommissioningButtonClicked(DiscoveredNodeData selectedCommissioner); - } } diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/ConnectionFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/ConnectionFragment.java index c48b28974ef22e..4f090b20e9ef20 100644 --- a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/ConnectionFragment.java +++ b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/ConnectionFragment.java @@ -25,7 +25,6 @@ public class ConnectionFragment extends Fragment { private final TvCastingApp tvCastingApp; private final DiscoveredNodeData selectedCommissioner; - private boolean verifyOrEstablishConnectionSuccess; private boolean openCommissioningWindowSuccess; private boolean sendUdcSuccess; @@ -83,9 +82,8 @@ public void handle(ContentApp contentApp) { if (selectedCommissioner != null && selectedCommissioner.isPreCommissioned()) { VideoPlayer videoPlayer = selectedCommissioner.toConnectableVideoPlayer(); Log.d(TAG, "Calling verifyOrEstablishConnectionSuccess with VideoPlayer: " + videoPlayer); - this.verifyOrEstablishConnectionSuccess = - tvCastingApp.verifyOrEstablishConnection( - videoPlayer, onConnectionSuccess, onConnectionFailure, onNewOrUpdatedEndpoints); + tvCastingApp.verifyOrEstablishConnection( + videoPlayer, onConnectionSuccess, onConnectionFailure, onNewOrUpdatedEndpoints); } else { Log.d(TAG, "Running commissioning"); this.openCommissioningWindowSuccess = diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java index a9bca3aaf46698..2590f82dc2bf92 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; public class DiscoveredNodeData { private static final String TAG = DiscoveredNodeData.class.getSimpleName(); @@ -216,4 +217,22 @@ public String toString() { + ipAddresses + '}'; } + + /** + * Checks to see if a discovered node is "effectively equal" to another by comparing the + * parameters that should not change. + * + * @param o the object to compare to. + * @return true if the objects are from the same source, false otherwise. + */ + public boolean discoveredNodeHasSameSource(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DiscoveredNodeData that = (DiscoveredNodeData) o; + return vendorId == that.vendorId + && productId == that.productId + && commissioningMode == that.commissioningMode + && deviceType == that.deviceType + && Objects.equals(hostName, that.hostName); + } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterError.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterError.java index aaaf95911f18da..e669a7b61bc56b 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterError.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterError.java @@ -23,6 +23,9 @@ public class MatterError { private int errorCode; private String errorMessage; + public static final MatterError DISCOVERY_SERVICE_LOST = + new MatterError(4, "Discovery service was lost."); + public static final MatterError NO_ERROR = new MatterError(0, null); public MatterError(int errorCode, String errorMessage) { diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java index ef5d66a6b5b566..905dcb67f86bc8 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java @@ -95,6 +95,7 @@ public void onServiceLost(NsdServiceInfo service) { // When the network service is no longer available. // Internal bookkeeping code goes here. Log.e(TAG, "Service lost: " + service); + failureCallback.handle(MatterError.DISCOVERY_SERVICE_LOST); } @Override diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java index 009678ff163ecf..316edf3ba26357 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java @@ -32,8 +32,6 @@ import chip.platform.PreferencesKeyValueStoreManager; import java.util.Arrays; import java.util.List; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; public class TvCastingApp { private static final String TAG = TvCastingApp.class.getSimpleName(); @@ -44,6 +42,11 @@ public class TvCastingApp { private Context applicationContext; private ChipAppServer chipAppServer; private NsdManagerServiceResolver.NsdManagerResolverAvailState nsdManagerResolverAvailState; + private boolean discoveryStarted = false; + + private WifiManager.MulticastLock multicastLock; + private NsdManager nsdManager; + private NsdDiscoveryListener nsdDiscoveryListener; public boolean initApp(Context applicationContext, AppParameters appParameters) { if (applicationContext == null || appParameters == null) { @@ -103,21 +106,25 @@ public boolean initApp(Context applicationContext, AppParameters appParameters) private native boolean initJni(AppParameters appParameters); public void discoverVideoPlayerCommissioners( - long discoveryDurationSeconds, SuccessCallback discoverySuccessCallback, FailureCallback discoveryFailureCallback) { Log.d(TAG, "TvCastingApp.discoverVideoPlayerCommissioners called"); + if (this.discoveryStarted) { + Log.d(TAG, "Discovery already started, stopping before starting again"); + stopVideoPlayerDiscovery(); + } + List preCommissionedVideoPlayers = readCachedVideoPlayers(); WifiManager wifiManager = (WifiManager) applicationContext.getSystemService(Context.WIFI_SERVICE); - WifiManager.MulticastLock multicastLock = wifiManager.createMulticastLock("multicastLock"); + multicastLock = wifiManager.createMulticastLock("multicastLock"); multicastLock.setReferenceCounted(true); multicastLock.acquire(); - NsdManager nsdManager = (NsdManager) applicationContext.getSystemService(Context.NSD_SERVICE); - NsdDiscoveryListener nsdDiscoveryListener = + nsdManager = (NsdManager) applicationContext.getSystemService(Context.NSD_SERVICE); + nsdDiscoveryListener = new NsdDiscoveryListener( nsdManager, DISCOVERY_TARGET_SERVICE_TYPE, @@ -129,22 +136,23 @@ public void discoverVideoPlayerCommissioners( nsdManager.discoverServices( DISCOVERY_TARGET_SERVICE_TYPE, NsdManager.PROTOCOL_DNS_SD, nsdDiscoveryListener); + Log.d(TAG, "TvCastingApp.discoverVideoPlayerCommissioners started"); + this.discoveryStarted = true; + } - Executors.newSingleThreadScheduledExecutor() - .schedule( - new Runnable() { - @Override - public void run() { - Log.d(TAG, "TvCastingApp stopping Video Player commissioner discovery"); - nsdManager.stopServiceDiscovery(nsdDiscoveryListener); - if (multicastLock.isHeld()) { - multicastLock.release(); - } - } - }, - discoveryDurationSeconds, - TimeUnit.SECONDS); - Log.d(TAG, "TvCastingApp.discoverVideoPlayerCommissioners ended"); + public void stopVideoPlayerDiscovery() { + Log.d(TAG, "TvCastingApp trying to stop video player discovery"); + if (this.discoveryStarted + && nsdManager != null + && multicastLock != null + && nsdDiscoveryListener != null) { + Log.d(TAG, "TvCastingApp stopping Video Player commissioner discovery"); + nsdManager.stopServiceDiscovery(nsdDiscoveryListener); + if (multicastLock.isHeld()) { + multicastLock.release(); + } + this.discoveryStarted = false; + } } public native boolean openBasicCommissioningWindow( diff --git a/examples/tv-casting-app/android/App/app/src/main/res/layout/commissionable_player_list_item.xml b/examples/tv-casting-app/android/App/app/src/main/res/layout/commissionable_player_list_item.xml new file mode 100644 index 00000000000000..a3adc515f19184 --- /dev/null +++ b/examples/tv-casting-app/android/App/app/src/main/res/layout/commissionable_player_list_item.xml @@ -0,0 +1,12 @@ + + + +