Skip to content

Commit

Permalink
Addressed comments by sharadb-amazon - 2nd review
Browse files Browse the repository at this point in the history
  • Loading branch information
@pgregorr-amazon
pgregorr-amazon committed Dec 28, 2023
1 parent 7131a44 commit 5d5e070
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 153 deletions.
149 changes: 60 additions & 89 deletions ...asting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
public class DiscoveryExampleFragment extends Fragment {
private static final String TAG = DiscoveryExampleFragment.class.getSimpleName();
// 35 represents device type of Matter Casting Player
private static final int DISCOVERY_TARGET_DEVICE_TYPE = 35;
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 =
Expand All @@ -70,98 +70,68 @@ public class DiscoveryExampleFragment extends Fragment {

@Override
public void onAdded(CastingPlayer castingPlayer) {
if (castingPlayer != null) {
Log.i(
TAG,
"onAdded() Discovered CastingPlayer deviceId: " + castingPlayer.getDeviceId());
// Display CastingPlayer info on the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
final Optional<CastingPlayer> playerInList =
castingPlayerList
.stream()
.filter(node -> castingPlayer.equals(node))
.findFirst();
if (playerInList.isPresent()) {
Log.d(
TAG,
"onAdded() Replacing existing CastingPlayer entry "
+ playerInList.get().getDeviceId()
+ " in castingPlayerList list");
arrayAdapter.remove(playerInList.get());
}
arrayAdapter.add(castingPlayer);
});
} else {
Log.d(TAG, "onAdded() CastingPlayer is null");
// Attempt to invoke interface method on a null object reference will throw an error
Log.d(
TAG,
"onAdded() Discovered CastingPlayer with deviceId: "
+ castingPlayer.getDeviceId());
}
Log.i(
TAG,
"onAdded() Discovered CastingPlayer deviceId: " + castingPlayer.getDeviceId());
// Display CastingPlayer info on the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
arrayAdapter.add(castingPlayer);
});
}

@Override
public void onChanged(CastingPlayer castingPlayer) {
if (castingPlayer != null) {
Log.i(
TAG,
"onChanged() Discovered changes to CastingPlayer with deviceId: "
+ castingPlayer.getDeviceId());
// Update the CastingPlayer on the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
final Optional<CastingPlayer> playerInList =
castingPlayerList
.stream()
.filter(node -> castingPlayer.equals(node))
.findFirst();
if (playerInList.isPresent()) {
Log.d(
TAG,
"onChanged() Updating existing CastingPlayer entry "
+ playerInList.get().getDeviceId()
+ " in castingPlayerList list");
arrayAdapter.remove(playerInList.get());
}
arrayAdapter.add(castingPlayer);
});
} else {
Log.d(TAG, "onChanged() CastingPlayer is null");
}
Log.i(
TAG,
"onChanged() Discovered changes to CastingPlayer with deviceId: "
+ castingPlayer.getDeviceId());
// Update the CastingPlayer on the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
final Optional<CastingPlayer> playerInList =
castingPlayerList
.stream()
.filter(node -> castingPlayer.equals(node))
.findFirst();
if (playerInList.isPresent()) {
Log.d(
TAG,
"onChanged() Updating existing CastingPlayer entry "
+ playerInList.get().getDeviceId()
+ " in castingPlayerList list");
arrayAdapter.remove(playerInList.get());
}
arrayAdapter.add(castingPlayer);
});
}

@Override
public void onRemoved(CastingPlayer castingPlayer) {
if (castingPlayer != null) {
Log.i(
TAG,
"onRemoved() Removed CastingPlayer with deviceId: "
+ castingPlayer.getDeviceId());
// Remove CastingPlayer from the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
final Optional<CastingPlayer> playerInList =
castingPlayerList
.stream()
.filter(node -> castingPlayer.equals(node))
.findFirst();
if (playerInList.isPresent()) {
Log.d(
TAG,
"onRemoved() Removing existing CastingPlayer entry "
+ playerInList.get().getDeviceId()
+ " in castingPlayerList list");
arrayAdapter.remove(playerInList.get());
}
});
} else {
Log.d(TAG, "onRemoved() CastingPlayer is null");
}
Log.i(
TAG,
"onRemoved() Removed CastingPlayer with deviceId: "
+ castingPlayer.getDeviceId());
// Remove CastingPlayer from the screen
new Handler(Looper.getMainLooper())
.post(
() -> {
final Optional<CastingPlayer> playerInList =
castingPlayerList
.stream()
.filter(node -> castingPlayer.equals(node))
.findFirst();
if (playerInList.isPresent()) {
Log.d(
TAG,
"onRemoved() Removing existing CastingPlayer entry "
+ playerInList.get().getDeviceId()
+ " in castingPlayerList list");
arrayAdapter.remove(playerInList.get());
}
});
}
};

