From 3849928de0a596a64cedd3ad4eb17cc3e88af645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Thu, 4 Nov 2021 15:00:43 +0100 Subject: [PATCH] [android] Use attribute subscriptions in sensors screen (#11318) * [android] Update device address on sensors screen start Automatically resolve the device address using DNS-SD upon entering the "Sensor Clusters" screen. * [android] Use attribute subscriptions in sensors screen The sensors screen currently actively polls the connected sensor device every 3 seconds. Use attribute subscriptions, instead. Add a new method for cancelling all active subscriptions for a given device. Previously, there was only a method for cancelling a specific subscription, but controller applications currently have no way of knowing the subscription ID. * Address code review comments --- .../clusterclient/SensorClientFragment.kt | 137 +++++++++++++----- .../app/src/main/res/values/strings.xml | 3 + src/app/InteractionModelEngine.cpp | 17 +++ src/app/InteractionModelEngine.h | 9 ++ src/app/ReadClient.cpp | 7 +- src/app/ReadClient.h | 4 +- src/controller/CHIPDevice.cpp | 5 + src/controller/CHIPDevice.h | 1 + .../java/CHIPDeviceController-JNI.cpp | 8 + .../ChipDeviceController.java | 7 + 10 files changed, 162 insertions(+), 36 deletions(-) diff --git a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/SensorClientFragment.kt b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/SensorClientFragment.kt index 3b7d173c146b09..cc3101cd7d05dc 100644 --- a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/SensorClientFragment.kt +++ b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/SensorClientFragment.kt @@ -5,6 +5,7 @@ import android.util.Log import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.view.inputmethod.EditorInfo import android.widget.AdapterView import android.widget.ArrayAdapter import android.widget.Toast @@ -28,18 +29,31 @@ private typealias ReadCallback = ChipClusters.IntegerAttributeCallback class SensorClientFragment : Fragment() { private val scope = CoroutineScope(Dispatchers.Main + Job()) - // Job for sending periodic sensor read requests - private var sensorWatchJob: Job? = null - // History of sensor values private val sensorData = LineGraphSeries() + // Device whose attribute is subscribed + private var subscribedDevicePtr = 0L + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View { return inflater.inflate(R.layout.sensor_client_fragment, container, false).apply { + ChipClient.getDeviceController(requireContext()).setCompletionListener(null) + deviceIdEd.setOnEditorActionListener { textView, actionId, _ -> + if (actionId == EditorInfo.IME_ACTION_DONE) { + updateAddress(textView.text.toString()) + resetSensorGraph() // reset the graph on device change + } + actionId == EditorInfo.IME_ACTION_DONE + } + endpointIdEd.setOnEditorActionListener { textView, actionId, _ -> + if (actionId == EditorInfo.IME_ACTION_DONE) + resetSensorGraph() // reset the graph on endpoint change + actionId == EditorInfo.IME_ACTION_DONE + } clusterNameSpinner.adapter = makeClusterNamesAdapter() clusterNameSpinner.onItemSelectedListener = object : AdapterView.OnItemSelectedListener { override fun onNothingSelected(parent: AdapterView<*>?) = Unit @@ -47,19 +61,21 @@ class SensorClientFragment : Fragment() { resetSensorGraph() // reset the graph on cluster change } } - readSensorBtn.setOnClickListener { scope.launch { readSensorButtonClick() } } + + readSensorBtn.setOnClickListener { scope.launch { readSensorCluster() } } watchSensorBtn.setOnCheckedChangeListener { _, isChecked -> if (isChecked) { - watchSensorButtonChecked() + scope.launch { subscribeSensorCluster() } } else { - watchSensorButtonUnchecked() + unsubscribeSensorCluster() } } + val currentTime = Calendar.getInstance().time.time sensorGraph.addSeries(sensorData) sensorGraph.viewport.isXAxisBoundsManual = true sensorGraph.viewport.setMinX(currentTime.toDouble()) - sensorGraph.viewport.setMaxX(currentTime.toDouble() + REFRESH_PERIOD_MS * MAX_DATA_POINTS) + sensorGraph.viewport.setMaxX(currentTime.toDouble() + MIN_REFRESH_PERIOD_S * 1000 * MAX_DATA_POINTS) sensorGraph.gridLabelRenderer.padding = 20 sensorGraph.gridLabelRenderer.numHorizontalLabels = 4 sensorGraph.gridLabelRenderer.setHorizontalLabelsAngle(150) @@ -77,12 +93,24 @@ class SensorClientFragment : Fragment() { override fun onStart() { super.onStart() deviceIdEd.setText(DeviceIdUtil.getLastDeviceId(requireContext()).toString()) + updateAddress(deviceIdEd.text.toString()) } override fun onStop() { - super.onStop() scope.cancel() resetSensorGraph() // reset the graph on fragment exit + super.onStop() + } + + private fun updateAddress(deviceId: String) { + try { + ChipClient.getDeviceController(requireContext()).updateDevice( + /* fabric ID */ 5544332211, + deviceId.toULong().toLong() + ) + } catch (ex: Exception) { + showMessage(R.string.update_device_address_failure, ex.toString()) + } } private fun resetSensorGraph() { @@ -101,41 +129,53 @@ class SensorClientFragment : Fragment() { } } - private suspend fun readSensorButtonClick() { + private suspend fun readSensorCluster() { try { - readSensorCluster(clusterNameSpinner.selectedItem.toString(), false) + val deviceId = deviceIdEd.text.toString().toULong().toLong() + val endpointId = endpointIdEd.text.toString().toInt() + val clusterName = clusterNameSpinner.selectedItem.toString() + val clusterRead = CLUSTERS[clusterName]!!["read"] as (Long, Int, ReadCallback) -> Unit + val device = ChipClient.getConnectedDevicePointer(requireContext(), deviceId) + val callback = makeReadCallback(clusterName, false) + + clusterRead(device, endpointId, callback) } catch (ex: Exception) { showMessage(R.string.sensor_client_read_error_text, ex.toString()) } } - private fun watchSensorButtonChecked() { - sensorWatchJob = scope.launch { - while (isActive) { - try { - readSensorCluster(clusterNameSpinner.selectedItem.toString(), true) - } catch (ex: Exception) { - showMessage(R.string.sensor_client_read_error_text, ex.toString()) - } - delay(REFRESH_PERIOD_MS) - } + private suspend fun subscribeSensorCluster() { + try { + val deviceId = deviceIdEd.text.toString().toULong().toLong() + val endpointId = endpointIdEd.text.toString().toInt() + val clusterName = clusterNameSpinner.selectedItem.toString() + val clusterSubscribe = CLUSTERS[clusterName]!!["subscribe"] as (Long, Int, ReadCallback) -> Unit + val device = ChipClient.getConnectedDevicePointer(requireContext(), deviceId) + val callback = makeReadCallback(clusterName, true) + + clusterSubscribe(device, endpointId, callback) + subscribedDevicePtr = device + } catch (ex: Exception) { + showMessage(R.string.sensor_client_subscribe_error_text, ex.toString()) } } - private fun watchSensorButtonUnchecked() { - sensorWatchJob?.cancel() - sensorWatchJob = null - } + private fun unsubscribeSensorCluster() { + if (subscribedDevicePtr == 0L) + return - private suspend fun readSensorCluster(clusterName: String, addToGraph: Boolean) { - val deviceId = deviceIdEd.text.toString().toULong().toLong() - val endpointId = endpointIdEd.text.toString().toInt() - val clusterConfig = CLUSTERS[clusterName] - val clusterRead = clusterConfig!!["read"] as (Long, Int, ReadCallback) -> Unit + try { + ChipClient.getDeviceController(requireContext()).shutdownSubscriptions(subscribedDevicePtr) + subscribedDevicePtr = 0 + } catch (ex: Exception) { + showMessage(R.string.sensor_client_unsubscribe_error_text, ex.toString()) + } + } - val device = ChipClient.getConnectedDevicePointer(requireContext(), deviceId) + private fun makeReadCallback(clusterName: String, addToGraph: Boolean): ReadCallback { + return object : ReadCallback { + val clusterConfig = CLUSTERS[clusterName]!! - clusterRead(device, endpointId, object : ReadCallback { override fun onSuccess(value: Int) { val unitValue = clusterConfig["unitValue"] as Double val unitSymbol = clusterConfig["unitSymbol"] as String @@ -145,7 +185,7 @@ class SensorClientFragment : Fragment() { override fun onError(ex: Exception) { showMessage(R.string.sensor_client_read_error_text, ex.toString()) } - }) + } } private fun consumeSensorValue(value: Double, unitSymbol: String, addToGraph: Boolean) { @@ -180,7 +220,8 @@ class SensorClientFragment : Fragment() { companion object { private const val TAG = "SensorClientFragment" - private const val REFRESH_PERIOD_MS = 3000L + private const val MIN_REFRESH_PERIOD_S = 2 + private const val MAX_REFRESH_PERIOD_S = 10 private const val MAX_DATA_POINTS = 60 private val CLUSTERS = mapOf( "Temperature" to mapOf( @@ -188,6 +229,16 @@ class SensorClientFragment : Fragment() { val cluster = ChipClusters.TemperatureMeasurementCluster(device, endpointId) cluster.readMeasuredValueAttribute(callback) }, + "subscribe" to { device: Long, endpointId: Int, callback: ReadCallback -> + val cluster = ChipClusters.TemperatureMeasurementCluster(device, endpointId) + cluster.reportMeasuredValueAttribute(callback) + cluster.subscribeMeasuredValueAttribute(object : ChipClusters.DefaultClusterCallback { + override fun onSuccess() = Unit + override fun onError(ex: Exception) { + callback.onError(ex) + } + }, MIN_REFRESH_PERIOD_S, MAX_REFRESH_PERIOD_S) + }, "unitValue" to 0.01, "unitSymbol" to "\u00B0C" ), @@ -196,6 +247,16 @@ class SensorClientFragment : Fragment() { val cluster = ChipClusters.PressureMeasurementCluster(device, endpointId) cluster.readMeasuredValueAttribute(callback) }, + "subscribe" to { device: Long, endpointId: Int, callback: ReadCallback -> + val cluster = ChipClusters.PressureMeasurementCluster(device, endpointId) + cluster.reportMeasuredValueAttribute(callback) + cluster.subscribeMeasuredValueAttribute(object : ChipClusters.DefaultClusterCallback { + override fun onSuccess() = Unit + override fun onError(ex: Exception) { + callback.onError(ex) + } + }, MIN_REFRESH_PERIOD_S, MAX_REFRESH_PERIOD_S) + }, "unitValue" to 1.0, "unitSymbol" to "hPa" ), @@ -204,6 +265,16 @@ class SensorClientFragment : Fragment() { val cluster = ChipClusters.RelativeHumidityMeasurementCluster(device, endpointId) cluster.readMeasuredValueAttribute(callback) }, + "subscribe" to { device: Long, endpointId: Int, callback: ReadCallback -> + val cluster = ChipClusters.RelativeHumidityMeasurementCluster(device, endpointId) + cluster.reportMeasuredValueAttribute(callback) + cluster.subscribeMeasuredValueAttribute(object : ChipClusters.DefaultClusterCallback { + override fun onSuccess() = Unit + override fun onError(ex: Exception) { + callback.onError(ex) + } + }, MIN_REFRESH_PERIOD_S, MAX_REFRESH_PERIOD_S) + }, "unitValue" to 0.01, "unitSymbol" to "%" ) diff --git a/src/android/CHIPTool/app/src/main/res/values/strings.xml b/src/android/CHIPTool/app/src/main/res/values/strings.xml index e6dd34946403e9..f37d4d4ef75189 100644 --- a/src/android/CHIPTool/app/src/main/res/values/strings.xml +++ b/src/android/CHIPTool/app/src/main/res/values/strings.xml @@ -29,6 +29,7 @@ Enter Device ID Enter Endpoint ID Update address + Device address update failed: %1$s Commission with IP address 3840 @@ -86,6 +87,8 @@ Watch Last value: %1$.2f %2$s Failed to read the sensor: %1$s + Failed to subscribe to the sensor: %1$s + Failed to unsubscribe from the sensor: %1$s Cluster Interaction Tool Multi-admin cluster diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 50bc098cff135f..5408bed1926ba6 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -220,6 +220,23 @@ CHIP_ERROR InteractionModelEngine::ShutdownSubscription(uint64_t aSubscriptionId return err; } +CHIP_ERROR InteractionModelEngine::ShutdownSubscriptions(FabricIndex aFabricIndex, NodeId aPeerNodeId) +{ + CHIP_ERROR err = CHIP_ERROR_KEY_NOT_FOUND; + + for (ReadClient & readClient : mReadClients) + { + if (!readClient.IsFree() && readClient.IsSubscriptionType() && readClient.GetFabricIndex() == aFabricIndex && + readClient.GetPeerNodeId() == aPeerNodeId) + { + readClient.Shutdown(); + err = CHIP_NO_ERROR; + } + } + + return err; +} + CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClientHandle & apWriteClient, WriteClient::Callback * apCallback) { apWriteClient.SetWriteClient(nullptr); diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index fc8cedec5a41e0..b7bda95c88f722 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -118,6 +118,15 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman * @retval #CHIP_NO_ERROR On success. */ CHIP_ERROR ShutdownSubscription(uint64_t aSubscriptionId); + + /** + * Tears down active subscriptions for a given peer node ID. + * + * @retval #CHIP_ERROR_KEY_NOT_FOUND If no active subscription is found. + * @retval #CHIP_NO_ERROR On success. + */ + CHIP_ERROR ShutdownSubscriptions(FabricIndex aFabricIndex, NodeId aPeerNodeId); + /** * Retrieve a WriteClient that the SDK consumer can use to send a write. If the call succeeds, * see WriteClient documentation for lifetime handling. diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 4277d770db5d07..3dc4d91c1e1796 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -79,6 +79,7 @@ void ReadClient::ShutdownInternal(CHIP_ERROR aError) mpExchangeCtx = nullptr; mInitialReport = true; mPeerNodeId = kUndefinedNodeId; + mFabricIndex = kUndefinedFabricIndex; MoveToState(ClientState::Uninitialized); } @@ -173,7 +174,8 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams) Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); SuccessOrExit(err); - mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId(); + mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId(); + mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex(); MoveToState(ClientState::AwaitingInitialReport); @@ -655,7 +657,8 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); SuccessOrExit(err); - mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId(); + mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId(); + mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex(); MoveToState(ClientState::AwaitingInitialReport); exit: diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 86508c425c3ada..ea8cb824967198 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -175,6 +175,7 @@ class ReadClient : public Messaging::ExchangeDelegate return mInteractionType == InteractionType::Subscribe ? returnType(mSubscriptionId) : returnType::Missing(); } + FabricIndex GetFabricIndex() const { return mFabricIndex; } NodeId GetPeerNodeId() const { return mPeerNodeId; } bool IsReadType() { return mInteractionType == InteractionType::Read; } bool IsSubscriptionType() const { return mInteractionType == InteractionType::Subscribe; }; @@ -184,7 +185,7 @@ class ReadClient : public Messaging::ExchangeDelegate friend class TestReadInteraction; friend class InteractionModelEngine; - enum class ClientState + enum class ClientState : uint8_t { Uninitialized = 0, ///< The client has not been initialized Initialized, ///< The client has been initialized and is ready for a SendReadRequest @@ -268,6 +269,7 @@ class ReadClient : public Messaging::ExchangeDelegate uint16_t mMaxIntervalCeilingSeconds = 0; uint64_t mSubscriptionId = 0; NodeId mPeerNodeId = kUndefinedNodeId; + FabricIndex mFabricIndex = kUndefinedFabricIndex; InteractionType mInteractionType = InteractionType::Read; }; diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index 910d40f971e6e4..04e5e6e9d6fefe 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -820,6 +820,11 @@ CHIP_ERROR Device::SendSubscribeAttributeRequest(app::AttributePathParams aPath, return CHIP_NO_ERROR; } +CHIP_ERROR Device::ShutdownSubscriptions() +{ + return app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(mFabricIndex, mDeviceId); +} + CHIP_ERROR Device::SendWriteAttributeRequest(app::WriteClientHandle aHandle, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback) { diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index 408d199d09b3b7..ba11b8d5dd8d26 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -139,6 +139,7 @@ class Device : public Messaging::ExchangeDelegate, public SessionEstablishmentDe CHIP_ERROR SendSubscribeAttributeRequest(app::AttributePathParams aPath, uint16_t mMinIntervalFloorSeconds, uint16_t mMaxIntervalCeilingSeconds, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback); + CHIP_ERROR ShutdownSubscriptions(); CHIP_ERROR SendWriteAttributeRequest(app::WriteClientHandle aHandle, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback); diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 2909979d339e9e..1b9c073df72636 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -304,6 +304,14 @@ JNI_METHOD(jboolean, isActive)(JNIEnv * env, jobject self, jlong handle) return chipDevice->IsActive(); } +JNI_METHOD(void, shutdownSubscriptions)(JNIEnv * env, jobject self, jlong handle, jlong devicePtr) +{ + chip::DeviceLayer::StackLock lock; + + Device * device = reinterpret_cast(devicePtr); + device->ShutdownSubscriptions(); +} + JNI_METHOD(jstring, getIpAddress)(JNIEnv * env, jobject self, jlong handle, jlong deviceId) { chip::DeviceLayer::StackLock lock; diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java index 4956908084cdcd..5ed8c507d4b094 100644 --- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java +++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java @@ -210,6 +210,11 @@ public boolean isActive(long deviceId) { return isActive(deviceControllerPtr, deviceId); } + /* Shutdown all cluster attribute subscriptions for a given device */ + public void shutdownSubscriptions(long devicePtr) { + shutdownSubscriptions(deviceControllerPtr, devicePtr); + } + /** * Generates a new PASE verifier and passcode ID for the given setup PIN code. * @@ -271,6 +276,8 @@ private native boolean openPairingWindowWithPIN( private native boolean isActive(long deviceControllerPtr, long deviceId); + private native void shutdownSubscriptions(long deviceControllerPtr, long devicePtr); + static { System.loadLibrary("CHIPController"); }