Addressed comments by sharadb-amazon

project-chip · Jan 24, 2024 · 9c99ed1 · 9c99ed1
1 parent 313c7a0
commit 9c99ed1
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 93 deletions.
diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MainActivity.java b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MainActivity.java
@@ -72,10 +72,7 @@ public void handleCommissioningComplete() {
 
   @Override
   public void handleConnectionComplete(CastingPlayer castingPlayer) {
-    Log.i(
-        TAG,
-        "MainActivity.handleConnectionComplete() called with CastingPlayer with deviceId: "
-            + castingPlayer.getDeviceId());
+    Log.i(TAG, "MainActivity.handleConnectionComplete() called ");
 
     // TODO: Implement in following PRs. Select Cluster Fragment.
     // showFragment(SelectClusterFragment.newInstance(tvCastingApp));

diff --git a/...sting-app/android/App/app/src/main/java/com/matter/casting/ConnectionExampleFragment.java b/...sting-app/android/App/app/src/main/java/com/matter/casting/ConnectionExampleFragment.java
@@ -27,6 +27,9 @@
 import androidx.fragment.app.Fragment;
 import com.R;
 import com.matter.casting.core.CastingPlayer;
+import com.matter.casting.support.DeviceTypeStruct;
+import com.matter.casting.support.EndpointFilter;
+import java.util.ArrayList;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executors;
 
@@ -36,16 +39,13 @@ public class ConnectionExampleFragment extends Fragment {
   // Time (in sec) to keep the commissioning window open, if commissioning is required.
   // Must be >= 3 minutes.
   private static final long MIN_CONNECTION_TIMEOUT_SEC = 3 * 60;
-  private final CastingPlayer selectedCastingPlayer;
+  private final CastingPlayer targetCastingPlayer;
   private TextView connectionFragmentStatusTextView;
   private Button connectionFragmentNextButton;
 
-  public ConnectionExampleFragment(CastingPlayer selectedCastingPlayer) {
-    Log.i(
-        TAG,
-        "ConnectionExampleFragment() called with CastingPlayer with deviceId: "
-            + selectedCastingPlayer.getDeviceId());
-    this.selectedCastingPlayer = selectedCastingPlayer;
+  public ConnectionExampleFragment(CastingPlayer targetCastingPlayer) {
+    Log.i(TAG, "ConnectionExampleFragment() called with target CastingPlayer");
+    this.targetCastingPlayer = targetCastingPlayer;
   }
 
   /**
@@ -78,67 +78,54 @@ public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
     super.onViewCreated(view, savedInstanceState);
     Log.i(TAG, "onViewCreated() called");
 
-    connectionFragmentStatusTextView = getView().findViewById(R.id.connectionWindowStatus);
+    connectionFragmentStatusTextView = getView().findViewById(R.id.connectionFragmentStatusText);
     connectionFragmentStatusTextView.setText(
         "Verifying or establishing connection with Casting Player with device name: "
-            + selectedCastingPlayer.getDeviceName());
+            + targetCastingPlayer.getDeviceName());
 
-    connectionFragmentNextButton = getView().findViewById(R.id.connectionWindowNextButton);
+    connectionFragmentNextButton = getView().findViewById(R.id.connectionFragmentNextButton);
     Callback callback = (ConnectionExampleFragment.Callback) this.getActivity();
     connectionFragmentNextButton.setOnClickListener(
         v -> {
-          Log.i(
-              TAG,
-              "onViewCreated() connectionWindowNextButton button clicked. Calling MainActivity.handleConnectionComplete()");
-          callback.handleConnectionComplete(selectedCastingPlayer);
+          Log.i(TAG, "onViewCreated() NEXT clicked. Calling handleConnectionComplete()");
+          callback.handleConnectionComplete(targetCastingPlayer);
         });
 
     Executors.newSingleThreadExecutor()
         .submit(
             () -> {
-              Log.d(
-                  TAG,
-                  "onViewCreated() calling verifyOrEstablishConnection() on CastingPlayer with deviceId: "
-                      + selectedCastingPlayer.getDeviceId());
+              Log.d(TAG, "onViewCreated() calling verifyOrEstablishConnection()");
 
+              EndpointFilter desiredEndpointFilter =
+                  new EndpointFilter(null, 65521, new ArrayList<DeviceTypeStruct>());
+              // The desired commissioning window timeout and EndpointFilter are optional.
               CompletableFuture<Void> completableFuture =
-                  selectedCastingPlayer.VerifyOrEstablishConnection();
-              // Optionally, we can specify the desired commissioning window duration and Endpoint
-              // Filter.
-              // EndpointFilter desiredEndpointFilter = new EndpointFilter(0, 0, new
-              // ArrayList<DeviceTypeStruct>());
-              // CompletableFuture<Void> completableFuture =
-              // selectedCastingPlayer.VerifyOrEstablishConnection(MIN_CONNECTION_TIMEOUT_SEC,
-              // desiredEndpointFilter);
+                  targetCastingPlayer.VerifyOrEstablishConnection(
+                      MIN_CONNECTION_TIMEOUT_SEC, desiredEndpointFilter);
 
               Log.d(TAG, "onViewCreated() verifyOrEstablishConnection() called");
-              if (completableFuture == null) {
-                Log.e(
-                    TAG,
-                    "onViewCreated() verifyOrEstablishConnection() Warning: completableFuture == null");
-              }
 
               completableFuture
                   .thenRun(
                       () -> {
                         Log.i(
                             TAG,
-                            "onViewCreated() CompletableFuture.thenRun(), Connected to CastingPlayer with deviceId: "
-                                + selectedCastingPlayer.getDeviceId());
+                            "CompletableFuture.thenRun(), connected to CastingPlayer with deviceId: "
+                                + targetCastingPlayer.getDeviceId());
                         getActivity()
                             .runOnUiThread(
                                 () -> {
                                   connectionFragmentStatusTextView.setText(
                                       "Connected to Casting Player with device name: "
-                                          + selectedCastingPlayer.getDeviceName());
+                                          + targetCastingPlayer.getDeviceName());
                                   connectionFragmentNextButton.setEnabled(true);
                                 });
                       })
                   .exceptionally(
                       exc -> {
                         Log.e(
                             TAG,
-                            "onViewCreated() CompletableFuture.exceptionally(), CastingPlayer connection failed due to exception: "
+                            "CompletableFuture.exceptionally(), CastingPLayer connection failed: "
                                 + exc.getMessage());
                         getActivity()
                             .runOnUiThread(
@@ -154,7 +141,7 @@ public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
 
   /** Interface for notifying the host. */
   public interface Callback {
-    /** Notifies listener to trigger transition on completion of commissioning */
+    /** Notifies listener to trigger transition on completion of connection */
     void handleConnectionComplete(CastingPlayer castingPlayer);
   }
 }
diff --git a/...v-casting-app/android/App/app/src/main/jni/com/matter/casting/support/EndpointFilter.java b/...v-casting-app/android/App/app/src/main/jni/com/matter/casting/support/EndpointFilter.java
@@ -20,13 +20,14 @@
 
 /** Describes an Endpoint that the client wants to connect to. */
 public class EndpointFilter {
-  // Value of 0 means unspecified
-  public int productId;
-  // Value of 0 means unspecified
-  public int vendorId;
+  // Value of null means unspecified
+  public Integer productId;
+  // Value of null means unspecified
+  public Integer vendorId;
   public List<DeviceTypeStruct> requiredDeviceTypes;
 
-  public EndpointFilter(int productId, int vendorId, List<DeviceTypeStruct> requiredDeviceTypes) {
+  public EndpointFilter(
+      Integer productId, Integer vendorId, List<DeviceTypeStruct> requiredDeviceTypes) {
     this.productId = productId;
     this.vendorId = vendorId;
     this.requiredDeviceTypes = requiredDeviceTypes;

diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp
@@ -53,13 +53,12 @@ JNI_METHOD(jobject, VerifyOrEstablishConnection)
     jfieldID _cppCastingPlayerFieldId = env->GetFieldID(castingPlayerClass, "_cppCastingPlayer", "J");
     VerifyOrReturnValue(
         _cppCastingPlayerFieldId != nullptr, nullptr,
-        ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() Warning: _cppCastingPlayerFieldId == nullptr"));
+        ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() _cppCastingPlayerFieldId == nullptr"));
 
     jlong _cppCastingPlayerValue  = env->GetLongField(thiz, _cppCastingPlayerFieldId);
     CastingPlayer * castingPlayer = reinterpret_cast<CastingPlayer *>(_cppCastingPlayerValue);
-    VerifyOrReturnValue(
-        castingPlayer != nullptr, nullptr,
-        ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() Warning: castingPlayer == nullptr"));
+    VerifyOrReturnValue(castingPlayer != nullptr, nullptr,
+                        ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() castingPlayer == nullptr"));
 
     // Create a new Java CompletableFuture
     jclass completableFutureClass          = env->FindClass("java/util/concurrent/CompletableFuture");
@@ -68,48 +67,45 @@ JNI_METHOD(jobject, VerifyOrEstablishConnection)
     jobject completableFutureObjGlobalRef  = env->NewGlobalRef(completableFutureObj);
     VerifyOrReturnValue(
         completableFutureObjGlobalRef != nullptr, nullptr,
-        ChipLogError(AppServer,
-                     "CastingPlayer-JNI::VerifyOrEstablishConnection() Warning: completableFutureObjGlobalRef == nullptr"));
+        ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() completableFutureObjGlobalRef == nullptr"));
 
     ConnectCallback callback = [completableFutureObjGlobalRef](CHIP_ERROR err, CastingPlayer * playerPtr) {
         ChipLogProgress(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback called");
+        JNIEnv * env                  = JniReferences::GetInstance().GetEnvForCurrentThread();
+        jclass completableFutureClass = env->FindClass("java/util/concurrent/CompletableFuture");
+
         if (completableFutureObjGlobalRef == nullptr)
         {
-            // Prevents an app crash at CallBooleanMethod
             ChipLogError(AppServer,
-                         "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, Warning: completableFutureObjGlobalRef "
-                         "== nullptr");
+                         "PHILIPGREGOR CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback called, "
+                         "completableFutureObjGlobalRef == nullptr");
+        }
+
+        if (err == CHIP_NO_ERROR)
+        {
+            ChipLogProgress(
+                AppServer,
+                "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, Casting Player connection successful!");
+            jmethodID completeMethod = env->GetMethodID(completableFutureClass, "complete", "(Ljava/lang/Object;)Z");
+            chip::DeviceLayer::StackUnlock unlock;
+            env->CallBooleanMethod(completableFutureObjGlobalRef, completeMethod, nullptr);
         }
         else
         {
-            JNIEnv * env                  = JniReferences::GetInstance().GetEnvForCurrentThread();
-            jclass completableFutureClass = env->FindClass("java/util/concurrent/CompletableFuture");
-
-            if (err == CHIP_NO_ERROR)
-            {
-                ChipLogProgress(
-                    AppServer,
-                    "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, Casting Player connection successful!");
-                jmethodID completeMethod = env->GetMethodID(completableFutureClass, "complete", "(Ljava/lang/Object;)Z");
-                env->CallBooleanMethod(completableFutureObjGlobalRef, completeMethod, NULL);
-            }
-            else
-            {
-                ChipLogError(AppServer,
-                             "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, Warning: connection error: "
-                             "%" CHIP_ERROR_FORMAT,
-                             err.Format());
-                jmethodID completeExceptionallyMethod =
-                    env->GetMethodID(completableFutureClass, "completeExceptionally", "(Ljava/lang/Throwable;)Z");
-                // Create a Throwable object (e.g., RuntimeException) to pass to completeExceptionallyMethod
-                jclass throwableClass          = env->FindClass("java/lang/RuntimeException");
-                jmethodID throwableConstructor = env->GetMethodID(throwableClass, "<init>", "(Ljava/lang/String;)V");
-                jstring errorMessage           = env->NewStringUTF(err.Format());
-                jobject throwableObject        = env->NewObject(throwableClass, throwableConstructor, errorMessage);
-                env->CallBooleanMethod(completableFutureObjGlobalRef, completeExceptionallyMethod, throwableObject);
-            }
-            env->DeleteGlobalRef(completableFutureObjGlobalRef);
+            ChipLogError(AppServer,
+                         "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, connection error: %" CHIP_ERROR_FORMAT,
+                         err.Format());
+            jmethodID completeExceptionallyMethod =
+                env->GetMethodID(completableFutureClass, "completeExceptionally", "(Ljava/lang/Throwable;)Z");
+            // Create a Throwable object (e.g., RuntimeException) to pass to completeExceptionallyMethod
+            jclass throwableClass          = env->FindClass("java/lang/RuntimeException");
+            jmethodID throwableConstructor = env->GetMethodID(throwableClass, "<init>", "(Ljava/lang/String;)V");
+            jstring errorMessage           = env->NewStringUTF(err.Format());
+            jobject throwableObject        = env->NewObject(throwableClass, throwableConstructor, errorMessage);
+            chip::DeviceLayer::StackUnlock unlock;
+            env->CallBooleanMethod(completableFutureObjGlobalRef, completeExceptionallyMethod, throwableObject);
         }
+        env->DeleteGlobalRef(completableFutureObjGlobalRef);
     };
 
     if (desiredEndpointFilterJavaObject == nullptr)
@@ -122,24 +118,36 @@ JNI_METHOD(jobject, VerifyOrEstablishConnection)
     }
     else
     {
-        ChipLogProgress(AppServer,
-                        "CastingPlayer-JNI::VerifyOrEstablishConnection() calling "
-                        "CastingPlayer::VerifyOrEstablishConnection(desiredEndpointFilter) on Casting Player with device ID: %s",
-                        castingPlayer->GetId());
         // Convert the EndpointFilter Java class to a C++ EndpointFilter
         jclass endpointFilterJavaClass = env->GetObjectClass(desiredEndpointFilterJavaObject);
-        jfieldID vendorIdFieldId       = env->GetFieldID(endpointFilterJavaClass, "vendorId", "I");
-        jfieldID productIdFieldId      = env->GetFieldID(endpointFilterJavaClass, "productId", "I");
+        jfieldID vendorIdFieldId       = env->GetFieldID(endpointFilterJavaClass, "vendorId", "Ljava/lang/Integer;");
+        jfieldID productIdFieldId      = env->GetFieldID(endpointFilterJavaClass, "productId", "Ljava/lang/Integer;");
+        jobject vendorIdIntegerObject  = env->GetObjectField(desiredEndpointFilterJavaObject, vendorIdFieldId);
+        jobject productIdIntegerObject = env->GetObjectField(desiredEndpointFilterJavaObject, productIdFieldId);
         // jfieldID requiredDeviceTypesFieldId = env->GetFieldID(endpointFilterJavaClass, "requiredDeviceTypes",
         // "Ljava/util/List;");
 
         matter::casting::core::EndpointFilter desiredEndpointFilter;
-        desiredEndpointFilter.vendorId = static_cast<uint16_t>(env->GetIntField(desiredEndpointFilterJavaObject, vendorIdFieldId));
-        desiredEndpointFilter.productId =
-            static_cast<uint16_t>(env->GetIntField(desiredEndpointFilterJavaObject, productIdFieldId));
+        // Value of 0 means unspecified
+        desiredEndpointFilter.vendorId  = vendorIdIntegerObject != nullptr
+             ? static_cast<uint16_t>(env->CallIntMethod(
+                  vendorIdIntegerObject, env->GetMethodID(env->GetObjectClass(vendorIdIntegerObject), "intValue", "()I")))
+             : 0;
+        desiredEndpointFilter.productId = productIdIntegerObject != nullptr
+            ? static_cast<uint16_t>(env->CallIntMethod(
+                  productIdIntegerObject, env->GetMethodID(env->GetObjectClass(productIdIntegerObject), "intValue", "()I")))
+            : 0;
+        ChipLogProgress(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() desiredEndpointFilter.vendorId: %d",
+                        desiredEndpointFilter.vendorId);
+        ChipLogProgress(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() desiredEndpointFilter.productId: %d",
+                        desiredEndpointFilter.productId);
         // TODO: In following PRs. Translate the Java requiredDeviceTypes list to a C++ requiredDeviceTypes vector. For now we're
         // passing an empty list of.
 
+        ChipLogProgress(AppServer,
+                        "CastingPlayer-JNI::VerifyOrEstablishConnection() calling "
+                        "CastingPlayer::VerifyOrEstablishConnection() on Casting Player with device ID: %s",
+                        castingPlayer->GetId());
         castingPlayer->VerifyOrEstablishConnection(callback, static_cast<unsigned long long int>(commissioningWindowTimeoutSec),
                                                    desiredEndpointFilter);
     }

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
@@ -88,6 +88,7 @@ class DiscoveryDelegateImpl : public DiscoveryDelegate
                                     "CastingPlayer jobject"));
 
         JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+        chip::DeviceLayer::StackUnlock unlock;
         env->CallVoidMethod(castingPlayerChangeListenerJavaObject, onAddedCallbackJavaMethodID, matterCastingPlayerJavaObject);
     }
 
@@ -113,6 +114,7 @@ class DiscoveryDelegateImpl : public DiscoveryDelegate
                                     "create CastingPlayer jobject"));
 
         JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+        chip::DeviceLayer::StackUnlock unlock;
         env->CallVoidMethod(castingPlayerChangeListenerJavaObject, onChangedCallbackJavaMethodID, matterCastingPlayerJavaObject);
     }
 

diff --git a/...tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_connection_example.xml b/...tv-casting-app/android/App/app/src/main/res/layout/fragment_matter_connection_example.xml
@@ -7,15 +7,15 @@
     android:padding="10sp">
 
     <TextView
-        android:id="@+id/connectionWindowStatus"
+        android:id="@+id/connectionFragmentStatusText"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:textSize="24sp"/>
 
     <Button
         android:enabled="false"
-        android:id="@+id/connectionWindowNextButton"
-        android:layout_below="@id/connectionWindowStatus"
+        android:id="@+id/connectionFragmentNextButton"
+        android:layout_below="@id/connectionFragmentStatusText"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         android:text="@string/matter_connection_next_button_text" />