Improve Java/Jni IM read/subscribe (#25666)

* Improve Java/Jni IM read/subscribe

-- Add auto-resubscribe capability for subscribe
-- Refactor read/subscribe with better exception check.
-- manually validate the resubscription via bring up/down/up
all-cluster-app
-- with custom cluster, report would supress the error generated from
clusterObjects.
-- Fix across platform::delete usage in jni
-- add imTimeout parameter for read/subscribe

* disable cluster state data cache in java/jni

-- Disable cluster state data chace in java/jni and only use event
cache, version cache from cluster state cache, since java also have
cache to store the data

* Fix android chip-tool build failure

---------

Co-authored-by: Restyled.io <commits@restyled.io>

examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/WildcardFragment.kt

@@ -179,7 +179,8 @@ class WildcardFragment : Fragment() {
                                        minInterval,
                                        maxInterval,
                                        keepSubscriptions,
-                                       isFabricFiltered)
+                                       isFabricFiltered,
+                                       /* imTimeoutMs= */ 0)
     } else if (type == EVENT) {
       val eventPath = ChipEventPath.newInstance(endpointId, clusterId, eventId, isUrgent)
       deviceController.subscribeToPath(subscriptionEstablishedCallback,
@@ -192,7 +193,8 @@ class WildcardFragment : Fragment() {
                                       minInterval,
                                       maxInterval,
                                       keepSubscriptions,
-                                      isFabricFiltered)
+                                      isFabricFiltered,
+                                      /* imTimeoutMs= */ 0)
     }
   }
 
@@ -209,15 +211,17 @@ class WildcardFragment : Fragment() {
                           addressUpdateFragment.deviceId),
                           listOf(attributePath),
                           null,
-                          isFabricFiltered)
+                          isFabricFiltered,
+                          /* imTimeoutMs= */ 0)
     } else if (type == EVENT) {
       val eventPath = ChipEventPath.newInstance(endpointId, clusterId, eventId)
       deviceController.readPath(reportCallback,
                           ChipClient.getConnectedDevicePointer(requireContext(),
                           addressUpdateFragment.deviceId),
                           null,
                           listOf(eventPath),
-                          isFabricFiltered)
+                          isFabricFiltered,
+                          /* imTimeoutMs= */ 0)
     }
   }
 

examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt

@@ -66,7 +66,7 @@ class PairOnNetworkLongImReadCommand(
       .getConnectedDevicePointer(getNodeId(), InternalGetConnectedDeviceCallback())
     clear()
     currentCommissioner()
-      .readPath(InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), false)
+      .readPath(InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), false, 0)
     waitCompleteMs(getTimeoutMillis())
   }
 

examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImSubscribeCommand.kt

@@ -81,7 +81,7 @@ class PairOnNetworkLongImSubscribeCommand(
       .getConnectedDevicePointer(getNodeId(), InternalGetConnectedDeviceCallback())
     clear()
     currentCommissioner()
-      .subscribeToPath(InternalSubscriptionEstablishedCallback(), InternalResubscriptionAttemptCallback(), InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), 0, 5, false, false)
+      .subscribeToPath(InternalSubscriptionEstablishedCallback(), InternalResubscriptionAttemptCallback(), InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), 0, 5, false, false, 0)
     waitCompleteMs(getTimeoutMillis())
     currentCommissioner().shutdownSubscriptions(currentCommissioner().getFabricIndex(), getNodeId(), subscriptionId);
   }

src/controller/java/AndroidCallbacks-JNI.cpp

@@ -38,7 +38,7 @@ JNI_METHOD(void, GetConnectedDeviceCallbackJni, deleteCallback)(JNIEnv * env, jo
     chip::DeviceLayer::StackLock lock;
     GetConnectedDeviceCallback * connectedDeviceCallback = reinterpret_cast<GetConnectedDeviceCallback *>(callbackHandle);
     VerifyOrReturn(connectedDeviceCallback != nullptr, ChipLogError(Controller, "GetConnectedDeviceCallback handle is nullptr"));
-    delete connectedDeviceCallback;
+    chip::Platform::Delete(connectedDeviceCallback);
 }
 
 JNI_METHOD(jlong, ReportCallbackJni, newCallback)
