From f216481cdba18808547dacbdb410a90fc647c798 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Mon, 12 Feb 2024 22:08:47 -0800 Subject: [PATCH] [Android] Add Status handling for onReport code path (#32082) --- .../pairing/PairOnNetworkLongImReadCommand.kt | 47 +++++++-- src/controller/java/AndroidCallbacks.cpp | 95 +++++++++++++------ src/controller/java/BUILD.gn | 1 + .../devicecontroller/model/ClusterState.java | 38 +++++++- .../devicecontroller/model/NodeState.java | 63 +++++++++++- .../chip/devicecontroller/model/Status.java | 57 +++++++++++ 6 files changed, 260 insertions(+), 41 deletions(-) create mode 100644 src/controller/java/src/chip/devicecontroller/model/Status.java diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt index 3e45d62323cfec..e0a95d584130eb 100644 --- a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt +++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt @@ -10,6 +10,7 @@ import chip.devicecontroller.model.ChipPathId import chip.devicecontroller.model.DataVersionFilter import chip.devicecontroller.model.EventState import chip.devicecontroller.model.NodeState +import chip.devicecontroller.model.Status import com.matter.controller.commands.common.CredentialsIssuer import java.util.logging.Level import java.util.logging.Logger @@ -34,13 +35,6 @@ class PairOnNetworkLongImReadCommand( eventPath: ChipEventPath?, e: Exception ) { - if (attributePath != null && attributePath.clusterId.getId() == UNIT_TEST_CLUSTER) { - logger.log( - Level.INFO, - "TODO: skip the error check for unit test cluster that covers most error result" - ) - return - } logger.log(Level.INFO, "Read receive onError") setFailure("read failure") } @@ -72,11 +66,20 @@ class PairOnNetworkLongImReadCommand( return cluster.equals(expected) } + fun checkUnitTestClusterGeneralStatus(status: Status): Boolean = + (status.getStatus() == CLUSTER_ID_TEST_GENERAL_ERROR_STATUS) && + !status.getClusterStatus().isPresent() + + fun checkUnitTestClusterClusterStatus(status: Status): Boolean = + (status.getStatus() == CLUSTER_ID_TEST_CLUSTER_ERROR_STATUS) && + status.getClusterStatus().isPresent() && + status.getClusterStatus().get() == CLUSTER_ID_TEST_CLUSTER_ERROR_CLUSTER_STATUS + private fun validateResponse(nodeState: NodeState) { val endpointZero = requireNotNull(nodeState.getEndpointState(0)) { "Endpoint zero not found." } - val endpointOne = requireNotNull(nodeState.getEndpointState(0)) { "Endpoint one not found." } + val endpointOne = requireNotNull(nodeState.getEndpointState(1)) { "Endpoint one not found." } val basicCluster = requireNotNull(endpointZero.getClusterState(CLUSTER_ID_BASIC)) { @@ -93,6 +96,11 @@ class PairOnNetworkLongImReadCommand( "No local config disabled attribute found." } + val unitTestCluster = + requireNotNull(endpointOne.getClusterState(UNIT_TEST_CLUSTER)) { + "Unit test cluster not found." + } + val startUpEvents = requireNotNull(basicCluster.getEventState(EVENT_ID_START_UP)) { "No start up event found." } @@ -122,6 +130,22 @@ class PairOnNetworkLongImReadCommand( require(checkAllAttributesJsonForFixedLabel(clusterAttributes)) { "Invalid fixed label cluster attributes Json ${clusterAttributes}" } + + require( + checkUnitTestClusterGeneralStatus( + unitTestCluster.getAttributeStatuses()[CLUSTER_ID_TEST_GENERAL_ERROR_BOOLEAN]!! + ) + ) { + "Invalid unit test cluster generalStatus check ${unitTestCluster}" + } + + require( + checkUnitTestClusterClusterStatus( + unitTestCluster.getAttributeStatuses()[CLUSTER_ID_TEST_CLUSTER_ERROR_BOOLEAN]!! + ) + ) { + "Invalid unit test cluster clusterStatus check ${unitTestCluster}" + } } override fun onReport(nodeState: NodeState) { @@ -212,10 +236,15 @@ class PairOnNetworkLongImReadCommand( private const val MATTER_PORT = 5540 private const val CLUSTER_ID_BASIC = 0x0028L private const val FIXED_LABEL_CLUSTER = 0x0040L - private const val UNIT_TEST_CLUSTER = 0xfff1fc05 + private const val UNIT_TEST_CLUSTER = 0xfff1fc05L private const val ATTR_ID_LOCAL_CONFIG_DISABLED = 16L private const val EVENT_ID_START_UP = 0L private const val GLOBAL_ATTRIBUTE_LIST = 65531L private const val CLUSTER_ID_BASIC_VERSION = 0L + private const val CLUSTER_ID_TEST_GENERAL_ERROR_BOOLEAN = 0x0031L + private const val CLUSTER_ID_TEST_CLUSTER_ERROR_BOOLEAN = 0x0032L + private const val CLUSTER_ID_TEST_GENERAL_ERROR_STATUS = 0x8d + private const val CLUSTER_ID_TEST_CLUSTER_ERROR_STATUS = 1 + private const val CLUSTER_ID_TEST_CLUSTER_ERROR_CLUSTER_STATUS = 17 } } diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp index 88bbf9844c9407..9e3e3e966c20d3 100644 --- a/src/controller/java/AndroidCallbacks.cpp +++ b/src/controller/java/AndroidCallbacks.cpp @@ -55,22 +55,6 @@ CHIP_ERROR CreateChipAttributePath(JNIEnv * env, const app::ConcreteDataAttribut return CHIP_NO_ERROR; } -CHIP_ERROR ReportCallback::CreateChipEventPath(JNIEnv * env, const app::ConcreteEventPath & aPath, jobject & outObj) -{ - jclass eventPathCls = nullptr; - ReturnErrorOnFailure( - JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/model/ChipEventPath", eventPathCls)); - - jmethodID eventPathCtor = - env->GetStaticMethodID(eventPathCls, "newInstance", "(IJJ)Lchip/devicecontroller/model/ChipEventPath;"); - VerifyOrReturnError(eventPathCtor != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND); - - outObj = env->CallStaticObjectMethod(eventPathCls, eventPathCtor, static_cast(aPath.mEndpointId), - static_cast(aPath.mClusterId), static_cast(aPath.mEventId)); - VerifyOrReturnError(outObj != nullptr, CHIP_JNI_ERROR_NULL_OBJECT); - return CHIP_NO_ERROR; -} - GetConnectedDeviceCallback::GetConnectedDeviceCallback(jobject wrapperCallback, jobject javaCallback) : mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnDeviceConnectionFailureFn, this) { @@ -270,6 +254,30 @@ CHIP_ERROR ConvertReportTlvToJson(const uint32_t id, TLV::TLVReader & data, std: return TlvToJson(readerForJson, json); } +static CHIP_ERROR CreateStatus(JNIEnv * env, const app::StatusIB & aStatus, jobject & outObj) +{ + jclass statusCls = nullptr; + ReturnErrorOnFailure(JniReferences::GetInstance().GetLocalClassRef(env, "chip/devicecontroller/model/Status", statusCls)); + jmethodID statusCtor = nullptr; + if (aStatus.mClusterStatus.HasValue()) + { + statusCtor = env->GetStaticMethodID(statusCls, "newInstance", "(II)Lchip/devicecontroller/model/Status;"); + VerifyOrReturnError(!env->ExceptionCheck(), CHIP_JNI_ERROR_EXCEPTION_THROWN); + VerifyOrReturnError(statusCtor != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND); + outObj = env->CallStaticObjectMethod(statusCls, statusCtor, static_cast(aStatus.mStatus), + static_cast(aStatus.mClusterStatus.Value())); + } + else + { + statusCtor = env->GetStaticMethodID(statusCls, "newInstance", "(I)Lchip/devicecontroller/model/Status;"); + VerifyOrReturnError(!env->ExceptionCheck(), CHIP_JNI_ERROR_EXCEPTION_THROWN); + VerifyOrReturnError(statusCtor != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND); + outObj = env->CallStaticObjectMethod(statusCls, statusCtor, static_cast(aStatus.mStatus)); + } + VerifyOrReturnError(outObj != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND); + return CHIP_NO_ERROR; +} + void ReportCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const app::StatusIB & aStatus) { @@ -277,14 +285,28 @@ void ReportCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPat JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); JniLocalReferenceScope scope(env); - - jobject attributePathObj = nullptr; - err = CreateChipAttributePath(env, aPath, attributePathObj); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Unable to create Java ChipAttributePath: %s", ErrorStr(err))); - VerifyOrReturn(!aPath.IsListItemOperation(), ChipLogError(Controller, "Expect non-list item operation"); aPath.LogPath()); - VerifyOrReturn(aStatus.IsSuccess(), ChipLogError(Controller, "Receive bad status %s", ErrorStr(aStatus.ToChipError())); - aPath.LogPath()); + + jobject nodeState = mNodeStateObj.ObjectRef(); + if (aStatus.IsFailure()) + { + ChipLogError(Controller, "Receive bad status %s", ErrorStr(aStatus.ToChipError())); + jobject statusObj = nullptr; + err = CreateStatus(env, aStatus, statusObj); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(Controller, "Fail to create status with error %" CHIP_ERROR_FORMAT, err.Format())); + // Add Attribute Status to NodeState + jmethodID addAttributeStatusMethod = nullptr; + err = JniReferences::GetInstance().FindMethod(env, nodeState, "addAttributeStatus", + "(IJJLchip/devicecontroller/model/Status;)V", &addAttributeStatusMethod); + VerifyOrReturn( + err == CHIP_NO_ERROR, + ChipLogError(Controller, "Could not find addAttributeStatus method with error %" CHIP_ERROR_FORMAT, err.Format())); + env->CallVoidMethod(nodeState, addAttributeStatusMethod, static_cast(aPath.mEndpointId), + static_cast(aPath.mClusterId), static_cast(aPath.mAttributeId), statusObj); + VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); + return; + } VerifyOrReturn(apData != nullptr, ChipLogError(Controller, "Receive empty apData"); aPath.LogPath()); TLV::TLVReader readerForJavaTLV; @@ -345,7 +367,6 @@ void ReportCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPat VerifyOrReturn(attributeStateObj != nullptr, ChipLogError(Controller, "Could not create AttributeState object"); aPath.LogPath()); - jobject nodeState = mNodeStateObj.ObjectRef(); // Add AttributeState to NodeState jmethodID addAttributeMethod; err = JniReferences::GetInstance().FindMethod(env, nodeState, "addAttribute", @@ -401,10 +422,28 @@ void ReportCallback::OnEventData(const app::EventHeader & aEventHeader, TLV::TLV CHIP_ERROR err = CHIP_NO_ERROR; JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread")); - jobject eventPathObj = nullptr; - err = CreateChipEventPath(env, aEventHeader.mPath, eventPathObj); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Unable to create Java ChipEventPath: %s", ErrorStr(err))); + jobject nodeState = mNodeStateObj.ObjectRef(); + if (apStatus != nullptr && apStatus->IsFailure()) + { + ChipLogError(Controller, "Receive bad status %s", ErrorStr(apStatus->ToChipError())); + jobject statusObj = nullptr; + err = CreateStatus(env, *apStatus, statusObj); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(Controller, "Fail to create status with error %" CHIP_ERROR_FORMAT, err.Format())); + // Add Event Status to NodeState + jmethodID addEventStatusMethod; + err = JniReferences::GetInstance().FindMethod(env, nodeState, "addEventStatus", + "(IJJLchip/devicecontroller/model/Status;)V", &addEventStatusMethod); + VerifyOrReturn( + err == CHIP_NO_ERROR, + ChipLogError(Controller, "Could not find addEventStatus method with error %" CHIP_ERROR_FORMAT, err.Format())); + env->CallVoidMethod(nodeState, addEventStatusMethod, static_cast(aEventHeader.mPath.mEndpointId), + static_cast(aEventHeader.mPath.mClusterId), static_cast(aEventHeader.mPath.mEventId), + statusObj); + VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe()); + return; + } VerifyOrReturn(apData != nullptr, ChipLogError(Controller, "Receive empty apData"); aEventHeader.LogPath()); TLV::TLVReader readerForJavaTLV; @@ -484,7 +523,7 @@ void ReportCallback::OnEventData(const app::EventHeader & aEventHeader, TLV::TLV // Add EventState to NodeState jmethodID addEventMethod; - jobject nodeState = mNodeStateObj.ObjectRef(); + err = JniReferences::GetInstance().FindMethod(env, nodeState, "addEvent", "(IJJLchip/devicecontroller/model/EventState;)V", &addEventMethod); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Could not find addEvent method with error %s", ErrorStr(err)); diff --git a/src/controller/java/BUILD.gn b/src/controller/java/BUILD.gn index 0f68fcebb6ed6e..0e5beda7115ae6 100644 --- a/src/controller/java/BUILD.gn +++ b/src/controller/java/BUILD.gn @@ -482,6 +482,7 @@ android_library("java") { "src/chip/devicecontroller/model/EventState.java", "src/chip/devicecontroller/model/InvokeElement.java", "src/chip/devicecontroller/model/NodeState.java", + "src/chip/devicecontroller/model/Status.java", ] if (matter_enable_tlv_decoder_api) { diff --git a/src/controller/java/src/chip/devicecontroller/model/ClusterState.java b/src/controller/java/src/chip/devicecontroller/model/ClusterState.java index 4c575f1a41df66..6ebe40e99fe0a2 100644 --- a/src/controller/java/src/chip/devicecontroller/model/ClusterState.java +++ b/src/controller/java/src/chip/devicecontroller/model/ClusterState.java @@ -32,12 +32,19 @@ public final class ClusterState { private static final String TAG = "ClusterState"; private Map attributes; private Map> events; + private Map attributeStatuses; + private Map> eventStatuses; private Optional dataVersion; - public ClusterState( - Map attributes, Map> events) { + protected ClusterState( + Map attributes, + Map> events, + Map attributeStatuses, + Map> eventStatuses) { this.attributes = attributes; this.events = events; + this.attributeStatuses = attributeStatuses; + this.eventStatuses = eventStatuses; this.dataVersion = Optional.empty(); } @@ -45,10 +52,18 @@ public Map getAttributeStates() { return attributes; } + public Map getAttributeStatuses() { + return attributeStatuses; + } + public Map> getEventStates() { return events; } + public Map> getEventStatuses() { + return eventStatuses; + } + public void setDataVersion(long version) { dataVersion = Optional.of(version); } @@ -130,6 +145,25 @@ public String toString() { builder.append("\n"); }); }); + attributeStatuses.forEach( + (attributeId, status) -> { + builder.append("Attribute Status "); + builder.append(attributeId); + builder.append(": "); + builder.append(status.toString()); + builder.append("\n"); + }); + eventStatuses.forEach( + (eventId, status) -> { + status.forEach( + (eventState) -> { + builder.append("Event Status"); + builder.append(eventId); + builder.append(": "); + builder.append(status.toString()); + builder.append("\n"); + }); + }); return builder.toString(); } } diff --git a/src/controller/java/src/chip/devicecontroller/model/NodeState.java b/src/controller/java/src/chip/devicecontroller/model/NodeState.java index decd9782f5369a..f77097b5452f62 100644 --- a/src/controller/java/src/chip/devicecontroller/model/NodeState.java +++ b/src/controller/java/src/chip/devicecontroller/model/NodeState.java @@ -55,10 +55,15 @@ private void addAttribute( ClusterState clusterState = endpointState.getClusterState(clusterId); if (clusterState == null) { - clusterState = new ClusterState(new HashMap<>(), new HashMap<>()); + clusterState = + new ClusterState(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>()); endpointState.getClusterStates().put(clusterId, clusterState); } + if (clusterState.getAttributeStatuses().containsKey(attributeId)) { + clusterState.getAttributeStatuses().remove(attributeId); + } + // This will overwrite previous attributes. clusterState.getAttributeStates().put(attributeId, attributeStateToAdd); } @@ -72,16 +77,70 @@ private void addEvent(int endpointId, long clusterId, long eventId, EventState e ClusterState clusterState = endpointState.getClusterState(clusterId); if (clusterState == null) { - clusterState = new ClusterState(new HashMap<>(), new HashMap<>()); + clusterState = + new ClusterState(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>()); endpointState.getClusterStates().put(clusterId, clusterState); } if (!clusterState.getEventStates().containsKey(eventId)) { clusterState.getEventStates().put(eventId, new ArrayList()); } + + if (clusterState.getEventStatuses().containsKey(eventId)) { + clusterState.getEventStatuses().remove(eventId); + } + clusterState.getEventStates().get(eventId).add(eventStateToAdd); } + // Called from native code only, which ignores access modifiers. + private void addAttributeStatus( + int endpointId, long clusterId, long attributeId, Status statusToAdd) { + EndpointState endpointState = getEndpointState(endpointId); + if (endpointState == null) { + endpointState = new EndpointState(new HashMap<>()); + getEndpointStates().put(endpointId, endpointState); + } + + ClusterState clusterState = endpointState.getClusterState(clusterId); + if (clusterState == null) { + clusterState = + new ClusterState(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>()); + endpointState.getClusterStates().put(clusterId, clusterState); + } + + if (clusterState.getAttributeStates().containsKey(attributeId)) { + clusterState.getAttributeStates().remove(attributeId); + } + + clusterState.getAttributeStatuses().put(attributeId, statusToAdd); + } + + private void addEventStatus(int endpointId, long clusterId, long eventId, Status statusToAdd) { + EndpointState endpointState = getEndpointState(endpointId); + if (endpointState == null) { + endpointState = new EndpointState(new HashMap<>()); + getEndpointStates().put(endpointId, endpointState); + } + + ClusterState clusterState = endpointState.getClusterState(clusterId); + if (clusterState == null) { + clusterState = + new ClusterState(new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>()); + endpointState.getClusterStates().put(clusterId, clusterState); + } + + if (!clusterState.getEventStatuses().containsKey(eventId)) { + clusterState.getEventStatuses().put(eventId, new ArrayList()); + } + + if (clusterState.getEventStates().containsKey(eventId)) { + clusterState.getEventStates().remove(eventId); + } + + clusterState.getEventStatuses().get(eventId).add(statusToAdd); + } + @Override public String toString() { StringBuilder builder = new StringBuilder(); diff --git a/src/controller/java/src/chip/devicecontroller/model/Status.java b/src/controller/java/src/chip/devicecontroller/model/Status.java new file mode 100644 index 00000000000000..7391f9a312f3f7 --- /dev/null +++ b/src/controller/java/src/chip/devicecontroller/model/Status.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package chip.devicecontroller.model; + +import java.util.Locale; +import java.util.Optional; + +public final class Status { + private Integer status; + private Optional clusterStatus; + + private Status(int status, Optional clusterStatus) { + this.status = status; + this.clusterStatus = clusterStatus; + } + + // Getters + public Integer getStatus() { + return status; + } + + public Optional getClusterStatus() { + return clusterStatus; + } + + public String toString() { + return String.format( + Locale.ENGLISH, + "status %s, clusterStatus %s", + String.valueOf(status), + clusterStatus.isPresent() ? String.valueOf(clusterStatus.get()) : "None"); + } + + public static Status newInstance(int status, int clusterStatus) { + return new Status(status, Optional.of(clusterStatus)); + } + + public static Status newInstance(int status) { + return new Status(status, Optional.empty()); + } +}