Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up compilation of all our WriteAttribute machinery. #11603

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/app/chip_data_model.gni
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ template("chip_data_model") {
"${invoker.zap_pregenerated_dir}/CHIPClusters.cpp",
"${invoker.zap_pregenerated_dir}/CHIPClusters.h",
"${invoker.zap_pregenerated_dir}/CHIPClustersInvoke.cpp",
"${invoker.zap_pregenerated_dir}/CHIPClustersWrite.cpp",
"${invoker.zap_pregenerated_dir}/attribute-size.cpp",
"${invoker.zap_pregenerated_dir}/callback-stub.cpp",
]
Expand All @@ -178,7 +177,6 @@ template("chip_data_model") {
sources += [
"${invoker.zap_pregenerated_dir}/tests/CHIPClustersTest.cpp",
"${invoker.zap_pregenerated_dir}/tests/CHIPClustersTest.h",
"${invoker.zap_pregenerated_dir}/tests/CHIPClustersTestWrite.cpp",
]
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/app/tests/suites/templates/templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@
"path": "../../../zap-templates/templates/app/tests/CHIPClusters-src.zapt",
"name": "Tests C++ API",
"output": "tests/CHIPClustersTest.cpp"
},
{
"path": "../../../zap-templates/templates/app/tests/CHIPClustersWrite-src.zapt",
"name": "Tests WriteAttribute instantiations",
"output": "tests/CHIPClustersTestWrite.cpp"
}
]
}
5 changes: 0 additions & 5 deletions src/app/zap-templates/app-templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@
"path": "templates/app/CHIPClustersInvoke-src.zapt",
"name": "C++ ZCL Invoke API",
"output": "CHIPClustersInvoke.cpp"
},
{
"path": "templates/app/CHIPClustersWrite-src.zapt",
"name": "C++ ZCL Write API",
"output": "CHIPClustersWrite.cpp"
}
]
}
56 changes: 0 additions & 56 deletions src/app/zap-templates/templates/app/CHIPClustersWrite-src.zapt

This file was deleted.

This file was deleted.

38 changes: 37 additions & 1 deletion src/controller/CHIPCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,45 @@ class DLL_EXPORT ClusterBase
CHIP_ERROR InvokeCommand(const RequestDataT & requestData, void * context,
CommandResponseSuccessCallback<ResponseDataT> successCb, CommandResponseFailureCallback failureCb);

/**
* Functions for writing attributes. We have lots of different
* AttributeInfo but a fairly small set of types that get written. So we
* want to keep the template on AttributeInfo very small, and put all the
* work in the template with a small number of instantiations (one per
* type).
*/
template <typename AttrType>
CHIP_ERROR WriteAttribute(const AttrType & requestData, void * context, ClusterId clusterId, AttributeId attributeId,
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb)
{
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

auto onSuccessCb = [context, successCb](const app::ConcreteAttributePath & commandPath) {
if (successCb != nullptr)
{
successCb(context);
}
};

auto onFailureCb = [context, failureCb](const app::ConcreteAttributePath * commandPath, app::StatusIB status,
CHIP_ERROR aError) {
if (failureCb != nullptr)
{
failureCb(context, app::ToEmberAfStatus(status.mStatus));
}
};

return chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId, attributeId,
requestData, onSuccessCb, onFailureCb);
}

template <typename AttributeInfo>
CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context,
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb);
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb)
{
return WriteAttribute(requestData, context, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), successCb,
failureCb);
}

protected:
ClusterBase(uint16_t cluster) : mClusterId(cluster) {}
Expand Down
26 changes: 20 additions & 6 deletions src/controller/WriteInteraction.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,15 @@ class WriteCallback final : public app::WriteClient::Callback
OnDoneCallbackType mOnDone;
};