@@ -56,7 +56,7 @@ JNI_METHOD(void, ReportCallbackJni, deleteCallback)(JNIEnv * env, jobject self,
     chip::DeviceLayer::StackLock lock;
     ReportCallback * reportCallback = reinterpret_cast<ReportCallback *>(callbackHandle);
     VerifyOrReturn(reportCallback != nullptr, ChipLogError(Controller, "ReportCallback handle is nullptr"));
-    delete reportCallback;
+    chip::Platform::Delete(reportCallback);
 }
 
 JNI_METHOD(jlong, ReportEventCallbackJni, newCallback)
@@ -91,7 +91,7 @@ JNI_METHOD(void, WriteAttributesCallbackJni, deleteCallback)(JNIEnv * env, jobje
     chip::DeviceLayer::StackLock lock;
     WriteAttributesCallback * writeAttributesCallback = reinterpret_cast<WriteAttributesCallback *>(callbackHandle);
     VerifyOrReturn(writeAttributesCallback != nullptr, ChipLogError(Controller, "WriteAttributesCallback handle is nullptr"));
-    delete writeAttributesCallback;
+    chip::Platform::Delete(writeAttributesCallback);
 }
 
 JNI_METHOD(jlong, InvokeCallbackJni, newCallback)
@@ -107,5 +107,5 @@ JNI_METHOD(void, InvokeCallbackJni, deleteCallback)(JNIEnv * env, jobject self,
     chip::DeviceLayer::StackLock lock;
     InvokeCallback * invokeCallback = reinterpret_cast<InvokeCallback *>(callbackHandle);
     VerifyOrReturn(invokeCallback != nullptr, ChipLogError(Controller, "InvokeCallback handle is nullptr"));
-    delete invokeCallback;
+    chip::Platform::Delete(invokeCallback);
 }

src/controller/java/AndroidCallbacks.cpp

