From 0179582ae61f5388b4b079798fbdf8d6b4f0ef16 Mon Sep 17 00:00:00 2001 From: Sagar Dhawan Date: Tue, 16 Jun 2020 16:21:29 -0700 Subject: [PATCH 1/6] Remove implicit dependency on misplaced header and fix the Darwin builds (#1139) * Remove implicit dependency on misplaced header and fix the Darwin builds * Remove ReferenceCounted.cpp and keep the impl in the header * Use a PeerAddress object to relay IP and Port information from the Controller * Restyled by clang-format * Remove unused header Co-authored-by: Restyled.io --- .../server/esp32/main/EchoServer.cpp | 1 + src/controller/CHIPDeviceController.cpp | 14 ++++++++++ src/controller/CHIPDeviceController.h | 9 ++++++ .../Framework/CHIP/CHIPDeviceController.mm | 28 +++++++++++-------- src/lib/core/ReferenceCounted.h | 14 ++++++---- 5 files changed, 50 insertions(+), 16 deletions(-) diff --git a/examples/wifi-echo/server/esp32/main/EchoServer.cpp b/examples/wifi-echo/server/esp32/main/EchoServer.cpp index 2b58a00feffb91..01d71b641106b0 100644 --- a/examples/wifi-echo/server/esp32/main/EchoServer.cpp +++ b/examples/wifi-echo/server/esp32/main/EchoServer.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 0c6652180411e9..ed26f1241415dc 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -187,6 +187,20 @@ CHIP_ERROR ChipDeviceController::ManualKeyExchange(const unsigned char * remote_ return err; } +CHIP_ERROR ChipDeviceController::PopulatePeerAddress(Transport::PeerAddress & peerAddress) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + + VerifyOrExit(IsSecurelyConnected(), err = CHIP_ERROR_INCORRECT_STATE); + + peerAddress.SetIPAddress(mDeviceAddr); + peerAddress.SetPort(mDevicePort); + peerAddress.SetTransportType(Transport::Type::kUdp); + +exit: + return err; +} + bool ChipDeviceController::IsConnected() { return kState_Initialized && (mConState == kConnectionState_Connected || mConState == kConnectionState_SecureConnected); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 533dfb97afd7b7..6e36bfb229efc2 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -89,6 +89,15 @@ class DLL_EXPORT ChipDeviceController CHIP_ERROR ManualKeyExchange(const unsigned char * remote_public_key, const size_t public_key_length, const unsigned char * local_private_key, const size_t private_key_length); + /** + * @brief + * Get the PeerAddress of a connected peer + * + * @param[inout] peerAddress The PeerAddress object which will be populated with the details of the connected peer + * @return CHIP_ERROR An error if there's no active connection + */ + CHIP_ERROR PopulatePeerAddress(Transport::PeerAddress & peerAddress); + /** * @brief * Disconnect from a connected device diff --git a/src/darwin/Framework/CHIP/CHIPDeviceController.mm b/src/darwin/Framework/CHIP/CHIPDeviceController.mm index 0f087ee5831e91..95acfec85fb934 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceController.mm +++ b/src/darwin/Framework/CHIP/CHIPDeviceController.mm @@ -34,6 +34,12 @@ static const char * const CHIP_WORK_QUEUE = "com.zigbee.chip.work"; static const char * const CHIP_SELECT_QUEUE = "com.zigbee.chip.select"; +// NOTE: Remote device ID is in sync with the echo server device id +// At some point, we may want to add an option to connect to a device without +// knowing its id, because the ID can be learned on the first response that is received. +constexpr chip::NodeId kLocalDeviceId = 112233; +constexpr chip::NodeId kRemoteDeviceId = 12344321; + @implementation AddressInfo - (instancetype)initWithIP:(NSString *)ip { @@ -89,7 +95,7 @@ - (instancetype)init return nil; } - if (CHIP_NO_ERROR != _cppController->Init()) { + if (CHIP_NO_ERROR != _cppController->Init(kLocalDeviceId)) { CHIP_LOG_ERROR("Error: couldn't initialize c++ controller"); delete _cppController; _cppController = NULL; @@ -161,7 +167,7 @@ - (BOOL)connect:(NSString *)ipAddress peer_key:(NSData *)peer_key error:(NSError * __autoreleasing *)error { - __block CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; // TODO maybe refactor // the work queue is being used for atomic access to chip's cpp controller @@ -204,13 +210,13 @@ - (BOOL)connect:(NSString *)ipAddress - (AddressInfo *)getAddressInfo { - __block CHIP_ERROR err = CHIP_NO_ERROR; - __block chip::IPAddress ipAddr; - __block uint16_t port; - + CHIP_ERROR err = CHIP_NO_ERROR; + chip::Transport::PeerAddress peerAddr = chip::Transport::PeerAddress::Uninitialized(); [self.lock lock]; - err = self.cppController->GetDeviceAddress(&ipAddr, &port); + err = self.cppController->PopulatePeerAddress(peerAddr); [self.lock unlock]; + chip::IPAddress ipAddr = peerAddr.GetIPAddress(); + uint16_t port = peerAddr.GetPort(); if (err != CHIP_NO_ERROR) { return nil; @@ -227,7 +233,7 @@ - (AddressInfo *)getAddressInfo - (BOOL)sendMessage:(NSData *)message error:(NSError * __autoreleasing *)error { - __block CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; [self.lock lock]; size_t messageLen = [message length]; @@ -252,7 +258,7 @@ - (BOOL)sendMessage:(NSData *)message error:(NSError * __autoreleasing *)error - (BOOL)sendCHIPCommand:(ChipZclClusterId_t)cluster command:(ChipZclCommandId_t)command { - __block CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; [self.lock lock]; // FIXME: This needs a better buffersizing setup! static const size_t bufferSize = 1024; @@ -288,7 +294,7 @@ - (BOOL)sendCHIPCommand:(ChipZclClusterId_t)cluster command:(ChipZclCommandId_t) - (BOOL)disconnect:(NSError * __autoreleasing *)error { - __block CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; [self.lock lock]; @@ -307,7 +313,7 @@ - (BOOL)disconnect:(NSError * __autoreleasing *)error - (BOOL)isConnected { - __block bool isConnected = false; + bool isConnected = false; [self.lock lock]; isConnected = self.cppController->IsConnected(); diff --git a/src/lib/core/ReferenceCounted.h b/src/lib/core/ReferenceCounted.h index 6a0ee27e574eee..b2ca34eabd6006 100644 --- a/src/lib/core/ReferenceCounted.h +++ b/src/lib/core/ReferenceCounted.h @@ -24,9 +24,7 @@ #ifndef REFERENCE_COUNTED_H_ #define REFERENCE_COUNTED_H_ -#include - -#include +#include namespace chip { @@ -43,7 +41,10 @@ class ReferenceCounted /** Adds one to the usage count of this class */ SUBCLASS * Retain(void) { - VerifyOrDie(mRefCount < UINT8_MAX); + if (mRefCount < UINT8_MAX) + { + abort(); + } ++mRefCount; return reinterpret_cast(this); @@ -52,7 +53,10 @@ class ReferenceCounted /** Release usage of this class */ void Release(void) { - VerifyOrDie(mRefCount != 0); + if (mRefCount != 0) + { + abort(); + } if (--mRefCount == 0) { From dda7932c7999b10821377a27e99b3c259eac962d Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 16 Jun 2020 16:46:11 -0700 Subject: [PATCH 2/6] Use correct key as default in iOS app (#1141) --- .../CHIPTool/CHIPTool/CHIPViewControllerBase.h | 4 ++-- .../CHIPTool/CHIPTool/CHIPViewControllerBase.m | 16 ++++++++-------- .../CHIPTool/UI/Base.lproj/Main.storyboard | 18 ++++++++---------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.h b/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.h index 90a7bc675daad6..4e3102bea9410b 100644 --- a/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.h +++ b/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.h @@ -22,8 +22,8 @@ NS_ASSUME_NONNULL_BEGIN @interface CHIPViewControllerBase : UIViewController -@property (readwrite) BOOL useCorrectKey; -@property (readwrite) BOOL useCorrectKeyStateChanged; +@property (readwrite) BOOL useIncorrectKey; +@property (readwrite) BOOL useIncorrectKeyStateChanged; @property (readwrite) CHIPDeviceController * chipController; @property (weak, nonatomic) IBOutlet UILabel * resultLabel; @property (weak, nonatomic) IBOutlet UISwitch * encryptionKeySwitch; diff --git a/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.m b/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.m index 70b1546b7481a2..c12985f8d7792e 100644 --- a/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.m +++ b/src/darwin/CHIPTool/CHIPTool/CHIPViewControllerBase.m @@ -84,8 +84,8 @@ - (void)viewDidLoad } [self.serverIPTextField setHidden:shouldHide]; [self.IPLabel setHidden:shouldHide]; - self.useCorrectKey = YES; - self.useCorrectKeyStateChanged = NO; + self.useIncorrectKey = NO; + self.useIncorrectKeyStateChanged = NO; } - (void)_appEnteredBackground:(NSNotification *)notification @@ -122,7 +122,7 @@ - (void)_connect NSData * peer_key = [NSData dataWithBytes:peer_key_bytes length:sizeof(peer_key_bytes)]; NSData * local_key = NULL; - if (self.useCorrectKey) { + if (!self.useIncorrectKey) { NSLog(@"Using correct key"); local_key = [NSData dataWithBytes:local_key_bytes length:sizeof(local_key_bytes)]; } else { @@ -143,13 +143,13 @@ - (void)_connect - (IBAction)encryptionKey:(id)sender { if ([self.encryptionKeySwitch isOn]) { - self.useCorrectKey = YES; + self.useIncorrectKey = YES; [self postResult:@"App will use correct key"]; } else { - self.useCorrectKey = NO; + self.useIncorrectKey = NO; [self postResult:@"App will use incorrect key"]; } - self.useCorrectKeyStateChanged = YES; + self.useIncorrectKeyStateChanged = YES; } - (void)dismissKeyboard @@ -180,9 +180,9 @@ - (void)reconnectIfNeeded } } - if (!addrInfo || needsReconnect || self.useCorrectKeyStateChanged) { + if (!addrInfo || needsReconnect || self.useIncorrectKeyStateChanged) { [self _connect]; - self.useCorrectKeyStateChanged = NO; + self.useIncorrectKeyStateChanged = NO; } } diff --git a/src/darwin/CHIPTool/CHIPTool/UI/Base.lproj/Main.storyboard b/src/darwin/CHIPTool/CHIPTool/UI/Base.lproj/Main.storyboard index 17141aedc6efaa..5468121cf3b87c 100644 --- a/src/darwin/CHIPTool/CHIPTool/UI/Base.lproj/Main.storyboard +++ b/src/darwin/CHIPTool/CHIPTool/UI/Base.lproj/Main.storyboard @@ -384,13 +384,13 @@ -