Skip to content

Commit

Permalink
Improve Java/Jni IM read/subscribe (#25666)
Browse files Browse the repository at this point in the history 
* Improve Java/Jni IM read/subscribe

-- Add auto-resubscribe capability for subscribe
-- Refactor read/subscribe with better exception check.
-- manually validate the resubscription via bring up/down/up
all-cluster-app
-- with custom cluster, report would supress the error generated from
clusterObjects.
-- Fix across platform::delete usage in jni
-- add imTimeout parameter for read/subscribe

* disable cluster state data cache in java/jni

-- Disable cluster state data chace in java/jni and only use event
cache, version cache from cluster state cache, since java also have
cache to store the data

* Fix android chip-tool build failure

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@yunhanw-google @restyled-commits
yunhanw-google and restyled-commits authored Mar 14, 2023
1 parent 3eac074 commit 53510fe
Show file tree
Hide file tree
Showing 8 changed files with 261 additions and 95 deletions.
12 changes: 8 additions & 4 deletions ...oid/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/WildcardFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ class WildcardFragment : Fragment() {
minInterval,
maxInterval,
keepSubscriptions,
isFabricFiltered)
isFabricFiltered,
/* imTimeoutMs= */ 0)
} else if (type == EVENT) {
val eventPath = ChipEventPath.newInstance(endpointId, clusterId, eventId, isUrgent)
deviceController.subscribeToPath(subscriptionEstablishedCallback,
Expand All @@ -192,7 +193,8 @@ class WildcardFragment : Fragment() {
minInterval,
maxInterval,
keepSubscriptions,
isFabricFiltered)
isFabricFiltered,
/* imTimeoutMs= */ 0)
}
}

Expand All @@ -209,15 +211,17 @@ class WildcardFragment : Fragment() {
addressUpdateFragment.deviceId),
listOf(attributePath),
null,
isFabricFiltered)
isFabricFiltered,
/* imTimeoutMs= */ 0)
} else if (type == EVENT) {
val eventPath = ChipEventPath.newInstance(endpointId, clusterId, eventId)
deviceController.readPath(reportCallback,
ChipClient.getConnectedDevicePointer(requireContext(),
addressUpdateFragment.deviceId),
null,
listOf(eventPath),
isFabricFiltered)
isFabricFiltered,
/* imTimeoutMs= */ 0)
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...troller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class PairOnNetworkLongImReadCommand(
.getConnectedDevicePointer(getNodeId(), InternalGetConnectedDeviceCallback())
clear()
currentCommissioner()
.readPath(InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), false)
.readPath(InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), false, 0)
waitCompleteMs(getTimeoutMillis())
}

Expand Down
2 changes: 1 addition & 1 deletion ...er/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImSubscribeCommand.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class PairOnNetworkLongImSubscribeCommand(
.getConnectedDevicePointer(getNodeId(), InternalGetConnectedDeviceCallback())
clear()
currentCommissioner()
.subscribeToPath(InternalSubscriptionEstablishedCallback(), InternalResubscriptionAttemptCallback(), InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), 0, 5, false, false)
.subscribeToPath(InternalSubscriptionEstablishedCallback(), InternalResubscriptionAttemptCallback(), InternalReportCallback(), devicePointer, attributePathList, Collections.emptyList(), 0, 5, false, false, 0)
waitCompleteMs(getTimeoutMillis())
currentCommissioner().shutdownSubscriptions(currentCommissioner().getFabricIndex(), getNodeId(), subscriptionId);
}
Expand Down
8 changes: 4 additions & 4 deletions src/controller/java/AndroidCallbacks-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ JNI_METHOD(void, GetConnectedDeviceCallbackJni, deleteCallback)(JNIEnv * env, jo
chip::DeviceLayer::StackLock lock;
GetConnectedDeviceCallback * connectedDeviceCallback = reinterpret_cast<GetConnectedDeviceCallback *>(callbackHandle);
VerifyOrReturn(connectedDeviceCallback != nullptr, ChipLogError(Controller, "GetConnectedDeviceCallback handle is nullptr"));
delete connectedDeviceCallback;
chip::Platform::Delete(connectedDeviceCallback);
}

JNI_METHOD(jlong, ReportCallbackJni, newCallback)
Expand All @@ -56,7 +56,7 @@ JNI_METHOD(void, ReportCallbackJni, deleteCallback)(JNIEnv * env, jobject self,
chip::DeviceLayer::StackLock lock;
ReportCallback * reportCallback = reinterpret_cast<ReportCallback *>(callbackHandle);
VerifyOrReturn(reportCallback != nullptr, ChipLogError(Controller, "ReportCallback handle is nullptr"));
delete reportCallback;
chip::Platform::Delete(reportCallback);
}

