Skip to content

Commit

Permalink
[android] Fix handling optional values + UI fix. (#19237)
Browse files Browse the repository at this point in the history 
* [nrf noup] [android] Use ScrollView in main screen

Some buttons were not visible on low-resolution devices.

Signed-off-by: Damian Krolik <[email protected]>

* [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 <[email protected]>

* Add IDL tests
  • Loading branch information
@Damian-Nordic @pull
Damian-Nordic authored and pull[bot] committed Feb 16, 2024
1 parent 47e7a12 commit fbf2d25
Show file tree
Hide file tree
Showing 15 changed files with 719 additions and 592 deletions.
3 changes: 3 additions & 0 deletions scripts/idl/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 11 additions & 9 deletions scripts/idl/generators/java/ChipClustersCpp.jinja
View file
Original file line number Diff line number Diff line change
@@ -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 %}

Expand Down
4 changes: 4 additions & 0 deletions scripts/idl/tests/available_tests.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions scripts/idl/tests/inputs/optional_argument.matter
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
client cluster MyCluster = 1 {
request struct FooRequest {
optional OCTET_STRING argument = 0;
}

command Foo(FooRequest): DefaultSuccess = 0;
}
79 changes: 79 additions & 0 deletions scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-InvokeSubscribeImpl.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#include <controller/java/zap-generated/CHIPCallbackTypes.h>
#include <controller/java/zap-generated/CHIPInvokeCallbacks.h>
#include <controller/java/zap-generated/CHIPReadCallbacks.h>

#include <app-common/zap-generated/cluster-objects.h>
#include <zap-generated/CHIPClientCallbacks.h>
#include <zap-generated/CHIPClusters.h>

#include <controller/java/AndroidCallbacks.h>
#include <controller/java/AndroidClusterExceptions.h>
#include <controller/java/CHIPDefaultCallbacks.h>
#include <jni.h>
#include <lib/support/CHIPListUtils.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/JniReferences.h>
#include <lib/support/JniTypeWrappers.h>
#include <lib/support/Span.h>
#include <platform/PlatformManager.h>
#include <vector>

#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<DeviceProxy *>(devicePtr), endpointId);
return reinterpret_cast<jlong>(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<Platform::UniquePtr<JniByteArray>> cleanupByteArrays;
std::vector<Platform::UniquePtr<JniUtfString>> 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<chip::JniByteArray>(env, static_cast<jbyteArray>(optionalValue_0)));
definedValue_0 = cleanupByteArrays.back()->byteSpan();
}
}


std::unique_ptr<CHIPDefaultSuccessCallback, void (*)(CHIPDefaultSuccessCallback *)> onSuccess(
Platform::New<CHIPDefaultSuccessCallback>(callback), Platform::Delete<CHIPDefaultSuccessCallback>);
std::unique_ptr<CHIPDefaultFailureCallback, void (*)(CHIPDefaultFailureCallback *)> onFailure(Platform::New<CHIPDefaultFailureCallback>(callback), Platform::Delete<CHIPDefaultFailureCallback>);
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<MyClusterCluster *>(clusterPtr);
VerifyOrReturn(cppCluster != nullptr, AndroidClusterExceptions::GetInstance().ReturnIllegalStateException(env, callback, "Error getting native cluster", CHIP_ERROR_INCORRECT_STATE));

auto successFn = chip::Callback::Callback<CHIPDefaultSuccessCallbackType>::FromCancelable(onSuccess->Cancel());
auto failureFn = chip::Callback::Callback<CHIPDefaultFailureCallbackType>::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();
}
14 changes: 14 additions & 0 deletions scripts/idl/tests/outputs/optional_argument/jni/MyClusterClient-ReadImpl.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

#include <controller/java/zap-generated/CHIPReadCallbacks.h>

#include <app-common/zap-generated/cluster-objects.h>
#include <zap-generated/CHIPClusters.h>

#include <controller/java/AndroidClusterExceptions.h>
#include <controller/java/CHIPDefaultCallbacks.h>
#include <jni.h>
#include <lib/support/CodeUtils.h>
#include <platform/PlatformManager.h>

#define JNI_METHOD(RETURN, CLASS_NAME, METHOD_NAME) \
extern "C" JNIEXPORT RETURN JNICALL Java_chip_devicecontroller_ChipClusters_00024##CLASS_NAME##_##METHOD_NAME
13 changes: 8 additions & 5 deletions ...n/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterDetailFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
)
}
Expand Down Expand Up @@ -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
}
}
Expand Down
182 changes: 94 additions & 88 deletions src/android/CHIPTool/app/src/main/res/layout/select_action_fragment.xml
View file
Original file line number Diff line number Diff line change
@@ -1,103 +1,109 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:orientation="vertical" android:layout_width="match_parent"
<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent">

<Button
android:id="@+id/scanQrBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/scan_qr_code_btn_text" />
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:orientation="vertical" android:layout_width="match_parent"
android:layout_height="wrap_content">

<Button
android:id="@+id/provisionWiFiCredentialsBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_wifi_credentials_btn_text" />
<Button
android:id="@+id/scanQrBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/scan_qr_code_btn_text" />

<Button
android:id="@+id/provisionThreadCredentialsBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_thread_credentials_btn_text" />
<Button
android:id="@+id/provisionWiFiCredentialsBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_wifi_credentials_btn_text" />

<Button
android:id="@+id/provisionCustomFlowBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_custom_flow_btn_text" />
<Button
android:id="@+id/provisionThreadCredentialsBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_thread_credentials_btn_text" />

<Button
android:id="@+id/onOffClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/on_off_level_btn_text" />
<Button
android:id="@+id/provisionCustomFlowBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/provision_custom_flow_btn_text" />

<Button
android:id="@+id/sensorClustersBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/sensor_client_btn_text" />
<Button
android:id="@+id/onOffClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/on_off_level_btn_text" />

<Button
android:id="@+id/multiAdminClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/multi_admin_client_btn_text" />
<Button
android:id="@+id/sensorClustersBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/sensor_client_btn_text" />

<Button
android:id="@+id/opCredClustersBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/op_cred_client_btn_text" />
<Button
android:id="@+id/multiAdminClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/multi_admin_client_btn_text" />

<Button
android:id="@+id/basicClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/basic_cluster_btn_text" />
<Button
android:id="@+id/opCredClustersBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/op_cred_client_btn_text" />

<Button
android:id="@+id/attestationTestBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:visibility="gone"
android:text="@string/attestation_test_btn_text" />
<Button
android:id="@+id/basicClusterBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/basic_cluster_btn_text" />

<Button
android:id="@+id/wildcardBtn"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/wildcard_btn_text" />
<Button
android:id="@+id/attestationTestBtn"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:visibility="gone"
android:text="@string/attestation_test_btn_text" />

<Button
android:id="@+id/clusterInteractionBtn"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/cluster_interaction_tool" />
<Button
android:id="@+id/wildcardBtn"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/wildcard_btn_text" />

</LinearLayout>
<Button
android:id="@+id/clusterInteractionBtn"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="8dp"
android:layout_marginTop="8dp"
android:text="@string/cluster_interaction_tool" />

</LinearLayout>

</ScrollView>
Loading

0 comments on commit fbf2d25

Please sign in to comment.