Skip to content

Commit

Permalink
Rationalize includes in chip-tool-darwin a bit. (#18427)
Browse files Browse the repository at this point in the history
Ideally chip-tool-darwin would just include CHIP/CHIP.h.  In practice,
because it's trying to reuse various code that chip-tool also uses,
this is not possible, but we want to be as close to it as we can.

Specific changes:

* Use CHIP/CHIP.h instead of including individual framework public headers.

* Minimize use of CHIPError_Internal.h by commoning up the "log it and
  SetCommandExitStatus" logic, which is forced on us by the
  code-sharing with chip-tool.

* Minimize the use of other "internal" SDK headers.  For each one,
  document exactly why it's being used.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jun 6, 2022
1 parent fb9db27 commit 3709192
Show file tree
Hide file tree
Showing 18 changed files with 6,315 additions and 10,609 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#pragma once

#import <CHIP/CHIP.h>
#import <CHIP/CHIPDevice_Internal.h>
#import <CHIP/CHIPDevice_Internal.h> // For NSObjectFromCHIPTLV
#include <lib/support/UnitTestUtils.h>

#include "ModelCommandBridge.h"
Expand Down Expand Up @@ -99,11 +99,10 @@ class ClusterCommand : public ModelCommand {
clientQueue:callbackQueue
completion:^(
NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
responsesNeeded--;
if (err != CHIP_NO_ERROR) {
mError = err;
ChipLogProgress(chipTool, "Error: %s", chip::ErrorStr(err));
if (error != nil) {
mError = error;
LogNSError("Error", error);
}
if (responsesNeeded == 0) {
SetCommandExitStatus(mError);
Expand All @@ -121,7 +120,7 @@ class ClusterCommand : public ModelCommand {
chip::Optional<uint16_t> mTimedInteractionTimeoutMs;
chip::Optional<uint16_t> mRepeatCount;
chip::Optional<uint16_t> mRepeatDelayInMs;
CHIP_ERROR mError = CHIP_NO_ERROR;
NSError * _Nullable mError = nil;

private:
chip::ClusterId mClusterId;
Expand Down
21 changes: 12 additions & 9 deletions examples/chip-tool-darwin/commands/clusters/ModelCommandBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
*
*/

#import <CHIP/CHIPError_Internal.h>

#include "ModelCommandBridge.h"
#include <app/InteractionModelEngine.h>
#include <inttypes.h>

using namespace ::chip;
Expand All @@ -32,17 +29,23 @@
[CurrentCommissioner() getConnectedDevice:mNodeId
queue:callbackQueue
completionHandler:^(CHIPDevice * _Nullable device, NSError * _Nullable error) {
CHIP_ERROR err = CHIP_NO_ERROR;
if (error) {
err = [CHIPError errorToCHIPErrorCode:error];
} else if (device == nil) {
if (error != nil) {
SetCommandExitStatus(error, "Error getting connected device");
return;
}

CHIP_ERROR err;
if (device == nil) {
err = CHIP_ERROR_INTERNAL;
} else {
err = SendCommand(device, mEndPointId);
}

ChipLogProgress(chipTool, "Error: %s", chip::ErrorStr(err));
VerifyOrReturn(CHIP_NO_ERROR == err, SetCommandExitStatus(err));
if (err != CHIP_NO_ERROR) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
SetCommandExitStatus(err);
return;
}
}];
return CHIP_NO_ERROR;
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,15 @@ class ReadAttribute : public ModelCommand {
params:params
clientQueue:callbackQueue
completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
if (error != nil) {
LogNSError("Error reading attribute", error);
}
if (values) {
for (id item in values) {
NSLog(@"Response Item: %@", [item description]);
}
}
if (err != CHIP_NO_ERROR) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(err));
}
SetCommandExitStatus(err);
SetCommandExitStatus(error);
}];
return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -147,7 +146,7 @@ class SubscribeAttribute : public ModelCommand {
}
}
if (error || !mWait) {
SetCommandExitStatus([CHIPError errorToCHIPErrorCode:error]);
SetCommandExitStatus(error);
}
}
subscriptionEstablished:nil];
Expand Down Expand Up @@ -208,7 +207,7 @@ class SubscribeEvent : public ModelCommand {
}
errorHandler:^(NSError * error) {
if (error && !mWait) {
SetCommandExitStatus([CHIPError errorToCHIPErrorCode:error]);
SetCommandExitStatus(error);
}
}
subscriptionEstablished:^() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,15 @@ class WriteAttribute : public ModelCommand {
: nil
clientQueue:callbackQueue
completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
CHIP_ERROR chipError = [CHIPError errorToCHIPErrorCode:error];
if (chipError != CHIP_NO_ERROR) {
ChipLogError(chipTool, "Error: %s", chip::ErrorStr(chipError));
if (error != nil) {
LogNSError("Error writing attribute", error);
}
if (values) {
for (id item in values) {
NSLog(@"Response Item: %@", [item description]);
}
}
SetCommandExitStatus(chipError);
SetCommandExitStatus(error);
}];
return CHIP_NO_ERROR;
}
Expand Down
11 changes: 10 additions & 1 deletion examples/chip-tool-darwin/commands/common/CHIPCommandBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*/