JNI_METHOD(jlong, ReportEventCallbackJni, newCallback)
Expand Down Expand Up @@ -91,7 +91,7 @@ JNI_METHOD(void, WriteAttributesCallbackJni, deleteCallback)(JNIEnv * env, jobje
chip::DeviceLayer::StackLock lock;
WriteAttributesCallback * writeAttributesCallback = reinterpret_cast<WriteAttributesCallback *>(callbackHandle);
VerifyOrReturn(writeAttributesCallback != nullptr, ChipLogError(Controller, "WriteAttributesCallback handle is nullptr"));
delete writeAttributesCallback;
chip::Platform::Delete(writeAttributesCallback);
}

JNI_METHOD(jlong, InvokeCallbackJni, newCallback)
Expand All @@ -107,5 +107,5 @@ JNI_METHOD(void, InvokeCallbackJni, deleteCallback)(JNIEnv * env, jobject self,
chip::DeviceLayer::StackLock lock;
InvokeCallback * invokeCallback = reinterpret_cast<InvokeCallback *>(callbackHandle);
VerifyOrReturn(invokeCallback != nullptr, ChipLogError(Controller, "InvokeCallback handle is nullptr"));
delete invokeCallback;
chip::Platform::Delete(invokeCallback);
}
23 changes: 20 additions & 3 deletions src/controller/java/AndroidCallbacks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ void GetConnectedDeviceCallback::OnDeviceConnectionFailureFn(void * context, con

ReportCallback::ReportCallback(jobject wrapperCallback, jobject subscriptionEstablishedCallback, jobject reportCallback,
jobject resubscriptionAttemptCallback) :
mClusterCacheAdapter(*this)
mClusterCacheAdapter(*this, Optional<EventNumber>::Missing(), false /*cacheData*/)
{
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
VerifyOrReturn(env != nullptr, ChipLogError(Controller, "Could not get JNIEnv for current thread"));
Expand Down Expand Up @@ -186,6 +186,19 @@ void ReportCallback::OnReportBegin()
mNodeStateObj = env->NewObject(mNodeStateCls, nodeStateCtor, map);
}

void ReportCallback::OnDeallocatePaths(app::ReadPrepareParams && aReadPrepareParams)
{
if (aReadPrepareParams.mpAttributePathParamsList != nullptr)
{
delete[] aReadPrepareParams.mpAttributePathParamsList;
}

if (aReadPrepareParams.mpEventPathParamsList != nullptr)
{
delete[] aReadPrepareParams.mpEventPathParamsList;
}
}

void ReportCallback::OnReportEnd()
{
UpdateClusterDataVersion();
Expand Down Expand Up @@ -239,8 +252,12 @@ void ReportCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPat
readerForJson.Init(*apData);

jobject value = DecodeAttributeValue(aPath, readerForJavaObject, &err);
// If we don't know this attribute, just skip it.
VerifyOrReturn(err != CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB);
// If we don't know this attribute, suppress it.
if (err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB)
{
err = CHIP_NO_ERROR;
}

VerifyOrReturn(err == CHIP_NO_ERROR, ReportError(attributePathObj, nullptr, err));
VerifyOrReturn(!env->ExceptionCheck(), env->ExceptionDescribe(),
ReportError(attributePathObj, nullptr, CHIP_JNI_ERROR_EXCEPTION_THROWN));
Expand Down
2 changes: 2 additions & 0 deletions src/controller/java/AndroidCallbacks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ struct ReportCallback : public app::ClusterStateCache::Callback

CHIP_ERROR OnResubscriptionNeeded(app::ReadClient * apReadClient, CHIP_ERROR aTerminationCause) override;

void OnDeallocatePaths(app::ReadPrepareParams && aReadPrepareParams) override;

/** Report errors back to Java layer. attributePath may be nullptr for general errors. */
void ReportError(jobject attributePath, jobject eventPath, CHIP_ERROR err);
void ReportError(jobject attributePath, jobject eventPath, Protocols::InteractionModel::Status status);
Expand Down
168 changes: 110 additions & 58 deletions src/controller/java/CHIPDeviceController-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1355,104 +1355,156 @@ JNI_METHOD(jobject, computePaseVerifier)

JNI_METHOD(void, subscribe)
(JNIEnv * env, jobject self, jlong handle, jlong callbackHandle, jlong devicePtr, jobject attributePathList, jobject eventPathList,
jint minInterval, jint maxInterval, jboolean keepSubscriptions, jboolean isFabricFiltered)
jint minInterval, jint maxInterval, jboolean keepSubscriptions, jboolean isFabricFiltered, jint imTimeoutMs)
{
chip::DeviceLayer::StackLock lock;
CHIP_ERROR err = CHIP_NO_ERROR;

DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
CHIP_ERROR err = CHIP_NO_ERROR;
app::ReadClient * readClient = nullptr;
jint numAttributePaths = 0;
jint numEventPaths = 0;
auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
if (device == nullptr)
{
ChipLogError(Controller, "No device found");
ChipLogProgress(Controller, "Could not cast device pointer to Device object");
JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, CHIP_ERROR_INCORRECT_STATE);
return;
}
app::ReadPrepareParams params(device->GetSecureSession().Value());

std::vector<app::AttributePathParams> attributePathParamsList;
err = ParseAttributePathList(attributePathList, attributePathParamsList);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java attribute paths: %s", ErrorStr(err)));
params.mMinIntervalFloorSeconds = minInterval;
params.mMaxIntervalCeilingSeconds = maxInterval;
params.mKeepSubscriptions = (keepSubscriptions != JNI_FALSE);
params.mIsFabricFiltered = (isFabricFiltered != JNI_FALSE);
params.mTimeout = imTimeoutMs != 0 ? System::Clock::Milliseconds32(imTimeoutMs) : System::Clock::kZero;

std::vector<app::EventPathParams> eventPathParamsList;
err = ParseEventPathList(eventPathList, eventPathParamsList);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java event paths: %s", ErrorStr(err)));
SuccessOrExit(err = JniReferences::GetInstance().GetListSize(attributePathList, numAttributePaths));
if (numAttributePaths > 0)
{
std::unique_ptr<chip::app::AttributePathParams[]> attributePaths(new chip::app::AttributePathParams[numAttributePaths]);
for (uint8_t i = 0; i < numAttributePaths; i++)
{
jobject attributePathItem = nullptr;
SuccessOrExit(err = JniReferences::GetInstance().GetListItem(attributePathList, i, attributePathItem));

EndpointId endpointId;
ClusterId clusterId;
AttributeId attributeId;
SuccessOrExit(err = ParseAttributePath(attributePathItem, endpointId, clusterId, attributeId));
attributePaths[i] = chip::app::AttributePathParams(endpointId, clusterId, attributeId);
}
params.mpAttributePathParamsList = attributePaths.get();
params.mAttributePathParamsListSize = numAttributePaths;
attributePaths.release();
}

app::ReadPrepareParams params(device->GetSecureSession().Value());
params.mMinIntervalFloorSeconds = minInterval;
params.mMaxIntervalCeilingSeconds = maxInterval;
params.mpAttributePathParamsList = attributePathParamsList.data();
params.mAttributePathParamsListSize = attributePathParamsList.size();
params.mpEventPathParamsList = eventPathParamsList.data();
params.mEventPathParamsListSize = eventPathParamsList.size();
params.mKeepSubscriptions = (keepSubscriptions != JNI_FALSE);
params.mIsFabricFiltered = (isFabricFiltered != JNI_FALSE);
SuccessOrExit(err = JniReferences::GetInstance().GetListSize(eventPathList, numEventPaths));
if (numEventPaths > 0)
{
std::unique_ptr<chip::app::EventPathParams[]> eventPaths(new chip::app::EventPathParams[numEventPaths]);
for (uint8_t i = 0; i < numEventPaths; i++)
{
jobject eventPathItem = nullptr;
SuccessOrExit(err = JniReferences::GetInstance().GetListItem(eventPathList, i, eventPathItem));

EndpointId endpointId;
ClusterId clusterId;
EventId eventId;
bool isUrgent;
SuccessOrExit(err = ParseEventPath(eventPathItem, endpointId, clusterId, eventId, isUrgent));
eventPaths[i] = chip::app::EventPathParams(endpointId, clusterId, eventId, isUrgent);
}

auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
params.mpEventPathParamsList = eventPaths.get();
params.mEventPathParamsListSize = numEventPaths;
eventPaths.release();
}

readClient = Platform::New<app::ReadClient>(app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
callback->mClusterCacheAdapter.GetBufferedCallback(),
app::ReadClient::InteractionType::Subscribe);

app::ReadClient * readClient = Platform::New<app::ReadClient>(
app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
callback->mClusterCacheAdapter.GetBufferedCallback(), app::ReadClient::InteractionType::Subscribe);
SuccessOrExit(err = readClient->SendAutoResubscribeRequest(std::move(params)));
callback->mReadClient = readClient;

err = readClient->SendRequest(params);
exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "JNI IM Subscribe Error: %s", err.AsString());
if (err == CHIP_JNI_ERROR_EXCEPTION_THROWN)
{
env->ExceptionDescribe();
env->ExceptionClear();
}
callback->OnError(err);
delete readClient;
delete callback;
return;
if (readClient != nullptr)
{
Platform::Delete(readClient);
}
if (callback != nullptr)
{
Platform::Delete(callback);
}
}

