From 1838aae7c211282ee4e31f326406c899243f464d Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Tue, 20 Aug 2024 21:58:39 -0400
Subject: [PATCH] Make descriptions for MTRDevice clearly say whether it's the
 XPC version.

This requires hoisting _nodeID and _deviceController ivars clearly into the
MTRDevice superclass, so they can be accessed from subclasses.

The XPC version does not have a bunch of the state the non-XPC one does, so for
now it does not try to log that
---
 src/darwin/Framework/CHIP/MTRDevice.mm        | 75 +------------------
 .../Framework/CHIP/MTRDevice_Concrete.mm      |  8 +-
 .../Framework/CHIP/MTRDevice_Internal.h       |  8 ++
 src/darwin/Framework/CHIP/MTRDevice_XPC.mm    | 10 +++
 4 files changed, 21 insertions(+), 80 deletions(-)

diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index ff4a76147d855f..f17e6d5c90f6eb 100644
--- a/src/darwin/Framework/CHIP/MTRDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -173,23 +173,6 @@ bool HaveSubscriptionEstablishedRightNow(MTRInternalDeviceState state)
     return state == MTRInternalDeviceStateInitialSubscriptionEstablished || state == MTRInternalDeviceStateLaterSubscriptionEstablished;
 }
 
-NSString * InternalDeviceStateString(MTRInternalDeviceState state)
-{
-    switch (state) {
-    case MTRInternalDeviceStateUnsubscribed:
-        return @"Unsubscribed";
-    case MTRInternalDeviceStateSubscribing:
-        return @"Subscribing";
-    case MTRInternalDeviceStateInitialSubscriptionEstablished:
-        return @"InitialSubscriptionEstablished";
-    case MTRInternalDeviceStateResubscribing:
-        return @"Resubscribing";
-    case MTRInternalDeviceStateLaterSubscriptionEstablished:
-        return @"LaterSubscriptionEstablished";
-    default:
-        return @"Unknown";
-    }
-}
 } // anonymous namespace
 
 typedef NS_ENUM(NSUInteger, MTRDeviceExpectedValueFieldIndex) {
@@ -491,6 +474,8 @@ - (instancetype)initForSubclassesWithNodeID:(NSNumber *)nodeID controller:(MTRDe
     if (self = [super init]) {
         _lock = OS_UNFAIR_LOCK_INIT;
         _delegates = [NSMutableSet set];
+        _deviceController = controller;
+        _nodeID = nodeID;
     }
 
     return self;
@@ -546,62 +531,6 @@ - (void)dealloc
     MTR_LOG("MTRDevice dealloc: %p", self);
 }
 
-- (NSString *)description
-{
-    id _Nullable vid;
-    id _Nullable pid;
-    NSNumber * _Nullable networkFeatures;
-    MTRInternalDeviceState internalDeviceState;
-    uint32_t lastSubscriptionAttemptWait;
-    NSDate * _Nullable mostRecentReportTime;
-    NSDate * _Nullable lastSubscriptionFailureTime;
-    {
-        std::lock_guard lock(_descriptionLock);
-        vid = _vid;
-        pid = _pid;
-        networkFeatures = _allNetworkFeatures;
-        internalDeviceState = _internalDeviceStateForDescription;
-        lastSubscriptionAttemptWait = _lastSubscriptionAttemptWaitForDescription;
-        mostRecentReportTime = _mostRecentReportTimeForDescription;
-        lastSubscriptionFailureTime = _lastSubscriptionFailureTimeForDescription;
-    }
-
-    if (vid == nil) {
-        vid = @"Unknown";
-    }
-
-    if (pid == nil) {
-        pid = @"Unknown";
-    }
-
-    NSString * wifi;
-    NSString * thread;
-    if (networkFeatures == nil) {
-        wifi = @"NO";
-        thread = @"NO";
-    } else {
-        wifi = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureWiFiNetworkInterface);
-        thread = YES_NO(networkFeatures.unsignedLongLongValue & MTRNetworkCommissioningFeatureThreadNetworkInterface);
-    }
-
-    NSString * reportAge;
-    if (mostRecentReportTime) {
-        reportAge = [NSString stringWithFormat:@" (%.0lfs ago)", -[mostRecentReportTime timeIntervalSinceNow]];
-    } else {
-        reportAge = @"";
-    }
-
-    NSString * subscriptionFailureAge;
-    if (lastSubscriptionFailureTime) {
-        subscriptionFailureAge = [NSString stringWithFormat:@" (%.0lfs ago)", -[lastSubscriptionFailureTime timeIntervalSinceNow]];
-    } else {
-        subscriptionFailureAge = @"";
-    }
-
-    return [NSString
-        stringWithFormat:@"<MTRDevice: %p, node: %016llX-%016llX (%llu), VID: %@, PID: %@, WiFi: %@, Thread: %@, state: %@, last subscription attempt wait: %lus, queued work: %lu, last report: %@%@, last subscription failure: %@%@, controller: %@>", self, _deviceController.compressedFabricID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, vid, pid, wifi, thread, InternalDeviceStateString(internalDeviceState), static_cast<unsigned long>(lastSubscriptionAttemptWait), static_cast<unsigned long>(_asyncWorkQueue.itemCount), mostRecentReportTime, reportAge, lastSubscriptionFailureTime, subscriptionFailureAge, _deviceController.uniqueIdentifier];
-}
-
 + (MTRDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
 {
     return [controller deviceForNodeID:nodeID];
diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
index c9f4b7f07d295e..120d29aff122d4 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
@@ -58,8 +58,6 @@
 // allow readwrite access to superclass properties
 @interface MTRDevice_Concrete ()
 
-@property (nonatomic, readwrite, copy) NSNumber * nodeID;
-@property (nonatomic, readwrite, nullable) MTRDeviceController * deviceController;
 @property (nonatomic, readwrite) MTRAsyncWorkQueue<MTRDevice *> * asyncWorkQueue;
 @property (nonatomic, readwrite) MTRDeviceState state;
 @property (nonatomic, readwrite, nullable) NSDate * estimatedStartTime;
@@ -356,8 +354,6 @@ @implementation MTRDevice_Concrete {
 }
 
 // synthesize superclass property readwrite accessors
-@synthesize nodeID = _nodeID;
-@synthesize deviceController = _deviceController;
 @synthesize queue = _queue;
 @synthesize asyncWorkQueue = _asyncWorkQueue;
 @synthesize state = _state;
@@ -372,9 +368,7 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
     if (self = [super initForSubclassesWithNodeID:nodeID controller:controller]) {
         _timeSyncLock = OS_UNFAIR_LOCK_INIT;
         _descriptionLock = OS_UNFAIR_LOCK_INIT;
-        _nodeID = [nodeID copy];
         _fabricIndex = controller.fabricIndex;
-        _deviceController = controller;
         _queue
             = dispatch_queue_create("org.csa-iot.matter.framework.device.workqueue", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
         _expectedValueCache = [NSMutableDictionary dictionary];
@@ -467,7 +461,7 @@ - (NSString *)description
     }
 
     return [NSString
-        stringWithFormat:@"<MTRDevice: %p, node: %016llX-%016llX (%llu), VID: %@, PID: %@, WiFi: %@, Thread: %@, state: %@, last subscription attempt wait: %lus, queued work: %lu, last report: %@%@, last subscription failure: %@%@, controller: %@>", self, _deviceController.compressedFabricID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, vid, pid, wifi, thread, InternalDeviceStateString(internalDeviceState), static_cast<unsigned long>(lastSubscriptionAttemptWait), static_cast<unsigned long>(_asyncWorkQueue.itemCount), mostRecentReportTime, reportAge, lastSubscriptionFailureTime, subscriptionFailureAge, _deviceController.uniqueIdentifier];
+        stringWithFormat:@"<MTRDevice: %p, XPC: NO, node: %016llX-%016llX (%llu), VID: %@, PID: %@, WiFi: %@, Thread: %@, state: %@, last subscription attempt wait: %lus, queued work: %lu, last report: %@%@, last subscription failure: %@%@, controller: %@>", self, _deviceController.compressedFabricID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, vid, pid, wifi, thread, InternalDeviceStateString(internalDeviceState), static_cast<unsigned long>(lastSubscriptionAttemptWait), static_cast<unsigned long>(_asyncWorkQueue.itemCount), mostRecentReportTime, reportAge, lastSubscriptionFailureTime, subscriptionFailureAge, _deviceController.uniqueIdentifier];
 }
 
 + (MTRDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
diff --git a/src/darwin/Framework/CHIP/MTRDevice_Internal.h b/src/darwin/Framework/CHIP/MTRDevice_Internal.h
index bce5abfdfed75d..b6a59ac9321d42 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_Internal.h
+++ b/src/darwin/Framework/CHIP/MTRDevice_Internal.h
@@ -112,6 +112,14 @@ MTR_DIRECT_MEMBERS
     // Lock that protects overall device state, including delegate storage.
     os_unfair_lock _lock;
     NSMutableSet<MTRDeviceDelegateInfo *> * _delegates;
+
+    // Our node ID, with the ivar declared explicitly so it's accessible to
+    // subclasses.
+    NSNumber * _nodeID;
+
+    // Our controller.  Declared nullable because our property is, though in
+    // practice it does not look like we ever set it to nil.
+    MTRDeviceController * _Nullable _deviceController;
 }
 
 - (instancetype)initForSubclassesWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller;
diff --git a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
index 7dfb8da370823e..03f318d83517d7 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
@@ -93,6 +93,16 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
     return self;
 }
 
+- (NSString *)description
+{
+    // TODO: Figure out whether, and if so how, to log: VID, PID, WiFi, Thread,
+    // internalDeviceState (do we even have such a thing here?), last
+    // subscription attempt wait (does that apply to us?) queued work (do we
+    // have any?), last report, last subscription failure (does that apply to us?).
+    return [NSString
+        stringWithFormat:@"<MTRDevice: %p, XPC: YES, node: %016llX-%016llX (%llu), controller: %@>", self, _deviceController.compressedFabricID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, _nodeID.unsignedLongLongValue, _deviceController.uniqueIdentifier];
+}
+
 #pragma mark - Client Callbacks (MTRDeviceDelegate)
 
 // required methods for MTRDeviceDelegates