#pragma once
#import <CHIP/CHIPDeviceController.h>
#import <CHIP/CHIP.h>
#include <commands/common/Command.h>
#include <commands/common/CredentialIssuerCommands.h>
#include <map>
Expand All @@ -37,6 +37,11 @@ class CHIPCommandBridge : public Command
/////////// Command Interface /////////
CHIP_ERROR Run() override;

// Will convert error to a CHIP_ERROR and call SetCommandExitStatus with the
// result. If a log string is provided, will log that plus the string
// representation of the CHIP_ERROR.
void SetCommandExitStatus(NSError * error, const char * logString = nullptr);

void SetCommandExitStatus(CHIP_ERROR status)
{
#if CHIP_CONFIG_ERROR_SOURCE
Expand Down Expand Up @@ -75,6 +80,10 @@ class CHIPCommandBridge : public Command

CHIPDeviceController * GetCommissioner(const char * identity);

// Will log the given string and given error (as progress if success, error
// if failure).
void LogNSError(const char * logString, NSError * error);

private:
CHIP_ERROR InitializeCommissioner(std::string key, chip::FabricId fabricId,
const chip::Credentials::AttestationTrustStore * trustStore);
Expand Down
24 changes: 22 additions & 2 deletions examples/chip-tool-darwin/commands/common/CHIPCommandBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
#include "CHIPCommandBridge.h"

#import "CHIPToolKeypair.h"
#import <CHIP/CHIPDeviceController.h>
#import <CHIP/CHIP.h>
#import <CHIP/CHIPError_Internal.h>

#include <core/CHIPBuildConfig.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <lib/support/CodeUtils.h>

const uint16_t kListenPort = 5541;
static CHIPToolPersistentStorageDelegate * storage = nil;
Expand Down Expand Up @@ -134,3 +135,22 @@
}
cvWaitingForResponse.notify_all();
}

void CHIPCommandBridge::SetCommandExitStatus(NSError * error, const char * logString)
{
if (logString != nullptr) {
LogNSError(logString, error);
}
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
SetCommandExitStatus(err);
}