callback->mReadClient = readClient;
}

JNI_METHOD(void, read)
(JNIEnv * env, jobject self, jlong handle, jlong callbackHandle, jlong devicePtr, jobject attributePathList, jobject eventPathList,
jboolean isFabricFiltered)
jboolean isFabricFiltered, jint imTimeoutMs)
{
chip::DeviceLayer::StackLock lock;
CHIP_ERROR err = CHIP_NO_ERROR;

DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
std::vector<app::AttributePathParams> attributePathParamsList;
std::vector<app::EventPathParams> eventPathParamsList;
app::ReadClient * readClient = nullptr;
DeviceProxy * device = reinterpret_cast<DeviceProxy *>(devicePtr);
if (device == nullptr)
{
ChipLogError(Controller, "No device found");
ChipLogProgress(Controller, "Could not cast device pointer to Device object");
JniReferences::GetInstance().ThrowError(env, sChipDeviceControllerExceptionCls, CHIP_ERROR_INCORRECT_STATE);
return;
}

std::vector<app::AttributePathParams> attributePathParamsList;
err = ParseAttributePathList(attributePathList, attributePathParamsList);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java attribute paths: %s", ErrorStr(err)));

std::vector<app::EventPathParams> eventPathParamsList;
err = ParseEventPathList(eventPathList, eventPathParamsList);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error parsing Java event paths: %s", ErrorStr(err)));

VerifyOrReturn(attributePathParamsList.size() != 0 || eventPathParamsList.size() != 0,
ChipLogError(Controller, "Error parsing Java both event paths and attribute paths"));
app::ReadPrepareParams params(device->GetSecureSession().Value());

SuccessOrExit(err = ParseAttributePathList(attributePathList, attributePathParamsList));
SuccessOrExit(err = ParseEventPathList(eventPathList, eventPathParamsList));
VerifyOrExit(attributePathParamsList.size() != 0 || eventPathParamsList.size() != 0, err = CHIP_ERROR_INVALID_ARGUMENT);
params.mpAttributePathParamsList = attributePathParamsList.data();
params.mAttributePathParamsListSize = attributePathParamsList.size();
params.mpEventPathParamsList = eventPathParamsList.data();
params.mEventPathParamsListSize = eventPathParamsList.size();

params.mIsFabricFiltered = (isFabricFiltered != JNI_FALSE);
params.mTimeout = imTimeoutMs != 0 ? System::Clock::Milliseconds32(imTimeoutMs) : System::Clock::kZero;

auto callback = reinterpret_cast<ReportCallback *>(callbackHandle);
readClient = Platform::New<app::ReadClient>(app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
callback->mClusterCacheAdapter.GetBufferedCallback(),
app::ReadClient::InteractionType::Read);

app::ReadClient * readClient = Platform::New<app::ReadClient>(
app::InteractionModelEngine::GetInstance(), device->GetExchangeManager(),
callback->mClusterCacheAdapter.GetBufferedCallback(), app::ReadClient::InteractionType::Read);
SuccessOrExit(err = readClient->SendRequest(params));
callback->mReadClient = readClient;

err = readClient->SendRequest(params);
exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "JNI IM Read Error: %s", err.AsString());
if (err == CHIP_JNI_ERROR_EXCEPTION_THROWN)
{
env->ExceptionDescribe();
env->ExceptionClear();
}
callback->OnError(err);
delete readClient;
delete callback;
return;
if (readClient != nullptr)
{
Platform::Delete(readClient);
}
if (callback != nullptr)
{
Platform::Delete(callback);
}
}

callback->mReadClient = readClient;
}

JNI_METHOD(void, write)
Expand Down Expand Up @@ -1576,11 +1628,11 @@ JNI_METHOD(void, write)
callback->OnError(writeClient, err);
if (writeClient != nullptr)
{
delete writeClient;
Platform::Delete(writeClient);
}
if (callback != nullptr)
{
delete callback;
Platform::Delete(callback);
}
}
}
Expand Down Expand Up @@ -1684,11 +1736,11 @@ JNI_METHOD(void, invoke)
callback->OnError(nullptr, err);
if (commandSender != nullptr)
{
delete commandSender;
Platform::Delete(commandSender);
}
if (callback != nullptr)
{
delete callback;
Platform::Delete(callback);
}
}
}
Expand Down
Loading

0 comments on commit 53510fe

Please sign in to comment.