@@ -123,7 +123,7 @@ void GetConnectedDeviceCallback::OnDeviceConnectionFailureFn(void * context, con
 
 ReportCallback::ReportCallback(jobject wrapperCallback, jobject subscriptionEstablishedCallback, jobject reportCallback,
                                jobject resubscriptionAttemptCallback) :
-    mClusterCacheAdapter(*this)
+    mClusterCacheAdapter(*this, Optional<EventNumber>::Missing(), false /*cacheData*/)
 {
     JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
     VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread"));
@@ -186,6 +186,19 @@ void ReportCallback::OnReportBegin()
     mNodeStateObj = env->NewObject(mNodeStateCls, nodeStateCtor, map);
 }
 
+void ReportCallback::OnDeallocatePaths(app::ReadPrepareParams && aReadPrepareParams)
+{
+    if (aReadPrepareParams.mpAttributePathParamsList != nullptr)
+    {
+        delete[] aReadPrepareParams.mpAttributePathParamsList;
+    }
+
+    if (aReadPrepareParams.mpEventPathParamsList != nullptr)
+    {
+        delete[] aReadPrepareParams.mpEventPathParamsList;
+    }
+}
+
 void ReportCallback::OnReportEnd()
 {
     UpdateClusterDataVersion();
@@ -239,8 +252,12 @@ void ReportCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPat
     readerForJson.Init(*apData);
 
     jobject value = DecodeAttributeValue(aPath, readerForJavaObject, &err);
-    // If we don't know this attribute, just skip it.
-    VerifyOrReturn(err != CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB);
+    // If we don't know this attribute, suppress it.
+    if (err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB)
+    {
+        err = CHIP_NO_ERROR;
+    }
+
     VerifyOrReturn(err == CHIP_NO_ERROR, ReportError(attributePathObj, nullptr, err));
     VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe(),
                    ReportError(attributePathObj, nullptr, CHIP_JNI_ERROR_EXCEPTION_THROWN));

src/controller/java/AndroidCallbacks.h

@@ -69,6 +69,8 @@ struct ReportCallback : public app::ClusterStateCache::Callback
 
     CHIP_ERROR OnResubscriptionNeeded(app::ReadClient * apReadClient, CHIP_ERROR aTerminationCause) override;
 
+    void OnDeallocatePaths(app::ReadPrepareParams && aReadPrepareParams) override;
+
     /** Report errors back to Java layer. attributePath may be nullptr for general errors. */
     void ReportError(jobject attributePath, jobject eventPath, CHIP_ERROR err);
     void ReportError(jobject attributePath, jobject eventPath, Protocols::InteractionModel::Status status);

src/controller/java/CHIPDeviceController-JNI.cpp

@@ -1355,104 +1355,156 @@ JNI_METHOD(jobject, computePaseVerifier)
 
 JNI_METHOD(void, subscribe)
 (JNIEnv * env, jobject self, jlong handle, jlong callbackHandle, jlong devicePtr, jobject attributePathList, jobject eventPathList,
- jint minInterval, jint maxInterval, jboolean keepSubscriptions, jboolean isFabricFiltered)
+ jint minInterval, jint maxInterval, jboolean keepSubscriptions, jboolean isFabricFiltered, jint imTimeoutMs)
 {
     chip::DeviceLayer::StackLock lock;
-    CHIP_ERROR err = CHIP_NO_ERROR;
-
-    DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
+    CHIP_ERROR err               = CHIP_NO_ERROR;
+    app::ReadClient * readClient = nullptr;
+    jint numAttributePaths       = 0;
+    jint numEventPaths           = 0;
+    auto callback                = reinterpret_cast<ReportCallback *>(callbackHandle);
+    DeviceProxy * device         = reinterpret_cast<DeviceProxy *>(devicePtr);
     if (device == nullptr)
     {
-        ChipLogError(Controller, "No device found");
+        ChipLogProgress(Controller, "Could not cast device pointer to Device object");
         JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, CHIP_ERROR_INCORRECT_STATE);
         return;
     }
+    app::ReadPrepareParams params(device->GetSecureSession().Value());
 
-    std::vector<app::AttributePathParams> attributePathParamsList;
-    err = ParseAttributePathList(attributePathList, attributePathParamsList);
-    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java attribute paths: %s", ErrorStr(err)));
+    params.mMinIntervalFloorSeconds   = minInterval;
+    params.mMaxIntervalCeilingSeconds = maxInterval;
+    params.mKeepSubscriptions         = (keepSubscriptions != JNI_FALSE);
+    params.mIsFabricFiltered          = (isFabricFiltered != JNI_FALSE);
+    params.mTimeout                   = imTimeoutMs != 0 ? System::Clock::Milliseconds32(imTimeoutMs) : System::Clock::kZero;
 
-    std::vector<app::EventPathParams> eventPathParamsList;
-    err = ParseEventPathList(eventPathList, eventPathParamsList);
-    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java event paths: %s", ErrorStr(err)));
+    SuccessOrExit(err = JniReferences::GetInstance().GetListSize(attributePathList, numAttributePaths));
+    if (numAttributePaths > 0)
+    {
+        std::unique_ptr<chip::app::AttributePathParams[]> attributePaths(new chip::app::AttributePathParams[numAttributePaths]);
+        for (uint8_t i = 0; i < numAttributePaths; i++)
+        {
+            jobject attributePathItem = nullptr;
+            SuccessOrExit(err = JniReferences::GetInstance().GetListItem(attributePathList, i, attributePathItem));
+
+            EndpointId endpointId;
+            ClusterId clusterId;
+            AttributeId attributeId;
+            SuccessOrExit(err = ParseAttributePath(attributePathItem, endpointId, clusterId, attributeId));
+            attributePaths[i] = chip::app::AttributePathParams(endpointId, clusterId, attributeId);
+        }
+        params.mpAttributePathParamsList    = attributePaths.get();
+        params.mAttributePathParamsListSize = numAttributePaths;
+        attributePaths.release();
+    }
 