void CHIPCommandBridge::LogNSError(const char * logString, NSError * error)
{
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
if (err == CHIP_NO_ERROR) {
ChipLogProgress(chipTool, "%s: %s", logString, chip::ErrorStr(err));
} else {
ChipLogError(chipTool, "%s: %s", logString, chip::ErrorStr(err));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "CHIPCommandStorageDelegate.h"
#import <CHIP/CHIP.h>
#import <CHIP/CHIPKeypair.h>
#include <crypto/CHIPCryptoPAL.h>

@interface CHIPToolKeypair : NSObject <CHIPKeypair>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#import "CHIPToolKeypair.h"
#include <CHIP/CHIP.h>
#import <CHIP/CHIPKeypair.h>
#include <credentials/CHIPCert.h>
#include <crypto/CHIPCryptoPAL.h>
#include <lib/asn1/ASN1.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@

#pragma once
#include "../common/CHIPCommandBridge.h"
#import <CHIP/CHIPClustersObjc.h>
#import <CHIP/CHIPDevicePairingDelegate.h>
#import <CHIP/CHIP.h>

enum class PairingMode
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@
*
*/

#import <CHIP/CHIPCommissioningParameters.h>
#import <CHIP/CHIPError_Internal.h>
#import <setup_payload/ManualSetupPayloadGenerator.h>
#import <setup_payload/SetupPayload.h>
#import <CHIP/CHIP.h>

#include "../common/CHIPCommandBridge.h"
#include "PairingCommandBridge.h"
#include "PairingDelegateBridge.h"
#include "platform/PlatformManager.h"
#include <lib/support/logging/CHIPLogging.h>

using namespace ::chip;
Expand Down Expand Up @@ -73,7 +69,10 @@
break;
}

return [CHIPError errorToCHIPErrorCode:error];
if (error != nil) {
SetCommandExitStatus(error);
}
return CHIP_NO_ERROR;
}

void PairingCommandBridge::PairWithCode(NSError * __autoreleasing * error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
*/

#include "PairingDelegateBridge.h"
#import <CHIP/CHIPCommissioningParameters.h>
#import <CHIP/CHIPError_Internal.h>
#import <CHIP/CHIP.h>

@interface CHIPToolPairingDelegate ()
@end
Expand All @@ -42,35 +41,27 @@ - (void)onStatusUpdate:(CHIPPairingStatus)status

- (void)onPairingComplete:(NSError *)error
{
NSError * __block commissionError;
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
if (err != CHIP_NO_ERROR) {
_commandBridge->SetCommandExitStatus(err);
if (error != nil) {
_commandBridge->SetCommandExitStatus(error);
return;
}
ChipLogProgress(chipTool, "Pairing Complete: %s", chip::ErrorStr(err));
ChipLogProgress(chipTool, "Pairing Complete");
NSError * commissionError;
[_commissioner commissionDevice:_deviceID commissioningParams:_params error:&commissionError];
err = [CHIPError errorToCHIPErrorCode:commissionError];
if (err != CHIP_NO_ERROR) {
_commandBridge->SetCommandExitStatus(err);
if (commissionError != nil) {
_commandBridge->SetCommandExitStatus(commissionError);
return;
}
}

- (void)onPairingDeleted:(NSError *)error
{
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
ChipLogProgress(chipTool, "Pairing Delete: %s", chip::ErrorStr(err));

_commandBridge->SetCommandExitStatus(err);
_commandBridge->SetCommandExitStatus(error, "Pairing Delete");
}

- (void)onCommissioningComplete:(NSError *)error
{
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
ChipLogProgress(chipTool, "Pairing Commissioning Complete: %s", chip::ErrorStr(err));

_commandBridge->SetCommandExitStatus(err);
_commandBridge->SetCommandExitStatus(error, "Pairing Commissioning Complete");
}

@end
7 changes: 4 additions & 3 deletions examples/chip-tool-darwin/commands/tests/TestCommandBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <zap-generated/cluster/CHIPTestClustersObjc.h>

#import <CHIP/CHIP.h>
#import <CHIP/CHIPDevice_Internal.h>
#import <CHIP/CHIPError_Internal.h>

class TestCommandBridge;
Expand Down Expand Up @@ -132,8 +131,10 @@ class TestCommandBridge : public CHIPCommandBridge,
[controller getConnectedDevice:value.nodeId
queue:mCallbackQueue
completionHandler:^(CHIPDevice * _Nullable device, NSError * _Nullable error) {
CHIP_ERROR err = [CHIPError errorToCHIPErrorCode:error];
VerifyOrReturn(CHIP_NO_ERROR == err, SetCommandExitStatus(err));
if (error != nil) {
SetCommandExitStatus(error);
return;
}

mConnectedDevices[identity] = device;
NextTest();
Expand Down
Loading

0 comments on commit 3709192

Please sign in to comment.