From e808d617c372afe8faef848fc95019ad0a911dec Mon Sep 17 00:00:00 2001 From: cliffamzn Date: Thu, 23 Feb 2023 11:13:58 -0800 Subject: [PATCH] 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 | 151 ++++++++++++------ .../com/chip/casting/DiscoveredNodeData.java | 22 +++ .../jni/com/chip/casting/MatterError.java | 3 + .../chip/casting/NsdDiscoveryListener.java | 1 + .../jni/com/chip/casting/TvCastingApp.java | 48 +++--- .../commissionable_player_list_item.xml | 16 ++ .../fragment_commissioner_discovery.xml | 5 + .../CommissionerDiscoveryViewModel.swift | 8 +- 9 files changed, 180 insertions(+), 75 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..1d26cade69324a 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 android.widget.TextView; 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.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,113 @@ 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); + ArrayAdapter arrayAdapter = + new VideoPlayerCommissionerAdapter(getActivity(), commissionerVideoPlayerList); + final ListView list = getActivity().findViewById(R.id.commissionerList); + list.setAdapter(arrayAdapter); + list.setOnItemClickListener( + (parent, view1, position, id) -> { + DiscoveredNodeData discoveredNodeData = + (DiscoveredNodeData) parent.getItemAtPosition(position); + Log.d(TAG, "OnItemClickListener.onClick called for " + discoveredNodeData); + Callback callback1 = (Callback) getActivity(); + callback1.handleCommissioningButtonClicked(discoveredNodeData); + }); - Context context = this.getContext(); - SuccessCallback successCallback = + 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( + () -> { + if (commissionerVideoPlayerList.contains(discoveredNodeData)) { + Log.d(TAG, "Replacing existing entry in players list"); + arrayAdapter.remove(discoveredNodeData); + } + 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 LayoutInflater inflater; + + public VideoPlayerCommissionerAdapter( + Context applicationContext, List playerList) { + super(applicationContext, 0, playerList); + this.playerList = playerList; + inflater = (LayoutInflater.from(applicationContext)); + } + + @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)); + TextView playerDescription = view.findViewById(R.id.commissionable_player_description); + playerDescription.setText(buttonText); + 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 +192,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/jni/com/chip/casting/DiscoveredNodeData.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java index b8a051036edd54..78106cc83246ba 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 @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; public class DiscoveredNodeData { private static final int MAX_IP_ADDRESSES = 5; @@ -182,4 +183,25 @@ public String toString() { + ipAddresses + '}'; } + + // autogenerated + @Override + public int hashCode() { + int result = Objects.hash(vendorId, productId, commissioningMode, deviceType); + result = 31 * result + Arrays.hashCode(rotatingId); + return result; + } + + // autogenerated + @Override + public boolean equals(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 5fda64cbbba8ef..3f1cbf53677169 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 43e7860e6b0527..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,20 +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); - 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..979093e5efb8d8 --- /dev/null +++ b/examples/tv-casting-app/android/App/app/src/main/res/layout/commissionable_player_list_item.xml @@ -0,0 +1,16 @@ + + + + + + \ No newline at end of file diff --git a/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_commissioner_discovery.xml b/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_commissioner_discovery.xml index c65a647957b28e..b9645a288d6449 100644 --- a/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_commissioner_discovery.xml +++ b/examples/tv-casting-app/android/App/app/src/main/res/layout/fragment_commissioner_discovery.xml @@ -31,6 +31,11 @@ android:layout_height="wrap_content" android:text="@string/discovery_message_text" android:textAppearance="@style/TextAppearance.AppCompat.Medium" /> + + diff --git a/examples/tv-casting-app/darwin/TvCasting/TvCasting/CommissionerDiscoveryViewModel.swift b/examples/tv-casting-app/darwin/TvCasting/TvCasting/CommissionerDiscoveryViewModel.swift index 4438759922949f..3bc7492de10dac 100644 --- a/examples/tv-casting-app/darwin/TvCasting/TvCasting/CommissionerDiscoveryViewModel.swift +++ b/examples/tv-casting-app/darwin/TvCasting/TvCasting/CommissionerDiscoveryViewModel.swift @@ -37,7 +37,9 @@ class CommissionerDiscoveryViewModel: ObservableObject { self.Log.info("discoveredCommissionerHandler called with \(commissioner)") if(self.commissioners.contains(commissioner)) { - self.Log.info("Skipping previously discovered commissioner \(commissioner.description)") + var index = self.commissioners.firstIndex(of: commissioner) + self.commissioners[index!] = commissioner + self.Log.info("Updating previously discovered commissioner \(commissioner.description)") } else if(commissioner.numIPs == 0) { @@ -69,7 +71,9 @@ class CommissionerDiscoveryViewModel: ObservableObject { if(commissioner != nil){ if(self.commissioners.contains(commissioner!)) { - self.Log.info("Skipping previously discovered commissioner \(commissioner!.description)") + var index = self.commissioners.firstIndex(of: commissioner!) + self.commissioners[index!] = commissioner! + self.Log.info("Updating previously discovered commissioner \(commissioner!.description)") } else {