-    app::ReadPrepareParams params(device->GetSecureSession().Value());
-    params.mMinIntervalFloorSeconds     = minInterval;
-    params.mMaxIntervalCeilingSeconds   = maxInterval;
-    params.mpAttributePathParamsList    = attributePathParamsList.data();
-    params.mAttributePathParamsListSize = attributePathParamsList.size();
-    params.mpEventPathParamsList        = eventPathParamsList.data();
-    params.mEventPathParamsListSize     = eventPathParamsList.size();
-    params.mKeepSubscriptions           = (keepSubscriptions != JNI_FALSE);
-    params.mIsFabricFiltered            = (isFabricFiltered != JNI_FALSE);
+    SuccessOrExit(err = JniReferences::GetInstance().GetListSize(eventPathList, numEventPaths));
+    if (numEventPaths > 0)
+    {
+        std::unique_ptr<chip::app::EventPathParams[]> eventPaths(new chip::app::EventPathParams[numEventPaths]);
+        for (uint8_t i = 0; i < numEventPaths; i++)
+        {
+            jobject eventPathItem = nullptr;
+            SuccessOrExit(err = JniReferences::GetInstance().GetListItem(eventPathList, i, eventPathItem));
+
+            EndpointId endpointId;
+            ClusterId clusterId;
+            EventId eventId;
+            bool isUrgent;
+            SuccessOrExit(err = ParseEventPath(eventPathItem, endpointId, clusterId, eventId, isUrgent));
+            eventPaths[i] = chip::app::EventPathParams(endpointId, clusterId, eventId, isUrgent);
+        }
 
-    auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
+        params.mpEventPathParamsList    = eventPaths.get();
+        params.mEventPathParamsListSize = numEventPaths;
+        eventPaths.release();
+    }
+
+    readClient = Platform::New<app::ReadClient>(app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
+                                                callback->mClusterCacheAdapter.GetBufferedCallback(),
+                                                app::ReadClient::InteractionType::Subscribe);
 
-    app::ReadClient * readClient = Platform::New<app::ReadClient>(
-        app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
-        callback->mClusterCacheAdapter.GetBufferedCallback(), app::ReadClient::InteractionType::Subscribe);
+    SuccessOrExit(err = readClient->SendAutoResubscribeRequest(std::move(params)));
+    callback->mReadClient = readClient;
 
-    err = readClient->SendRequest(params);
+exit:
     if (err != CHIP_NO_ERROR)
     {
+        ChipLogError(Controller, "JNI IM Subscribe Error: %s", err.AsString());
+        if (err == CHIP_JNI_ERROR_EXCEPTION_THROWN)
+        {
+            env->ExceptionDescribe();
+            env->ExceptionClear();
+        }
         callback->OnError(err);
-        delete readClient;
-        delete callback;
-        return;
+        if (readClient != nullptr)
+        {
+            Platform::Delete(readClient);
+        }
+        if (callback != nullptr)
+        {
+            Platform::Delete(callback);
+        }
     }
-
-    callback->mReadClient = readClient;
 }
 
 JNI_METHOD(void, read)
 (JNIEnv * env, jobject self, jlong handle, jlong callbackHandle, jlong devicePtr, jobject attributePathList, jobject eventPathList,
- jboolean isFabricFiltered)
+ jboolean isFabricFiltered, jint imTimeoutMs)
 {
     chip::DeviceLayer::StackLock lock;
     CHIP_ERROR err = CHIP_NO_ERROR;
 
-    DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
+    auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
+    std::vector<app::AttributePathParams> attributePathParamsList;
+    std::vector<app::EventPathParams> eventPathParamsList;
+    app::ReadClient * readClient = nullptr;
+    DeviceProxy * device         = reinterpret_cast<DeviceProxy *>(devicePtr);
     if (device == nullptr)
     {
-        ChipLogError(Controller, "No device found");
+        ChipLogProgress(Controller, "Could not cast device pointer to Device object");
         JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, CHIP_ERROR_INCORRECT_STATE);
         return;
     }
