[Android] Use InvokeCommand and support nullable/optional for commands

src/controller/java/BUILD.gn

@@ -34,6 +34,8 @@ shared_library("jni") {
     "CHIPDeviceController-JNI.cpp",
     "zap-generated/CHIPClusters-JNI.cpp",
     "zap-generated/CHIPClustersRead-JNI.cpp",
+    "zap-generated/CHIPInvokeCallbacks.cpp",
+    "zap-generated/CHIPInvokeCallbacks.h",
     "zap-generated/CHIPReadCallbacks.cpp",
     "zap-generated/CHIPReadCallbacks.h",
   ]

src/controller/java/templates/CHIPCallbackTypes.zapt

@@ -0,0 +1,14 @@
+{{> header}}
+{{#if (chip_has_client_clusters)}}
+#include <app/util/af-enums.h>
+#include <app-common/zap-generated/cluster-objects.h>
+
+typedef void (*CHIPDefaultSuccessCallbackType)(void *, const chip::app::DataModel::NullObjectType &);
+typedef void (*CHIPDefaultFailureCallbackType)(void *, EmberAfStatus);
+
+{{#chip_client_clusters}}
+{{#chip_cluster_responses}}
+typedef void (*CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}CallbackType)(void *, const chip::app::Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::DecodableType &);
+{{/chip_cluster_responses}}
+{{/chip_client_clusters}}
+{{/if}}

src/controller/java/templates/CHIPClusters-JNI.zapt

@@ -1,5 +1,7 @@
 {{> header}}
 {{#if (chip_has_client_clusters)}}
+#include "CHIPCallbackTypes.h"
+#include "CHIPInvokeCallbacks.h"
 #include "CHIPReadCallbacks.h"
 
 #include <app-common/zap-generated/cluster-objects.h>
@@ -8,7 +10,6 @@
 
 #include <controller/java/AndroidClusterExceptions.h>
 #include <controller/java/CHIPDefaultCallbacks.h>
-#include <lib/support/CHIPJNIError.h>
 #include <lib/support/JniReferences.h>
 #include <lib/support/JniTypeWrappers.h>
 #include <jni.h>
@@ -22,110 +23,6 @@
 using namespace chip;
 using namespace chip::Controller;
 
-{{! TODO(#8773): Clean up callbacks. }}
-
-{{#chip_client_clusters}}
-{{#chip_cluster_responses}}
-class CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback : public Callback::Callback<{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback>
-{
-    public:
-        CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(jobject javaCallback): Callback::Callback<{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback>(CallbackFn, this)
-        {
-            JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
-            if (env == nullptr) {
-                ChipLogError(Zcl, "Could not create global reference for Java callback");
-                return;
-            }
-
-            javaCallbackRef = env->NewGlobalRef(javaCallback);
-            if (javaCallbackRef == nullptr) {
-                ChipLogError(Zcl, "Could not create global reference for Java callback");
-            }
-        }
-        ~CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback()
-        {
-            JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
-            if (env == nullptr) {
-                ChipLogError(Zcl, "Could not create global reference for Java callback");
-                return;
-            }
-            env->DeleteGlobalRef(javaCallbackRef);
-        };
-
-        static void CallbackFn(void * context{{#chip_cluster_response_arguments}}, {{asUnderlyingZclType type}} {{asSymbol label}}{{/chip_cluster_response_arguments}})
-        {
-            chip::DeviceLayer::StackUnlock unlock;
-            CHIP_ERROR err = CHIP_NO_ERROR;
-            JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
-            jobject javaCallbackRef;
-            jmethodID javaMethod;
-            CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback * cppCallback = nullptr;
-            {{#chip_cluster_response_arguments}}
-            {{#if (isOctetString type)}}
-            jbyteArray {{asSymbol label}}Arr;
-            {{else if (isShortString type)}}
-            UtfString {{asSymbol label}}Str(env, {{asSymbol label}});
-            {{/if}}
-            {{/chip_cluster_response_arguments}}
-
-            VerifyOrExit(env != nullptr, err = CHIP_JNI_ERROR_NO_ENV);
-
-            cppCallback = reinterpret_cast<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback *>(context);
-            VerifyOrExit(cppCallback != nullptr, err = CHIP_JNI_ERROR_NULL_OBJECT);
-
-            javaCallbackRef = cppCallback->javaCallbackRef;
-            VerifyOrExit(javaCallbackRef != nullptr, err = CHIP_NO_ERROR);
-
-            err = JniReferences::GetInstance().FindMethod(env, javaCallbackRef, "onSuccess", "({{#chip_cluster_response_arguments}}{{#if isArray}}{{else if (isOctetString type)}}[B{{else if (isShortString type)}}Ljava/lang/String;{{else}}{{asJniSignature type false}}{{/if}}{{/chip_cluster_response_arguments}})V", &javaMethod);
-            SuccessOrExit(err);
-
-            {{#chip_cluster_response_arguments}}
-            {{#if (isOctetString type)}}
-            {{asSymbol label}}Arr = env->NewByteArray({{asSymbol label}}.size());
-            VerifyOrExit({{asSymbol label}}Arr != nullptr, err = CHIP_ERROR_NO_MEMORY);
-            env->ExceptionClear();
-            env->SetByteArrayRegion({{asSymbol label}}Arr, 0, {{asSymbol label}}.size(), reinterpret_cast<const jbyte *>({{asSymbol label}}.data()));
-            VerifyOrExit(!env->ExceptionCheck(), err = CHIP_JNI_ERROR_EXCEPTION_THROWN);
-            {{/if}}
-            {{/chip_cluster_response_arguments}}
-
-            env->CallVoidMethod(javaCallbackRef, javaMethod
-                {{#chip_cluster_response_arguments}}
-                {{#if isArray}}
-                // {{asSymbol label}}: {{asUnderlyingZclType type}}
-                // Conversion from this type to Java is not properly implemented yet
-                {{else if (isOctetString type)}}
-                , {{asSymbol label}}Arr
-                {{else if (isShortString type)}}
-                , {{asSymbol label}}Str.jniValue()
-                {{else}}
-                , static_cast<{{asJniBasicTypeForZclType type}}>({{asSymbol label}})
-                {{/if}}
-                {{/chip_cluster_response_arguments}}
-            );
-
-            {{#chip_cluster_response_arguments}}
-            {{#if (isOctetString type)}}
-            env->DeleteLocalRef({{asSymbol label}}Arr);
-            {{/if}}
-            {{/chip_cluster_response_arguments}}
-
-        exit:
-            if (err != CHIP_NO_ERROR) {
-                ChipLogError(Zcl, "Error invoking Java callback: %" CHIP_ERROR_FORMAT, err.Format());
-            }
-            if (cppCallback != nullptr) {
-                cppCallback->Cancel();
-                delete cppCallback;
-            }
-        }
-
-    private:
-        jobject javaCallbackRef;
-};
-
-{{/chip_cluster_responses}}
-{{/chip_client_clusters}}
 JNI_METHOD(void, BaseChipCluster, deleteCluster)(JNIEnv * env, jobject self, jlong clusterPtr)
 {
     chip::DeviceLayer::StackLock lock;
@@ -146,58 +43,37 @@ JNI_METHOD(jlong, {{asUpperCamelCase name}}Cluster, initWithDevice)(JNIEnv * env
 }
 
 {{#chip_cluster_commands}}
-JNI_METHOD(void, {{asUpperCamelCase ../name}}Cluster, {{asLowerCamelCase name}})(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback{{#chip_cluster_command_arguments_with_structs_expanded}}, {{asJniBasicType type false}} {{asLowerCamelCase label}}{{/chip_cluster_command_arguments_with_structs_expanded}})
+JNI_METHOD(void, {{asUpperCamelCase ../name}}Cluster, {{asLowerCamelCase name}})(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback{{#chip_cluster_command_arguments_with_structs_expanded}}, {{asJniBasicType type true}} {{asLowerCamelCase label}}{{/chip_cluster_command_arguments_with_structs_expanded}})
 {
     chip::DeviceLayer::StackLock lock;
     CHIP_ERROR err = CHIP_NO_ERROR;
     {{asUpperCamelCase ../name}}Cluster * cppCluster;
 
-    {{#chip_cluster_command_arguments_with_structs_expanded}}
-    {{#if (isOctetString type)}}
-    JniByteArray {{asLowerCamelCase label}}Arr(env, {{asLowerCamelCase label}});
-    {{else if (isCharString type)}}
-    JniUtfString {{asLowerCamelCase label}}Str(env, {{asLowerCamelCase label}});
-    {{/if}}
-    {{/chip_cluster_command_arguments_with_structs_expanded}}
+    chip::app::Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::Type request;
 
-    {{#if hasSpecificResponse}}
-    std::unique_ptr<CHIP{{asCamelCased parent.name false}}Cluster{{asCamelCased responseName false}}Callback, void (*)(CHIP{{asCamelCased parent.name false}}Cluster{{asCamelCased responseName false}}Callback *)> onSuccess(
-        Platform::New<CHIP{{asCamelCased parent.name false}}Cluster{{asCamelCased responseName false}}Callback>(callback), Platform::Delete<CHIP{{asCamelCased parent.name false}}Cluster{{asCamelCased responseName false}}Callback>);
-    {{else}}
-    std::unique_ptr<CHIPDefaultSuccessCallback, void (*)(CHIPDefaultSuccessCallback *)> onSuccess(Platform::New<CHIPDefaultSuccessCallback>(callback), Platform::Delete<CHIPDefaultSuccessCallback>);
-    {{/if}}
+    {{#chip_cluster_command_arguments}}
+    {{>encode_value target=(concat "request." (asLowerCamelCase label)) source=(asLowerCamelCase label)}}
+    {{/chip_cluster_command_arguments}}
+
+    {{#*inline "callbackName"}}{{#if hasSpecificResponse}}{{asUpperCamelCase parent.name false}}Cluster{{asUpperCamelCase responseName false}}{{else}}DefaultSuccess{{/if}}{{/inline}}
+
+    std::unique_ptr<CHIP{{>callbackName}}Callback, void (*)(CHIP{{>callbackName}}Callback *)> onSuccess(
+        Platform::New<CHIP{{>callbackName}}Callback>(callback), Platform::Delete<CHIP{{>callbackName}}Callback>);
     std::unique_ptr<CHIPDefaultFailureCallback, void (*)(CHIPDefaultFailureCallback *)> onFailure(Platform::New<CHIPDefaultFailureCallback>(callback), Platform::Delete<CHIPDefaultFailureCallback>);
-    VerifyOrExit(onSuccess.get() != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
-    VerifyOrExit(onFailure.get() != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturn(onSuccess.get() != nullptr, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native callback", CHIP_ERROR_NO_MEMORY));
+    VerifyOrReturn(onFailure.get() != nullptr, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error creating native callback", CHIP_ERROR_NO_MEMORY));
 
     cppCluster = reinterpret_cast<{{asUpperCamelCase ../name}}Cluster *>(clusterPtr);
-    VerifyOrExit(cppCluster != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
-
-    err = cppCluster->{{asCamelCased name false}}(onSuccess->Cancel(), onFailure->Cancel()
-    {{#chip_cluster_command_arguments_with_structs_expanded}}, {{#if_chip_enum type}}static_cast<{{chipType}}>({{asLowerCamelCase label}}){{else if (isOctetString type)}}{{asUnderlyingZclType type}}((const uint8_t*) {{asLowerCamelCase label}}Arr.data(), {{asLowerCamelCase label}}Arr.size()){{else if (isCharString type)}}chip::CharSpan({{asLowerCamelCase label}}Str.c_str(), strlen({{asLowerCamelCase label}}Str.c_str())){{else}}{{asLowerCamelCase label}}{{/if_chip_enum}}{{/chip_cluster_command_arguments_with_structs_expanded}});
-    SuccessOrExit(err);
+    VerifyOrReturn(cppCluster != nullptr, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error getting native cluster", CHIP_ERROR_INCORRECT_STATE));
 
-exit: 
-    if (err != CHIP_NO_ERROR) {
-        jthrowable exception;
-        jmethodID method;
+    auto successFn = chip::Callback::Callback<CHIP{{>callbackName}}CallbackType>::FromCancelable(onSuccess->Cancel());
+    auto failureFn = chip::Callback::Callback<CHIPDefaultFailureCallbackType>::FromCancelable(onFailure->Cancel());
 
-        err = JniReferences::GetInstance().FindMethod(env, callback, "onError", "(Ljava/lang/Exception;)V", &method);
-        if (err != CHIP_NO_ERROR) {
-            ChipLogError(Zcl, "Error throwing IllegalStateException %" CHIP_ERROR_FORMAT, err.Format());
-            return;
-        }
+    err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall);
+    VerifyOrReturn(err == CHIP_NO_ERROR, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error invoking command", CHIP_ERROR_INCORRECT_STATE));
 
-        err = chip::AndroidClusterExceptions::GetInstance().CreateIllegalStateException(env, "Error invoking cluster", err, exception);
-        if (err != CHIP_NO_ERROR) {
-            ChipLogError(Zcl, "Error throwing IllegalStateException %" CHIP_ERROR_FORMAT, err.Format());
-            return;
-        }
-        env->CallVoidMethod(callback, method, exception);
-    } else {
-        onSuccess.release();
-        onFailure.release();
-    }
+    onSuccess.release();
+    onFailure.release();
 }
 {{/chip_cluster_commands}}
 {{#chip_server_cluster_attributes}}

src/controller/java/templates/CHIPInvokeCallbacks-src.zapt

@@ -0,0 +1,76 @@
+{{> header}}
+{{#if (chip_has_client_clusters)}}
+#include "CHIPCallbackTypes.h"
+#include "CHIPInvokeCallbacks.h"
+
+#include <app-common/zap-generated/cluster-objects.h>
+#include <jni.h>
+#include <lib/support/CodeUtils.h>
+#include <lib/support/CHIPJNIError.h>
+#include <lib/support/JniReferences.h>
+#include <lib/support/JniTypeWrappers.h>
+#include <platform/PlatformManager.h>
+
+{{! TODO(#8773): Clean up callbacks. }}
+
+namespace chip {
+
+{{#chip_client_clusters}}
+{{#chip_cluster_responses}}
+CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback::CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(jobject javaCallback): Callback::Callback<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}CallbackType>(CallbackFn, this)
+{
+    JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+    if (env == nullptr) {
+        ChipLogError(Zcl, "Could not create global reference for Java callback");
+        return;
+    }
+
+    javaCallbackRef = env->NewGlobalRef(javaCallback);
+    if (javaCallbackRef == nullptr) {
+        ChipLogError(Zcl, "Could not create global reference for Java callback");
+    }
+}
+
+CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback::~CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback()
+{
+    JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+    if (env == nullptr) {
+        ChipLogError(Zcl, "Could not delete global reference for Java callback");
+        return;
+    }
+    env->DeleteGlobalRef(javaCallbackRef);
+};
+
+void CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback::CallbackFn(void * context, const chip::app::Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::DecodableType & dataResponse)
+{
+    chip::DeviceLayer::StackUnlock unlock;
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+    jobject javaCallbackRef;
+    jmethodID javaMethod;
+
+    VerifyOrReturn(env != nullptr, ChipLogError(Zcl, "Error invoking Java callback: no JNIEnv"));
+
+    std::unique_ptr<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback, void (*)(CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback *)> cppCallback(
+        reinterpret_cast<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback *>(context),
+        chip::Platform::Delete<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback>
+    );
+    VerifyOrReturn(cppCallback != nullptr, ChipLogError(Zcl, "Error invoking Java callback: failed to cast native callback"));
+
+    javaCallbackRef = cppCallback->javaCallbackRef;
+    // Java callback is allowed to be null, exit early if this is the case.
+    VerifyOrReturn(javaCallbackRef != nullptr);
+
+    err = JniReferences::GetInstance().FindMethod(env, javaCallbackRef, "onSuccess", "({{#chip_cluster_response_arguments}}{{#if isArray}}{{else if isOptional}}Ljava/util/Optional;{{else if (isOctetString type)}}[B{{else if (isShortString type)}}Ljava/lang/String;{{else}}{{asJniSignature type true}}{{/if}}{{/chip_cluster_response_arguments}})V", &javaMethod);
+    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Zcl, "Error invoking Java callback: %s", ErrorStr(err)));
+
+    {{#chip_cluster_response_arguments}}
+    {{>decode_value}}
+    {{/chip_cluster_response_arguments}}
+
+    env->CallVoidMethod(javaCallbackRef, javaMethod{{#chip_cluster_response_arguments}}, {{asSymbol label}}{{/chip_cluster_response_arguments}});
+}
+{{/chip_cluster_responses}}
+{{/chip_client_clusters}}
+} // namespace chip
+{{/if}}

src/controller/java/templates/CHIPInvokeCallbacks.zapt

@@ -0,0 +1,29 @@
+{{> header}}
+{{#if (chip_has_client_clusters)}}
+#include "CHIPCallbackTypes.h"
+
+#include <app-common/zap-generated/cluster-objects.h>
+#include <jni.h>
+#include <zap-generated/CHIPClientCallbacks.h>
+
+namespace chip {
+
+{{#chip_client_clusters}}
+{{#chip_cluster_responses}}
+class CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback : public Callback::Callback<CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}CallbackType>
+{
+public:
+    CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(jobject javaCallback);
+
+    ~CHIP{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback();
+
+    static void CallbackFn(void * context, const chip::app::Clusters::{{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::DecodableType & data);
+
+private:
+    jobject javaCallbackRef;
+};
+
+{{/chip_cluster_responses}}
+{{/chip_client_clusters}}
+} // namespace chip
+{{/if}}