[Andriod] Remove extra sub-interface layer from DeviceAttestationDelegate interface (project-chip#24771)

* Remove DeviceAttestationCompletionCallback and DeviceAttestationFailureCallback sub-interfaces from DeviceAttestationDelegate

* Address review comments

* Move DeviceController init from onCreateView() to onCreate()

Author: yufengwangca

examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/DeviceProvisioningFragment.kt

@@ -30,8 +30,8 @@ import androidx.appcompat.app.AlertDialog
 import androidx.fragment.app.Fragment
 import androidx.lifecycle.lifecycleScope
 import chip.devicecontroller.AttestationInfo
-import chip.devicecontroller.DeviceAttestationDelegate.DeviceAttestationCompletionCallback
-import chip.devicecontroller.DeviceAttestationDelegate.DeviceAttestationFailureCallback
+import chip.devicecontroller.ChipDeviceController
+import chip.devicecontroller.DeviceAttestationDelegate
 import chip.devicecontroller.NetworkCredentials
 import com.google.chip.chiptool.NetworkCredentialsParcelable
 import com.google.chip.chiptool.ChipClient
@@ -57,15 +57,23 @@ class DeviceProvisioningFragment : Fragment() {
   private val networkCredentialsParcelable: NetworkCredentialsParcelable?
     get() = arguments?.getParcelable(ARG_NETWORK_CREDENTIALS)
 
+  private lateinit var deviceController: ChipDeviceController
+
   private lateinit var scope: CoroutineScope
 
+  override fun onCreate(savedInstanceState: Bundle?) {
+    super.onCreate(savedInstanceState)
+    deviceController = ChipClient.getDeviceController(requireContext())
+  }
+
   override fun onCreateView(
     inflater: LayoutInflater,
     container: ViewGroup?,
     savedInstanceState: Bundle?
   ): View {
     scope = viewLifecycleOwner.lifecycleScope
     deviceInfo = checkNotNull(requireArguments().getParcelable(ARG_DEVICE_INFO))
+    
     return inflater.inflate(R.layout.single_fragment_container, container, false).apply {
       if (savedInstanceState == null) {
         if (deviceInfo.ipAddress != null) {
@@ -82,13 +90,65 @@ class DeviceProvisioningFragment : Fragment() {
     gatt = null
   }
 
+  override fun onDestroy() {
+    super.onDestroy()
+    deviceController.close()
+    deviceController.setDeviceAttestationDelegate(0, EmptyAttestationDelegate())
+  }
+
+  private class EmptyAttestationDelegate : DeviceAttestationDelegate {
+    override fun onDeviceAttestationCompleted(
+      devicePtr: Long,
+      attestationInfo: AttestationInfo,
+      errorCode: Int) {}
+  }
+
+  private fun setAttestationDelegate() {
+    deviceController.setDeviceAttestationDelegate(DEVICE_ATTESTATION_FAILED_TIMEOUT
+    ) { devicePtr, attestationInfo, errorCode ->
+      Log.i(TAG, "Device attestation errorCode: $errorCode, " +
+              "Look at 'src/credentials/attestation_verifier/DeviceAttestationVerifier.h' " +
+              "AttestationVerificationResult enum to understand the errors")
+
+      val activity = requireActivity()
+
+      if (errorCode == STATUS_PAIRING_SUCCESS) {       
+        activity.runOnUiThread(Runnable {
+          deviceController.continueCommissioning(devicePtr, true)
+        })
+
+        return@setDeviceAttestationDelegate
+      }
+
+      activity.runOnUiThread(Runnable {
+        val dialog = AlertDialog.Builder(activity)
+          .setPositiveButton("Continue",
+              DialogInterface.OnClickListener { dialog, id ->
+                deviceController.continueCommissioning(devicePtr, true)
+              })
+          .setNegativeButton("No",
+              DialogInterface.OnClickListener { dialog, id ->
+                deviceController.continueCommissioning(devicePtr, false)
+              })
+          .setTitle("Device Attestation")
+          .setMessage("Device Attestation failed for device under commissioning. Do you wish to continue pairing?")
+          .create()
+
+        dialog.show()
+      })
+    }
+  }
+
   private fun pairDeviceWithAddress() {
     // IANA CHIP port
     val port = 5540
     val id = DeviceIdUtil.getNextAvailableId(requireContext())
-    val deviceController = ChipClient.getDeviceController(requireContext())
+
     DeviceIdUtil.setNextAvailableId(requireContext(), id + 1)
     deviceController.setCompletionListener(ConnectionCallback())
+
+    setAttestationDelegate()
+
     deviceController.pairDeviceWithAddress(
       id,
       deviceInfo.ipAddress,
@@ -103,9 +163,7 @@ class DeviceProvisioningFragment : Fragment() {
     if (gatt != null) {
       return
     }
-
     scope.launch {
-      val deviceController = ChipClient.getDeviceController(requireContext())
       val bluetoothManager = BluetoothManager()
 
       showMessage(
@@ -135,36 +193,14 @@ class DeviceProvisioningFragment : Fragment() {
       if (wifi != null) {
         network = NetworkCredentials.forWiFi(NetworkCredentials.WiFiCredentials(wifi.ssid, wifi.password))
       }
+
       val thread = networkParcelable.threadCredentials
       if (thread != null) {
         network = NetworkCredentials.forThread(NetworkCredentials.ThreadCredentials(thread.operationalDataset))
       }
-      deviceController.setDeviceAttestationFailureCallback(DEVICE_ATTESTATION_FAILED_TIMEOUT
-      ) { devicePtr, errorCode ->
-        Log.i(TAG, "Device attestation errorCode: $errorCode, " +
-                "Look at 'src/credentials/attestation_verifier/DeviceAttestationVerifier.h' " +
-                "AttestationVerificationResult enum to understand the errors")
-        requireActivity().runOnUiThread(Runnable {
-          val alertDialog: AlertDialog? = activity?.let {
-            val builder = AlertDialog.Builder(it)
-            builder.apply {
-              setPositiveButton("Continue",
-                DialogInterface.OnClickListener { dialog, id ->
-                  deviceController.continueCommissioning(devicePtr, true)
-                })
-              setNegativeButton("No",
-                DialogInterface.OnClickListener { dialog, id ->
-                  deviceController.continueCommissioning(devicePtr, false)
-                })
-            }
-            builder.setTitle("Device Attestation")
-            builder.setMessage("Device Attestation failed for device under commissioning. Do you wish to continue pairing?")
-            // Create the AlertDialog
-            builder.create()
-          }
-          alertDialog?.show()
-        })
-      }
+
+      setAttestationDelegate()
+
       deviceController.pairDevice(gatt, connId, deviceId, deviceInfo.setupPinCode, network)
       DeviceIdUtil.setNextAvailableId(requireContext(), deviceId + 1)
     }

src/controller/java/CHIPDeviceController-JNI.cpp

@@ -1555,14 +1555,14 @@ CHIP_ERROR CreateDeviceAttestationDelegateBridge(JNIEnv * env, jlong handle, job
     CHIP_ERROR err                        = CHIP_NO_ERROR;
     chip::Optional<uint16_t> timeoutSecs  = chip::MakeOptional(static_cast<uint16_t>(failSafeExpiryTimeoutSecs));
     bool shouldWaitAfterDeviceAttestation = false;
-    jclass completionCallbackCls          = nullptr;
+    jclass deviceAttestationDelegateCls   = nullptr;
     jobject deviceAttestationDelegateRef  = env->NewGlobalRef(deviceAttestationDelegate);
+
     VerifyOrExit(deviceAttestationDelegateRef != nullptr, err = CHIP_JNI_ERROR_NULL_OBJECT);
-    JniReferences::GetInstance().GetClassRef(
-        env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationCompletionCallback", completionCallbackCls);
-    VerifyOrExit(completionCallbackCls != nullptr, err = CHIP_JNI_ERROR_TYPE_NOT_FOUND);
+    JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/DeviceAttestationDelegate", deviceAttestationDelegateCls);
+    VerifyOrExit(deviceAttestationDelegateCls != nullptr, err = CHIP_JNI_ERROR_TYPE_NOT_FOUND);
 
-    if (env->IsInstanceOf(deviceAttestationDelegate, completionCallbackCls))
+    if (env->IsInstanceOf(deviceAttestationDelegate, deviceAttestationDelegateCls))
     {
         shouldWaitAfterDeviceAttestation = true;
     }

src/controller/java/DeviceAttestationDelegateBridge.cpp

@@ -71,19 +71,15 @@ void DeviceAttestationDelegateBridge::OnDeviceAttestationCompleted(
     mResult = attestationResult;
     if (mDeviceAttestationDelegate != nullptr)
     {
-        JNIEnv * env                 = JniReferences::GetInstance().GetEnvForCurrentThread();
-        jclass completionCallbackCls = nullptr;
-        JniReferences::GetInstance().GetClassRef(
-            env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationCompletionCallback", completionCallbackCls);
-        VerifyOrReturn(completionCallbackCls != nullptr,
-                       ChipLogError(Controller, "Could not find device attestation completion callback class."));
-        jclass failureCallbackCls = nullptr;
-        JniReferences::GetInstance().GetClassRef(
-            env, "chip/devicecontroller/DeviceAttestationDelegate$DeviceAttestationFailureCallback", failureCallbackCls);
-        VerifyOrReturn(failureCallbackCls != nullptr,
-                       ChipLogError(Controller, "Could not find device attestation failure callback class."));
+        JNIEnv * env                        = JniReferences::GetInstance().GetEnvForCurrentThread();
+        jclass deviceAttestationDelegateCls = nullptr;
 
-        if (env->IsInstanceOf(mDeviceAttestationDelegate, completionCallbackCls))
+        JniReferences::GetInstance().GetClassRef(env, "chip/devicecontroller/DeviceAttestationDelegate",
+                                                 deviceAttestationDelegateCls);
+        VerifyOrReturn(deviceAttestationDelegateCls != nullptr,
+                       ChipLogError(Controller, "Could not find device attestation delegate class."));
+
+        if (env->IsInstanceOf(mDeviceAttestationDelegate, deviceAttestationDelegateCls))
         {
             jmethodID onDeviceAttestationCompletedMethod;
             JniReferences::GetInstance().FindMethod(env, mDeviceAttestationDelegate, "onDeviceAttestationCompleted",
@@ -98,17 +94,6 @@ void DeviceAttestationDelegateBridge::OnDeviceAttestationCompleted(
             env->CallVoidMethod(mDeviceAttestationDelegate, onDeviceAttestationCompletedMethod, reinterpret_cast<jlong>(device),
                                 javaAttestationInfo, static_cast<jint>(attestationResult));
         }
-        else if ((attestationResult != chip::Credentials::AttestationVerificationResult::kSuccess) &&
-                 env->IsInstanceOf(mDeviceAttestationDelegate, failureCallbackCls))
-        {
-            jmethodID onDeviceAttestationFailedMethod;
-            JniReferences::GetInstance().FindMethod(env, mDeviceAttestationDelegate, "onDeviceAttestationFailed", "(JI)V",
-                                                    &onDeviceAttestationFailedMethod);
-            VerifyOrReturn(onDeviceAttestationFailedMethod != nullptr,
-                           ChipLogError(Controller, "Could not find deviceAttestation failed method"));
-            env->CallVoidMethod(mDeviceAttestationDelegate, onDeviceAttestationFailedMethod, reinterpret_cast<jlong>(device),
-                                static_cast<jint>(attestationResult));
-        }
     }
 }
 

src/controller/java/src/chip/devicecontroller/ChipDeviceController.java

@@ -85,42 +85,23 @@ public void setNOCChainIssuer(NOCChainIssuer issuer) {
   }
 
   /**
-   * If DeviceAttestationCompletionCallback is setted, then it will always be called when device
-   * attestation completes.
+   * If DeviceAttestationDelegate is setted, then it will always be called when device attestation
+   * completes. In case the device attestation fails, the client can decide to continue or stop the
+   * commissioning.
    *
-   * <p>When {@link
-   * DeviceAttestationDelegate.DeviceAttestationCompletionCallback#onDeviceAttestationCompleted(long,
-   * long, AttestationInfo, int)} is received, {@link #continueCommissioning(long, boolean)} must be
+   * <p>When {@link DeviceAttestationDelegate#onDeviceAttestationCompleted(long, long,
+   * AttestationInfo, int)} is received, {@link #continueCommissioning(long, boolean)} must be
    * called.
    *
    * @param failSafeExpiryTimeoutSecs the value to set for the fail-safe timer before
    *     onDeviceAttestationCompleted is invoked. The unit is seconds.
-   * @param completionCallback the callback will be invoked when deviceattestation completed with
-   *     device info for additional verification.
+   * @param deviceAttestationDelegate the delegate for device attestation completed with device info
+   *     for additional verification.
    */
-  public void setDeviceAttestationCompletionCallback(
-      int failSafeExpiryTimeoutSecs,
-      DeviceAttestationDelegate.DeviceAttestationCompletionCallback completionCallback) {
+  public void setDeviceAttestationDelegate(
+      int failSafeExpiryTimeoutSecs, DeviceAttestationDelegate deviceAttestationDelegate) {
     setDeviceAttestationDelegate(
-        deviceControllerPtr, failSafeExpiryTimeoutSecs, completionCallback);
-  }
-
-  /**
-   * If DeviceAttestationFailureCallback is setted, then it will be called when device attestation
-   * fails, and the client can decide to continue or stop the commissioning.
-   *
-   * <p>When {@link
-   * DeviceAttestationDelegate.DeviceAttestationFailureCallback#onDeviceAttestationFailed(long,
-   * long, int)} is received, {@link #continueCommissioning(long, boolean)} must be called.
-   *
-   * @param failSafeExpiryTimeoutSecs the value to set for the fail-safe timer before
-   *     onDeviceAttestationFailed is invoked. The unit is seconds.
-   * @param failureCallback the callback will be invoked when device attestation failed.
-   */
-  public void setDeviceAttestationFailureCallback(
-      int failSafeExpiryTimeoutSecs,
-      DeviceAttestationDelegate.DeviceAttestationFailureCallback failureCallback) {
-    setDeviceAttestationDelegate(deviceControllerPtr, failSafeExpiryTimeoutSecs, failureCallback);
+        deviceControllerPtr, failSafeExpiryTimeoutSecs, deviceAttestationDelegate);
   }
 
   public void pairDevice(

src/controller/java/src/chip/devicecontroller/DeviceAttestationDelegate.java

@@ -1,17 +1,13 @@
 package chip.devicecontroller;
 
 /**
- * Only one of the following delegate callbacks should be implemented.
+ * Delegate for device attestation verifiers.
  *
- * <p>If one of the following callbacks is implemented, {@link
- * ChipDeviceController#continueCommissioning(long, boolean)} is expected to be called if
- * commissioning should continue.
+ * <p>If DeviceAttestationDelegate is implemented, then onDeviceAttestationCompleted will always be
+ * called when device attestation completes.
  *
- * <p>If DeviceAttestationCompletionCallback is implemented, then it will always be called when
- * device attestation completes.
- *
- * <p>If DeviceAttestationFailureCallback is implemented, then it will be called when device
- * attestation fails, and the client can decide to continue or stop the commissioning.
+ * <p>If device attestation fails, {@link ChipDeviceController#continueCommissioning(long, boolean)}
+ * is expected to be called to continue or stop the commissioning.
  *
  * <p>For example:
  *
@@ -24,30 +20,16 @@
  * </pre>
  */
 public interface DeviceAttestationDelegate {
-
-  public interface DeviceAttestationCompletionCallback extends DeviceAttestationDelegate {
-    /**
-     * The callback will be invoked when device attestation completed with device info for
-     * additional verification.
-     *
-     * <p>This allows the callback to stop commissioning after examining the device info (DAC, PAI,
-     * CD).
-     *
-     * @param devicePtr Handle of device being commissioned
-     * @param attestationInfo Attestation information for the device
-     * @param errorCode Error code on attestation failure. 0 if success.
-     */
-    void onDeviceAttestationCompleted(
-        long devicePtr, AttestationInfo attestationInfo, int errorCode);
-  }
-
-  public interface DeviceAttestationFailureCallback extends DeviceAttestationDelegate {
-    /**
-     * The callback will be invoked when device attestation failed
-     *
-     * @param devicePtr Handle of device being commissioned
-     * @param errorCode Error code for the failure.
-     */
-    void onDeviceAttestationFailed(long devicePtr, int errorCode);
-  }
+  /**
+   * The callback will be invoked when device attestation completed with device info for additional
+   * verification.
+   *
+   * <p>This allows the callback to stop commissioning after examining the device info (DAC, PAI,
+   * CD).
+   *
+   * @param devicePtr Handle of device being commissioned
+   * @param attestationInfo Attestation information for the device, null is errorCode is 0.
+   * @param errorCode Error code on attestation failure. 0 if succeed.
+   */
+  void onDeviceAttestationCompleted(long devicePtr, AttestationInfo attestationInfo, int errorCode);
 }