Skip to content

Commit

Permalink
Stop using ReadInteraction APIs for Darwin framework read/subscribe.
Browse files Browse the repository at this point in the history
This reduces Darwin build CI times from ~2 hours 30 mins to ~1 hour 20 mins.

Also includes the following fixes:

* Fix support for setting resubscribeIfLost to false in the Darwin "subsribe an
  attribute" APIs.
* Fix handling of the --autoResubscribe argument for darwin-framework-tool
  "subscribe an attribute" commands.  It used to be ignored, because the
  underlying API did not support setting it to false.
* Fix handling of fabricFiltered and keepSubscriptions in darwin-framework-tool
  YAML tests.
* Fix handling of the mKeepAlive member of Darwin callback bridges.  Before
  this, we could end up with an OnError callback, which would queue a task to
  delete the bridge, but then OnDone for a subscription would also delete the
  bridge.  This would fail any time a subscription in fact errored out.  In the
  new setup, mKeepAlive only gets set for subscriptions, and only once we have
  started the async process that will always call OnDone, so we can rely on
  deleting on error if !mKeepAlive and deleting in OnDone if mKeepAlive.
  • Loading branch information
bzbarsky-apple committed Nov 8, 2022
1 parent 092c91c commit 78d8afb
Show file tree
Hide file tree
Showing 13 changed files with 29,547 additions and 47,674 deletions.
3 changes: 3 additions & 0 deletions examples/darwin-framework-tool/templates/commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ public:
if (mFabricFiltered.HasValue()) {
params.filterByFabric = mFabricFiltered.Value();
}
if (mAutoResubscribe.HasValue()) {
params.resubscribeIfLost = mAutoResubscribe.Value();
}
[cluster subscribe{{>attribute}}WithParams:params
subscriptionEstablished:^(){ mSubscriptionEstablished=YES; }
reportHandler:^({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ class {{filename}}: public TestCommandBridge
{{asObjectiveCBasicType type}} {{asLowerCamelCase name}}Argument = {{asTypedLiteral definedValue type}};
{{/chip_tests_item_parameters}}
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(minIntervalArgument) maxInterval:@(maxIntervalArgument)];
params.filterByFabric = {{#if fabricFiltered}}true{{else}}false{{/if}};
params.replaceExistingSubscriptions = {{#if keepSubscriptions}}false{{else}}true{{/if}};
[cluster subscribeAttribute{{asUpperCamelCase attribute}}WithParams:params
subscriptionEstablished:^{
VerifyOrReturn(testSendCluster{{parent.filename}}_{{waitForReport.index}}_{{asUpperCamelCase waitForReport.command}}_Fulfilled, SetCommandExitStatus(CHIP_ERROR_INCORRECT_STATE));
Expand Down
8 changes: 4 additions & 4 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,9 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const
// Callback bridge for MTRDataValueDictionaryCallback
class MTRDataValueDictionaryCallbackBridge : public MTRCallbackBridge<MTRDataValueDictionaryCallback> {
public:
MTRDataValueDictionaryCallbackBridge(dispatch_queue_t queue, MTRBaseDevice * device, MTRDeviceResponseHandler handler,
MTRActionBlock action, bool keepAlive = false)
: MTRCallbackBridge<MTRDataValueDictionaryCallback>(queue, device, handler, action, OnSuccessFn, keepAlive) {};
MTRDataValueDictionaryCallbackBridge(
dispatch_queue_t queue, MTRBaseDevice * device, MTRDeviceResponseHandler handler, MTRActionBlock action)
: MTRCallbackBridge<MTRDataValueDictionaryCallback>(queue, device, handler, action, OnSuccessFn) {};

static void OnSuccessFn(void * context, id value) { DispatchSuccess(context, value); }
};
Expand Down Expand Up @@ -1228,7 +1228,7 @@ - (void)subscribeToAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
auto readClient = Platform::New<app::ReadClient>(
engine, exchangeManager, callback->GetBufferedCallback(), chip::app::ReadClient::InteractionType::Subscribe);

if (params.replaceExistingSubscriptions) {
if (!params.resubscribeIfLost) {
err = readClient->SendRequest(readParams);
} else {
err = readClient->SendAutoResubscribeRequest(std::move(readParams));
Expand Down
23 changes: 9 additions & 14 deletions src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ typedef void (*DefaultFailureCallbackType)(void *, CHIP_ERROR);

template <class T> class MTRCallbackBridge {
public:
using SuccessCallbackType = T;

/**
* Run the given MTRLocalActionBlock on the Matter thread, then handle
* converting the value produced by the success callback to the right type
Expand All @@ -53,10 +55,9 @@ template <class T> class MTRCallbackBridge {
* Does not attempt to establish any sessions to devices. Must not be used
* with any action blocks that need a session.
*/
MTRCallbackBridge(dispatch_queue_t queue, ResponseHandler handler, MTRLocalActionBlock action, T OnSuccessFn, bool keepAlive)
MTRCallbackBridge(dispatch_queue_t queue, ResponseHandler handler, MTRLocalActionBlock action, T OnSuccessFn)
: mQueue(queue)
, mHandler(handler)
, mKeepAlive(keepAlive)
, mSuccess(OnSuccessFn, this)
, mFailure(OnFailureFn, this)
{
Expand Down Expand Up @@ -84,11 +85,10 @@ template <class T> class MTRCallbackBridge {
* we're templated over.
*/
MTRCallbackBridge(dispatch_queue_t queue, chip::NodeId nodeID, MTRDeviceController * controller, ResponseHandler handler,
MTRActionBlock action, T OnSuccessFn, bool keepAlive)
MTRActionBlock action, T OnSuccessFn)
: mQueue(queue)
, mHandler(handler)
, mAction(action)
, mKeepAlive(keepAlive)
, mSuccess(OnSuccessFn, this)
, mFailure(OnFailureFn, this)
{
Expand All @@ -101,12 +101,10 @@ template <class T> class MTRCallbackBridge {
* the success value to whatever type it needs to be to call the callback
* type we're templated over.
*/
MTRCallbackBridge(dispatch_queue_t queue, MTRBaseDevice * device, ResponseHandler handler, MTRActionBlock action, T OnSuccessFn,
bool keepAlive)
MTRCallbackBridge(dispatch_queue_t queue, MTRBaseDevice * device, ResponseHandler handler, MTRActionBlock action, T OnSuccessFn)
: mQueue(queue)
, mHandler(handler)
, mAction(action)
, mKeepAlive(keepAlive)
, mSuccess(OnSuccessFn, this)
, mFailure(OnFailureFn, this)
{
Expand Down Expand Up @@ -188,6 +186,7 @@ template <class T> class MTRCallbackBridge {

protected:
dispatch_queue_t mQueue;
bool mKeepAlive = false;

private:
static void DispatchCallbackResult(void * context, NSError * error, id value)
Expand All @@ -198,15 +197,12 @@ template <class T> class MTRCallbackBridge {
}

if (!callbackBridge->mQueue) {
delete callbackBridge;
if (!callbackBridge->mKeepAlive) {
delete callbackBridge;
}
return;
}

if (error) {
// We should delete ourselves; there will be no more callbacks.
callbackBridge->mKeepAlive = false;
}

dispatch_async(callbackBridge->mQueue, ^{
ChipLogDetail(Controller, "%s %f seconds", callbackBridge->mCookie.UTF8String,
-[callbackBridge->mRequestTime timeIntervalSinceNow]);
Expand All @@ -220,7 +216,6 @@ template <class T> class MTRCallbackBridge {

ResponseHandler mHandler;
MTRActionBlock mAction;
bool mKeepAlive;

chip::Callback::Callback<T> mSuccess;
chip::Callback::Callback<DefaultFailureCallbackType> mFailure;
Expand Down
Loading

0 comments on commit 78d8afb

Please sign in to comment.