From fbf2d252623a9c21f1174e5baf0b217b1372fd94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Tue, 7 Jun 2022 15:11:36 +0200 Subject: [PATCH] [android] Fix handling optional values + UI fix. (#19237) * [nrf noup] [android] Use ScrollView in main screen Some buttons were not visible on low-resolution devices. Signed-off-by: Damian Krolik * [nrf noup] [android] Fix crash on DoorLock.lockDoor command Cluster Interaction Tool screen would crash when trying to send a command that takes an optional argument. The reason was that optional arguments were incorrectly converted in the application and the underlying JNI layer. Signed-off-by: Damian Krolik * Add IDL tests --- scripts/idl/BUILD.gn | 3 + .../idl/generators/java/ChipClustersCpp.jinja | 20 +- scripts/idl/tests/available_tests.yaml | 4 + .../idl/tests/inputs/optional_argument.matter | 7 + .../MyClusterClient-InvokeSubscribeImpl.cpp | 79 +++ .../jni/MyClusterClient-ReadImpl.cpp | 14 + .../ClusterDetailFragment.kt | 13 +- .../res/layout/select_action_fragment.xml | 182 ++--- .../clusterinfo/CommandParameterInfo.java | 8 +- .../java/templates/ClusterInfo-java.zapt | 2 +- .../ClusterInfo-write-interaction.zapt | 2 +- src/controller/java/templates/helper.js | 30 +- .../devicecontroller/ClusterInfoMapping.java | 655 +++++++++--------- .../devicecontroller/ClusterWriteMapping.java | 290 ++++---- src/lib/support/JniReferences.cpp | 2 +- 15 files changed, 719 insertions(+), 592 deletions(-) create mode 100644 scripts/idl/tests/inputs/optional_argument.matter create mode 100644 scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp create mode 100644 scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp diff --git a/scripts/idl/BUILD.gn b/scripts/idl/BUILD.gn index 16393d939c59e8..d17697ffb40f1b 100644 --- a/scripts/idl/BUILD.gn +++ b/scripts/idl/BUILD.gn @@ -32,12 +32,15 @@ pw_python_package("idl") { "tests/available_tests.yaml", "tests/inputs/cluster_struct_attribute.matter", "tests/inputs/global_struct_attribute.matter", + "tests/inputs/optional_argument.matter", "tests/inputs/several_clusters.matter", "tests/inputs/simple_attribute.matter", "tests/outputs/cluster_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp", "tests/outputs/cluster_struct_attribute/jni/DemoClusterClient-InvokeSubscribeImpl.cpp", "tests/outputs/global_struct_attribute/jni/DemoClusterClient-ReadImpl.cpp", "tests/outputs/global_struct_attribute/jni/DemoClusterClient-InvokeSubscribeImpl.cpp", + "tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp", + "tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp", "tests/outputs/several_clusters/jni/FirstClient-ReadImpl.cpp", "tests/outputs/several_clusters/jni/SecondClient-ReadImpl.cpp", "tests/outputs/several_clusters/jni/ThirdClient-ReadImpl.cpp", diff --git a/scripts/idl/generators/java/ChipClustersCpp.jinja b/scripts/idl/generators/java/ChipClustersCpp.jinja index 22c8bb7f0b91fa..d32f20c9f40c4d 100644 --- a/scripts/idl/generators/java/ChipClustersCpp.jinja +++ b/scripts/idl/generators/java/ChipClustersCpp.jinja @@ -1,14 +1,16 @@ {%- macro encode_optional(target, source, depth, encodable) -%} - if ({{source}} != nullptr) { - jobject optionalValue_{{depth}}; + { + jobject optionalValue_{{depth}} = nullptr; chip::JniReferences::GetInstance().GetOptionalValue({{source}}, optionalValue_{{depth}}); - auto & definedValue_{{depth}} = {{target}}.Emplace(); - {{ encode_value( - "definedValue_{}".format(depth), - "optionalValue_{}".format(depth), - depth+1, - encodable.without_optional() - )}} + if (optionalValue_{{depth}}) { + auto & definedValue_{{depth}} = {{target}}.Emplace(); + {{ encode_value( + "definedValue_{}".format(depth), + "optionalValue_{}".format(depth), + depth+1, + encodable.without_optional() + )}} + } } {%- endmacro %} diff --git a/scripts/idl/tests/available_tests.yaml b/scripts/idl/tests/available_tests.yaml index e0a6c39dc5b88e..107eb0ad4fb42a 100644 --- a/scripts/idl/tests/available_tests.yaml +++ b/scripts/idl/tests/available_tests.yaml @@ -30,3 +30,7 @@ java: jni/FirstClient-InvokeSubscribeImpl.cpp: outputs/several_clusters/jni/FirstClient-InvokeSubscribeImpl.cpp jni/SecondClient-InvokeSubscribeImpl.cpp: outputs/several_clusters/jni/SecondClient-InvokeSubscribeImpl.cpp jni/ThirdClient-InvokeSubscribeImpl.cpp: outputs/several_clusters/jni/ThirdClient-InvokeSubscribeImpl.cpp + + inputs/optional_argument.matter: + jni/MyClusterClient-ReadImpl.cpp: outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp + jni/MyClusterClient-InvokeSubscribeImpl.cpp: outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp diff --git a/scripts/idl/tests/inputs/optional_argument.matter b/scripts/idl/tests/inputs/optional_argument.matter new file mode 100644 index 00000000000000..2b14f9a5987b7c --- /dev/null +++ b/scripts/idl/tests/inputs/optional_argument.matter @@ -0,0 +1,7 @@ +client cluster MyCluster = 1 { + request struct FooRequest { + optional OCTET_STRING argument = 0; + } + + command Foo(FooRequest): DefaultSuccess = 0; +} \ No newline at end of file diff --git a/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp b/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp new file mode 100644 index 00000000000000..7c63c12f242f00 --- /dev/null +++ b/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp @@ -0,0 +1,79 @@ +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME) \ + extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME + +using namespace chip; +using namespace chip::Controller; + +JNI_METHOD(jlong, MyClusterCluster, initWithDevice)(JNIEnv * env, jobject self, jlong devicePtr, jint endpointId) +{ + chip::DeviceLayer::StackLock lock; + MyClusterCluster * cppCluster = new MyClusterCluster(); + + cppCluster->Associate(reinterpret_cast(devicePtr), endpointId); + return reinterpret_cast(cppCluster); +} + +JNI_METHOD(void, MyClusterCluster, + foo)(JNIEnv * env, jobject self, jlong clusterPtr, jobject callback,jobject argument,jobject timedInvokeTimeoutMs) +{ + chip::DeviceLayer::StackLock lock; + CHIP_ERROR err = CHIP_NO_ERROR; + MyClusterCluster * cppCluster; + + ListFreer listFreer; + chip::app::Clusters::MyCluster::Commands::Foo::Type request; + + std::vector> cleanupByteArrays; + std::vector> cleanupStrings;{ + jobject optionalValue_0 = nullptr; + chip::JniReferences::GetInstance().GetOptionalValue(argument, optionalValue_0); + if (optionalValue_0) { + auto & definedValue_0 = request.argument.Emplace(); + cleanupByteArrays.push_back(chip::Platform::MakeUnique(env, static_cast(optionalValue_0))); + definedValue_0 = cleanupByteArrays.back()->byteSpan(); + } + } + + + std::unique_ptr onSuccess( + Platform::New(callback), Platform::Delete); + std::unique_ptr onFailure(Platform::New(callback), Platform::Delete); + 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(clusterPtr); + VerifyOrReturn(cppCluster != nullptr, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error getting native cluster", CHIP_ERROR_INCORRECT_STATE)); + + auto successFn = chip::Callback::Callback::FromCancelable(onSuccess->Cancel()); + auto failureFn = chip::Callback::Callback::FromCancelable(onFailure->Cancel()); + + if (timedInvokeTimeoutMs == nullptr) { + err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall); + } else { + err = cppCluster->InvokeCommand(request, onSuccess->mContext, successFn->mCall, failureFn->mCall, chip::JniReferences::GetInstance().IntegerToPrimitive(timedInvokeTimeoutMs)); + } + VerifyOrReturn(err == CHIP_NO_ERROR, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error invoking command", CHIP_ERROR_INCORRECT_STATE)); + + onSuccess.release(); + onFailure.release(); +} diff --git a/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp b/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp new file mode 100644 index 00000000000000..6293b92540a107 --- /dev/null +++ b/scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp @@ -0,0 +1,14 @@ + +#include + +#include +#include + +#include +#include +#include +#include +#include + +#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME) \ + extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME \ No newline at end of file diff --git a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterDetailFragment.kt b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterDetailFragment.kt index 87321a93c40211..5e9769764a5cdc 100644 --- a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterDetailFragment.kt +++ b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterDetailFragment.kt @@ -45,6 +45,8 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.cancel import kotlinx.coroutines.launch +import java.util.* +import kotlin.collections.HashMap /** * ClusterDetailFragment allows user to pick cluster, command, specify parameters and see @@ -135,15 +137,15 @@ class ClusterDetailFragment : Fragment() { ) parameterList.forEach { val parameterName = it.clusterParameterNameTv.text.toString() - val castType = - selectedInteractionInfo.commandParameters[parameterName]!!.type - val data = castStringToType(it.clusterParameterData.text.toString(), castType)!! + val parameterType = selectedInteractionInfo.commandParameters[parameterName]!!.type + val parameterUnderlyingType = selectedInteractionInfo.commandParameters[parameterName]!!.underlyingType + val data = castStringToType(it.clusterParameterData.text.toString(), parameterType, parameterUnderlyingType)!! commandArguments[it.clusterParameterNameTv.text.toString()] = data clusterInteractionHistoryList[0].parameterList.add( HistoryParameterInfo( parameterName, data.toString(), - castType + parameterUnderlyingType ) ) } @@ -185,12 +187,13 @@ class ClusterDetailFragment : Fragment() { } } - private fun castStringToType(data: String, type: Class<*>): Any? { + private fun castStringToType(data: String, type: Class<*>, underlyingType: Class<*>): Any? { return when (type) { Int::class.java -> data.toInt() Boolean::class.java -> data.toBoolean() ByteArray::class.java -> data.encodeToByteArray() Long::class.java -> data.toLong() + Optional::class.java -> if (data.isEmpty()) Optional.empty() else Optional.of(castStringToType(data, underlyingType, underlyingType)!!) else -> data } } diff --git a/src/android/CHIPTool/app/src/main/res/layout/select_action_fragment.xml b/src/android/CHIPTool/app/src/main/res/layout/select_action_fragment.xml index 89b4ea62e2316b..3098a324158270 100644 --- a/src/android/CHIPTool/app/src/main/res/layout/select_action_fragment.xml +++ b/src/android/CHIPTool/app/src/main/res/layout/select_action_fragment.xml @@ -1,103 +1,109 @@ - -