Expand Down Expand Up @@ -201,14 +171,13 @@ public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
Log.d(TAG, "onViewCreated() creating callbacks");

// TODO: In following PRs. Enable startDiscoveryButton and stopDiscoveryButton when
// stopDiscovery is implemented in the core Matter SKD DNS-SD API. Enable in
// 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 -> {
Log.i(
TAG, "onViewCreated() startDiscoveryButton button clicked. Calling startDiscovery()");
// arrayAdapter.clear();
if (!startDiscovery()) {
Log.e(TAG, "onViewCreated() startDiscovery() call Failed");
}
Expand Down Expand Up @@ -262,6 +231,8 @@ public interface Callback {
private boolean startDiscovery() {
Log.i(TAG, "startDiscovery() called");

arrayAdapter.clear();

// Add the implemented CastingPlayerChangeListener to listen to changes in the discovered
// CastingPlayers
MatterError err =
Expand All @@ -271,7 +242,7 @@ private boolean startDiscovery() {
return false;
}
// Start discovery
Log.i(TAG, "startDiscovery() calling startDiscovery()");
Log.i(TAG, "startDiscovery() calling CastingPlayerDiscovery.startDiscovery()");
err = matterCastingPlayerDiscovery.startDiscovery(DISCOVERY_TARGET_DEVICE_TYPE);
if (err.hasError()) {
Log.e(TAG, "startDiscovery() startDiscovery() called, err Start: " + err);
Expand Down Expand Up @@ -392,7 +363,7 @@ private String getCastingPlayerButtonText(CastingPlayer player) {
player.getDeviceType() > 0
? (aux.isEmpty() ? "" : ", ") + "Device Type: " + player.getDeviceType()
: "";
aux += (aux.isEmpty() ? "" : ", ") + "Resolved IP: " + (player.getNumberIPs() > 0);
aux += (aux.isEmpty() ? "" : ", ") + "Resolved IP?: " + (player.getIpAddresses().size() > 0);

aux = aux.isEmpty() ? aux : "\n" + aux;
return main + aux;
Expand Down
7 changes: 4 additions & 3 deletions ...es/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ public interface CastingPlayer {

String getInstanceName();

int getNumberIPs();

List<InetAddress> getIpAddresses();

int getPort();
Expand All @@ -46,12 +44,15 @@ public interface CastingPlayer {

int getProductId();

int getDeviceType();
long getDeviceType();

@Override
String toString();

@Override
boolean equals(Object o);

@Override
int hashCode();

// TODO: Implement in following PRs. Related to player connection implementation.
Expand Down
39 changes: 20 additions & 19 deletions ...ting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayerDiscovery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import java.util.List;

/**
* The CastingPlayerDiscovery interface defines the API to control Matter Casting Player discovery
* over DNS-SD, and to collect discovery results. Discovery is centrally managed by the native C++
* layer in the Matter SDK. This class exposes native functions to add and remove a
* CastingPlayerChangeListener, which contains the C++ to Java callbacks for when Casting Players
* are discovered, updated, or lost from the network. This class is a singleton.
* The CastingPlayerDiscovery interface defines the client API to control Matter Casting Player
* discovery over DNS-SD, and to collect discovery results. This interface defines the methods to
* add and remove a CastingPlayerChangeListener. It also defines the CastingPlayerChangeListener
* handler class which must be implemented by the API client. The handler contains the methods
* called when Casting Players are discovered, updated, or lost from the network.
*/
public interface CastingPlayerDiscovery {

Expand All @@ -38,11 +38,12 @@ public interface CastingPlayerDiscovery {
/**
* Starts Casting Players discovery or returns an error.
*
* @param discoveryTargetDeviceType the target device type to be discovered using DNS-SD. 35
* represents device type of Matter Casting Player.
* @param discoveryTargetDeviceType the target device type to be discovered using DNS-SD. For
* example: 35 represents device type of Matter Casting Video Player. If "null" is passed in,
* discovery will default to all "_matterd._udp" device types.
* @return a specific MatterError if the the operation failed or NO_ERROR if succeeded.
*/
MatterError startDiscovery(int discoveryTargetDeviceType);
MatterError startDiscovery(Long discoveryTargetDeviceType);

/**
* Stops Casting Players discovery or returns an error.
Expand All @@ -53,7 +54,7 @@ public interface CastingPlayerDiscovery {

/**
* Adds a CastingPlayerChangeListener instance to be used during discovery. The
* CastingPlayerChangeListener contains the C++ to Java callbacks for when Casting Players are
* CastingPlayerChangeListener defines the handler methods for when Casting Players are
* discovered, updated, or lost from the network. Should be called prior to calling
* MatterCastingPlayerDiscovery.startDiscovery().
*
Expand All @@ -64,46 +65,46 @@ public interface CastingPlayerDiscovery {
MatterError addCastingPlayerChangeListener(CastingPlayerChangeListener listener);

/**
* Removes CastingPlayerChangeListener from the native layer.
* Removes CastingPlayerChangeListener added by addCastingPlayerChangeListener().
*
* @param listener the specific instance of CastingPlayerChangeListener to be removed.
* @return a specific MatterError if the the operation failed or NO_ERROR if succeeded.
*/
MatterError removeCastingPlayerChangeListener(CastingPlayerChangeListener listener);

/**
* The CastingPlayerChangeListener can discover CastingPlayers by implementing the onAdded,
* onChanged and onRemoved callbacks which are called as CastingPlayers, are discovered, updated,
* or lost from the network. The onAdded(), onChanged() and onRemoved() callbacks must be
* implemented by the APIs client.
* The CastingPlayerChangeListener can discover CastingPlayers by implementing the onAdded(),
* onChanged() and onRemoved() callbacks which are called as CastingPlayers, are discovered,
* updated, or lost from the network. The onAdded(), onChanged() and onRemoved() handlers must
* be implemented by the API client.
*/
abstract class CastingPlayerChangeListener {
static final String TAG = CastingPlayerChangeListener.class.getSimpleName();

/**
* Called by the native C++ layer when a Casting Player is added to the local network.
* This handler is called when a Casting Player is added to the network.
*
* @param castingPlayer the Casting Player added.
*/
public abstract void onAdded(CastingPlayer castingPlayer);

/**
* Called by the native C++ layer when a Casting Player on the local network is changed.
* This handler is called when a Casting Player previously detected on the network is changed.
*
* @param castingPlayer the Casting Player changed.
*/
public abstract void onChanged(CastingPlayer castingPlayer);

/**
* Called by the native C++ layer when a Casting Player is removed from the local network.
* This handler is called when a Casting Player previously detected on the network is removed.
*
* @param castingPlayer the Casting Player removed.
*/
public abstract void onRemoved(CastingPlayer castingPlayer);

/**
* The following methods are used to catch possible exceptions thrown by the methods above, when
* not implemented correctly.
* The following methods are used to catch possible exceptions thrown by the methods above
* (onAdded(), onChanged() and onRemoved()), when not implemented correctly by the client.
*/
protected final void _onAdded(CastingPlayer castingPlayer) {
try {
Expand Down
15 changes: 3 additions & 12 deletions ...casting-app/android/App/app/src/main/jni/com/matter/casting/core/MatterCastingPlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,28 @@ public class MatterCastingPlayer implements CastingPlayer {
private String deviceName;
private String hostName;
private String instanceName;
private int numberIPs;
private List<InetAddress> ipAddresses;
private int port;
private int productId;
private int vendorId;
private int deviceType;
private long deviceType;

public MatterCastingPlayer(
boolean connected,
String deviceId,
String hostName,
String deviceName,
String instanceName,
int numberIPs,
List<InetAddress> ipAddresses,
int port,
int productId,
int vendorId,
int deviceType) {
long deviceType) {
this.connected = connected;
this.deviceId = deviceId;
this.hostName = hostName;
this.deviceName = deviceName;
this.instanceName = instanceName;
this.numberIPs = numberIPs;
this.ipAddresses = ipAddresses;
this.port = port;
this.productId = productId;
Expand Down Expand Up @@ -97,12 +94,6 @@ public String getInstanceName() {
return this.instanceName;
}

/** @return an int, corresponding to the number of valid IP addresses for this Casting PLayer. */
@Override
public int getNumberIPs() {
return this.numberIPs;
}

/** @return a list of valid IP addresses for this Casting PLayer. */
@Override
public List<InetAddress> getIpAddresses() {
Expand All @@ -125,7 +116,7 @@ public int getProductId() {
}

@Override
public int getDeviceType() {
public long getDeviceType() {
return this.deviceType;
}

Expand Down
Loading

0 comments on commit 5d5e070

Please sign in to comment.