-
-    std::vector<app::AttributePathParams> attributePathParamsList;
-    err = ParseAttributePathList(attributePathList, attributePathParamsList);
-    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java attribute paths: %s", ErrorStr(err)));
-
-    std::vector<app::EventPathParams> eventPathParamsList;
-    err = ParseEventPathList(eventPathList, eventPathParamsList);
-    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java event paths: %s", ErrorStr(err)));
-
-    VerifyOrReturn(attributePathParamsList.size() != 0 || eventPathParamsList.size() != 0,
-                   ChipLogError(Controller, "Error parsing Java both event paths and attribute paths"));
     app::ReadPrepareParams params(device->GetSecureSession().Value());
+
+    SuccessOrExit(err = ParseAttributePathList(attributePathList, attributePathParamsList));
+    SuccessOrExit(err = ParseEventPathList(eventPathList, eventPathParamsList));
+    VerifyOrExit(attributePathParamsList.size() != 0 || eventPathParamsList.size() != 0, err = CHIP_ERROR_INVALID_ARGUMENT);
     params.mpAttributePathParamsList    = attributePathParamsList.data();
     params.mAttributePathParamsListSize = attributePathParamsList.size();
     params.mpEventPathParamsList        = eventPathParamsList.data();
     params.mEventPathParamsListSize     = eventPathParamsList.size();
 
     params.mIsFabricFiltered = (isFabricFiltered != JNI_FALSE);
+    params.mTimeout          = imTimeoutMs != 0 ? System::Clock::Milliseconds32(imTimeoutMs) : System::Clock::kZero;
 
-    auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
+    readClient = Platform::New<app::ReadClient>(app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
+                                                callback->mClusterCacheAdapter.GetBufferedCallback(),
+                                                app::ReadClient::InteractionType::Read);
 
-    app::ReadClient * readClient = Platform::New<app::ReadClient>(
-        app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
-        callback->mClusterCacheAdapter.GetBufferedCallback(), app::ReadClient::InteractionType::Read);
+    SuccessOrExit(err = readClient->SendRequest(params));
+    callback->mReadClient = readClient;
 
-    err = readClient->SendRequest(params);
+exit:
     if (err != CHIP_NO_ERROR)
     {
+        ChipLogError(Controller, "JNI IM Read Error: %s", err.AsString());
+        if (err == CHIP_JNI_ERROR_EXCEPTION_THROWN)
+        {
+            env->ExceptionDescribe();
+            env->ExceptionClear();
+        }
         callback->OnError(err);
-        delete readClient;
-        delete callback;
-        return;
+        if (readClient != nullptr)
+        {
+            Platform::Delete(readClient);
+        }
+        if (callback != nullptr)
+        {
+            Platform::Delete(callback);
+        }
     }
-
-    callback->mReadClient = readClient;
 }
 
 JNI_METHOD(void, write)
@@ -1576,11 +1628,11 @@ JNI_METHOD(void, write)
         callback->OnError(writeClient, err);
         if (writeClient != nullptr)
         {
-            delete writeClient;
+            Platform::Delete(writeClient);
         }
         if (callback != nullptr)
         {
-            delete callback;
+            Platform::Delete(callback);
         }
     }
 }
@@ -1684,11 +1736,11 @@ JNI_METHOD(void, invoke)
         callback->OnError(nullptr, err);
         if (commandSender != nullptr)
         {
-            delete commandSender;
+            Platform::Delete(commandSender);
         }
         if (callback != nullptr)
         {
-            delete callback;
+            Platform::Delete(callback);
         }
     }
 }