From 495688209274384fd356b08e32b6b0a2ac25a063 Mon Sep 17 00:00:00 2001 From: Marcos B <15697303+gmarcosb@users.noreply.github.com> Date: Wed, 10 May 2023 11:39:10 -0600 Subject: [PATCH] Chip tool fixes (#24420) * * Change various references of "SessionHandle &" to instead use "const SessionHandle &" to match how SessionHandle is often used as a parameter & to support rvalue * Add Java/JNI hooks into for-test classes (like MessagingContext) to allow for unit tests of Java & JNI functionality * Add first JNI unit test, GetConnectedDeviceCallbackJniTest, testing success/failure cases for device callback * Restyled by whitespace * Restyled by clang-format * Restyled by gn * * Fix a user-reported bug where we were seeing the following callstack: JNIEnv::NewObject(_jclass*, _jmethodID*, ...) chip::Controller::GetConnectedDeviceCallback::OnDeviceConnectionFailureFn(void*, chip::ScopedNodeId const&, chip::ChipError) chip::OperationalSessionSetup::DequeueConnectionCallbacks(chip::ChipError) chip::OperationalSessionSetup::OnNodeAddressResolutionFailed(chip::PeerId const&, chip::ChipError) chip::AddressResolve::Impl::Resolver::HandleAction(chip::IntrusiveList >::Iterator&) chip::AddressResolve::Impl::Resolver::HandleTimer() chip::System::LayerImplSelect::HandleEvents() Though JNI tests were added to try to catch this at presubmit, those tests did not fail, which leads me to believe there's a difference in the JRE (wherein the version with the user-reported bug), a cast to an int is not being done, whereas in android emulator, the cast to int is done * Add a README pointing to building android guide & specifying that these tests must be run externally due to android emulator dependency * Fix path to android_building.md * Restyled by clang-format * Restyled by prettier-markdown * Missed const correctness changes for SessionHandle& * Restyled by clang-format * More missed const correctness changes for SessionHandle& * Restyled by clang-format * More missed const correctness changes for SessionHandle& * Restyled by clang-format * Only build JNI test libs if chip_link_tests * Restyled by gn * Add missing tests.gni import * Fixes to Java CHIPTool * Match class name to file name --------- Co-authored-by: Restyled.io Co-authored-by: Andrei Litvin --- examples/android/CHIPTool/app/src/main/AndroidManifest.xml | 2 ++ .../app/src/main/java/com/google/chip/chiptool/ChipClient.kt | 4 ++-- .../clusterinteraction/{endpointItem.kt => EndpointItem.kt} | 0 3 files changed, 4 insertions(+), 2 deletions(-) rename examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/{endpointItem.kt => EndpointItem.kt} (100%) diff --git a/examples/android/CHIPTool/app/src/main/AndroidManifest.xml b/examples/android/CHIPTool/app/src/main/AndroidManifest.xml index 0d7401fea46066..0526dc5a78160d 100644 --- a/examples/android/CHIPTool/app/src/main/AndroidManifest.xml +++ b/examples/android/CHIPTool/app/src/main/AndroidManifest.xml @@ -3,6 +3,8 @@ xmlns:tools="http://schemas.android.com/tools" package="com.google.chip.chiptool"> + + diff --git a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt index 3d130f835de459..add408b7af9693 100644 --- a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt +++ b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt @@ -33,7 +33,7 @@ import chip.platform.PreferencesKeyValueStoreManager import com.google.chip.chiptool.attestation.ExampleAttestationTrustStoreDelegate import kotlin.coroutines.resume import kotlin.coroutines.resumeWithException -import kotlin.coroutines.suspendCoroutine +import kotlinx.coroutines.suspendCancellableCoroutine /** Lazily instantiates [ChipDeviceController] and holds a reference to it. */ object ChipClient { @@ -75,7 +75,7 @@ object ChipClient { // once we are done with the returned device pointer. Memory leak was introduced since the refactor // that introduced it was very large in order to fix a use after free, which was considered to be // worse than the memory leak that was introduced. - return suspendCoroutine { continuation -> + return suspendCancellableCoroutine { continuation -> getDeviceController(context).getConnectedDevicePointer( nodeId, object : GetConnectedDeviceCallback { diff --git a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/endpointItem.kt b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/EndpointItem.kt similarity index 100% rename from examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/endpointItem.kt rename to examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/EndpointItem.kt