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.

Sign up for GitHub

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

Jump to bottom

Simplify the logic in invokeCommandWithEndpointID in MTRBaseDevice. #25199

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Simplify the logic in invokeCommandWithEndpointID in MTRBaseDevice. #25199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion src/darwin/Framework/CHIP/MTRBaseClusterUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,19 @@ template <typename InvokeBridgeType, typename ResponseType> class MTRInvokeCallb
mOnError(mBridge, error);
}

void OnDone(chip::app::CommandSender * commandSender) override { chip::Platform::Delete(this); }
void OnDone(chip::app::CommandSender * commandSender) override
{
if (!mCalledCallback) {
// This can happen if the server sends a response with an empty
// InvokeResponses list. Since we are not sending wildcard command
// paths, that's not a valid response and we should treat it as an
// error. Use the error we would have gotten if we in fact expected
// a nonempty list.
OnError(commandSender, CHIP_END_OF_TLV);
}

chip::Platform::Delete(this);
}

InvokeBridgeType * _Nonnull mBridge;

Expand Down
5 changes: 4 additions & 1 deletion src/darwin/Framework/CHIP/MTRBaseDevice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,10 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
*
* @param timeoutMs timeout in milliseconds for timed invoke, or nil.
*
* @param completion response handler will receive either values or error.
* @param completion response handler will receive either values or error. A
* path-specific error status from the command invocation
* will result in an error being passed to the completion, so
* values will only be passed in when the command succeeds.
*/
- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
Expand Down
68 changes: 36 additions & 32 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,21 +1036,47 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aCommandPath,
const app::StatusIB & aStatus, TLV::TLVReader * aReader) override;

void OnError(const app::CommandSender * apCommandSender, CHIP_ERROR aError) override { mOnError(aError); }
void OnError(const app::CommandSender * apCommandSender, CHIP_ERROR aError) override
{
if (mCalledCallback) {
return;
}
mCalledCallback = true;

mOnError(aError);
}

void OnDone(app::CommandSender * apCommandSender) override { mOnDone(apCommandSender); }
void OnDone(app::CommandSender * apCommandSender) override
{
if (!mCalledCallback) {
// This can happen if the server sends a response with an empty
// InvokeResponses list. Since we are not sending wildcard command
// paths, that's not a valid response and we should treat it as an
// error. Use the error we would have gotten if we in fact expected
// a nonempty list.
OnError(apCommandSender, CHIP_END_OF_TLV);
}

mOnDone(apCommandSender);
}

OnSuccessCallbackType mOnSuccess;
OnErrorCallbackType mOnError;
OnDoneCallbackType mOnDone;
chip::ClusterId mClusterId;
// Id of the command we send.
chip::CommandId mCommandId;
bool mCalledCallback = false;
};

void NSObjectCommandCallback::OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aCommandPath,
const app::StatusIB & aStatus, TLV::TLVReader * aReader)
{
if (mCalledCallback) {
return;
}
mCalledCallback = true;

MTRDataValueDictionaryDecodableType response;
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down Expand Up @@ -1096,11 +1122,11 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
auto * bridge = new MTRDataValueDictionaryCallbackBridge(queue, completion,
^(ExchangeManager & exchangeManager, const SessionHandle & session, MTRDataValueDictionaryCallback successCb,
MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
auto resultArray = [[NSMutableArray alloc] init];
auto resultSuccess = [[NSMutableArray alloc] init];
auto resultFailure = [[NSMutableArray alloc] init];
auto onSuccessCb = [resultArray, resultSuccess](const app::ConcreteCommandPath & commandPath,
const app::StatusIB & status, const MTRDataValueDictionaryDecodableType & responseData) {
// NSObjectCommandCallback guarantees that there will be exactly one call to either the success callback or the failure
// callback.
auto onSuccessCb = [successCb, bridge](const app::ConcreteCommandPath & commandPath, const app::StatusIB & status,
const MTRDataValueDictionaryDecodableType & responseData) {
auto resultArray = [[NSMutableArray alloc] init];
if (responseData.GetDecodedObject()) {
[resultArray addObject:@ {
MTRCommandPathKey : [[MTRCommandPath alloc] initWithPath:commandPath],
Expand All @@ -1109,16 +1135,10 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
} else {
[resultArray addObject:@ { MTRCommandPathKey : [[MTRCommandPath alloc] initWithPath:commandPath] }];
}
if ([resultSuccess count] == 0) {
[resultSuccess addObject:[NSNumber numberWithBool:YES]];
}
successCb(bridge, resultArray);
};

auto onFailureCb = [resultFailure](CHIP_ERROR aError) {
if ([resultFailure count] == 0) {
[resultFailure addObject:[MTRError errorForCHIPErrorCode:aError]];
}
};
auto onFailureCb = [failureCb, bridge](CHIP_ERROR aError) { failureCb(bridge, aError); };

app::CommandPathParams commandPath = { static_cast<chip::EndpointId>([endpointID unsignedShortValue]), 0,
static_cast<chip::ClusterId>([clusterID unsignedLongValue]),
Expand All @@ -1129,23 +1149,7 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
VerifyOrReturnError(decoder != nullptr, CHIP_ERROR_NO_MEMORY);

auto rawDecoderPtr = decoder.get();
auto onDoneCb = [rawDecoderPtr, bridge, successCb, failureCb, resultArray, resultSuccess, resultFailure](
app::CommandSender * commandSender) {
if ([resultFailure count] > 0 || [resultSuccess count] == 0) {
// Failure
if (failureCb) {
if ([resultFailure count] > 0) {
failureCb(bridge, [MTRError errorToCHIPErrorCode:resultFailure[0]]);
} else {
failureCb(bridge, CHIP_ERROR_WRITE_FAILED);
}
}
} else {
// Success
if (successCb) {
successCb(bridge, resultArray);
}
}
auto onDoneCb = [rawDecoderPtr](app::CommandSender * commandSender) {
chip::Platform::Delete(commandSender);
chip::Platform::Delete(rawDecoderPtr);
};
Expand Down
5 changes: 4 additions & 1 deletion src/darwin/Framework/CHIP/MTRDevice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ typedef NS_ENUM(NSUInteger, MTRDeviceState) {
* @param timeout timeout in milliseconds for timed invoke, or nil. This value must be within [1, UINT16_MAX], and will be clamped
* to this range.
*
* @param completion response handler will receive either values or error.
* @param completion response handler will receive either values or error. A
* path-specific error status from the command invocation
* will result in an error being passed to the completion, so
* values will only be passed in when the command succeeds.
*/
- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
Expand Down
7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ - (void)test003_InvokeCommand
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: MoveToLevelWithOnOff values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -485,6 +486,7 @@ - (void)test005_Subscribe
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -535,6 +537,7 @@ - (void)test005_Subscribe
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -1090,6 +1093,7 @@ - (void)test012_SubscriptionError
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -1681,6 +1685,7 @@ - (void)test900_SubscribeAllAttributes
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -1711,6 +1716,7 @@ - (void)test900_SubscribeAllAttributes
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down Expand Up @@ -1760,6 +1766,7 @@ - (void)test900_SubscribeAllAttributes
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);

XCTAssertNil(error);
XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], 0);

{
Expand Down