template <typename AttributeInfo>
CHIP_ERROR WriteAttribute(Messaging::ExchangeManager * aExchangeMgr, SessionHandle sessionHandle, chip::EndpointId endpointId,
const typename AttributeInfo::Type & requestCommandData, WriteCallback::OnSuccessCallbackType onSuccessCb,
/**
* Functions for writing attributes. We have lots of different AttributeInfo
* but a fairly small set of types that get written. So we want to keep the
* template on AttributeInfo very small, and put all the work in the template
* with a small number of instantiations (one per type).
*/
template <typename AttrType>
CHIP_ERROR WriteAttribute(SessionHandle sessionHandle, chip::EndpointId endpointId, ClusterId clusterId, AttributeId attributeId,
const AttrType & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb,
WriteCallback::OnErrorCallbackType onErrorCb)
{
app::WriteClientHandle handle;
Expand All @@ -91,14 +97,22 @@ CHIP_ERROR WriteAttribute(Messaging::ExchangeManager * aExchangeMgr, SessionHand
VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY);

ReturnErrorOnFailure(app::InteractionModelEngine::GetInstance()->NewWriteClient(handle, callback.get()));
ReturnErrorOnFailure(handle.EncodeAttributeWritePayload(
chip::app::AttributePathParams(endpointId, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId()),
requestCommandData));
ReturnErrorOnFailure(
handle.EncodeAttributeWritePayload(chip::app::AttributePathParams(endpointId, clusterId, attributeId), requestData));
ReturnErrorOnFailure(handle.SendWriteRequest(sessionHandle));

callback.release();
return CHIP_NO_ERROR;
}

template <typename AttributeInfo>
CHIP_ERROR WriteAttribute(SessionHandle sessionHandle, chip::EndpointId endpointId,
const typename AttributeInfo::Type & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb,
WriteCallback::OnErrorCallbackType onErrorCb)
{
return WriteAttribute(sessionHandle, endpointId, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), requestData,
onSuccessCb, onErrorCb);
}

} // namespace Controller
} // namespace chip
7 changes: 1 addition & 6 deletions src/controller/data_model/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,7 @@ chip_data_model("data_model") {
zap_pregenerated_dir =
"${chip_root}/zzz_generated/controller-clusters/zap-generated"

# On Android, we don't really need the tests APIs, and the compiler has
# trouble compiling some of those files anyway.
#
# TODO: This should default to false and consumers should opt in to it
# as needed.
use_tests_apis = current_os != "android"
use_tests_apis = true
use_default_client_callbacks = true
allow_circular_includes_from = [ "${chip_root}/src/controller" ]
}
8 changes: 4 additions & 4 deletions src/controller/tests/data_model/TestWrite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ void TestWriteInteraction::TestDataResponse(nlTestSuite * apSuite, void * apCont
auto onFailureCb = [&onFailureCbInvoked](const app::ConcreteAttributePath * attributePath, app::StatusIB status,
CHIP_ERROR aError) { onFailureCbInvoked = true; };

chip::Controller::WriteAttribute<TestCluster::Attributes::ListStructOctetString::TypeInfo>(
&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, value, onSuccessCb, onFailureCb);
chip::Controller::WriteAttribute<TestCluster::Attributes::ListStructOctetString::TypeInfo>(sessionHandle, kTestEndpointId,
value, onSuccessCb, onFailureCb);

ctx.DrainAndServiceIO();

Expand Down Expand Up @@ -189,8 +189,8 @@ void TestWriteInteraction::TestAttributeError(nlTestSuite * apSuite, void * apCo
onFailureCbInvoked = true;
};

chip::Controller::WriteAttribute<TestCluster::Attributes::ListStructOctetString::TypeInfo>(
&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, value, onSuccessCb, onFailureCb);
chip::Controller::WriteAttribute<TestCluster::Attributes::ListStructOctetString::TypeInfo>(sessionHandle, kTestEndpointId,
value, onSuccessCb, onFailureCb);

ctx.DrainAndServiceIO();

Expand Down
4 changes: 0 additions & 4 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
2F79A67726CE6672006377B0 /* im-client-callbacks.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2F79A67626CE6672006377B0 /* im-client-callbacks.cpp */; };
2FD775552695557E00FF4B12 /* error-mapping.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2FD775542695557E00FF4B12 /* error-mapping.cpp */; };
5129BCFD26A9EE3300122DDF /* CHIPError.h in Headers */ = {isa = PBXBuildFile; fileRef = 5129BCFC26A9EE3300122DDF /* CHIPError.h */; settings = {ATTRIBUTES = (Public, ); }; };
515CCFD227351183002A3C82 /* CHIPClustersTestWrite.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */; };
991DC0842475F45400C13860 /* CHIPDeviceController.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC0822475F45400C13860 /* CHIPDeviceController.h */; settings = {ATTRIBUTES = (Public, ); }; };
991DC0892475F47D00C13860 /* CHIPDeviceController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 991DC0872475F47D00C13860 /* CHIPDeviceController.mm */; };
991DC08B247704DC00C13860 /* CHIPLogging.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC08A247704DC00C13860 /* CHIPLogging.h */; };
Expand Down Expand Up @@ -142,7 +141,6 @@
2F79A67626CE6672006377B0 /* im-client-callbacks.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "im-client-callbacks.cpp"; path = "../../../app/util/im-client-callbacks.cpp"; sourceTree = "<group>"; };
2FD775542695557E00FF4B12 /* error-mapping.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "error-mapping.cpp"; path = "../../../app/util/error-mapping.cpp"; sourceTree = "<group>"; };
5129BCFC26A9EE3300122DDF /* CHIPError.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = CHIPError.h; path = CHIP/CHIPError.h; sourceTree = "<group>"; };
515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = CHIPClustersTestWrite.cpp; path = "../../../zzz_generated/controller-clusters/zap-generated/tests/CHIPClustersTestWrite.cpp"; sourceTree = "<group>"; };
991DC0822475F45400C13860 /* CHIPDeviceController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceController.h; sourceTree = "<group>"; };
991DC0872475F47D00C13860 /* CHIPDeviceController.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPDeviceController.mm; sourceTree = "<group>"; };
991DC08A247704DC00C13860 /* CHIPLogging.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPLogging.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -242,7 +240,6 @@
B20252832459E34F00F97062 = {
isa = PBXGroup;
children = (
515CCFD127351183002A3C82 /* CHIPClustersTestWrite.cpp */,
5129BCFC26A9EE3300122DDF /* CHIPError.h */,
BA107AEE2470CFBB004287EB /* chip_xcode_build_connector.sh */,
B202528F2459E34F00F97062 /* CHIP */,
Expand Down Expand Up @@ -481,7 +478,6 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
515CCFD227351183002A3C82 /* CHIPClustersTestWrite.cpp in Sources */,
2C8C8FC2253E0C2100797F05 /* CHIPPersistentStorageDelegateBridge.mm in Sources */,
2CB7163C252E8A7C0026E2BB /* CHIPDevicePairingDelegateBridge.mm in Sources */,
997DED162695343400975E97 /* CHIPThreadOperationalDataset.mm in Sources */,
Expand Down
18 changes: 0 additions & 18 deletions zzz_generated/all-clusters-app/zap-generated/CHIPClustersWrite.cpp

This file was deleted.

18 changes: 0 additions & 18 deletions zzz_generated/bridge-app/zap-generated/CHIPClustersWrite.cpp

This file was deleted.

Loading