From 146e8650387fbde22f62dead988fd999d6fb048e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 7 Mar 2022 23:05:46 -0500 Subject: [PATCH] Don't kill a wildcard subscription on Darwin if there is an error for (#15943) one attribute. Instead just report the error to the subscription client. --- .../TemperatureSensorViewController.m | 6 ++- src/darwin/Framework/CHIP/CHIPDevice.h | 14 ++++-- src/darwin/Framework/CHIP/CHIPDevice.mm | 43 +++++++++---------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/darwin/CHIPTool/CHIPTool/View Controllers/Temperature Sensor/TemperatureSensorViewController.m b/src/darwin/CHIPTool/CHIPTool/View Controllers/Temperature Sensor/TemperatureSensorViewController.m index 39472e86907171..96ad42b82a6c92 100644 --- a/src/darwin/CHIPTool/CHIPTool/View Controllers/Temperature Sensor/TemperatureSensorViewController.m +++ b/src/darwin/CHIPTool/CHIPTool/View Controllers/Temperature Sensor/TemperatureSensorViewController.m @@ -213,7 +213,11 @@ - (void)reportFromUserEnteredSettings // These should be exposed by the SDK if ([report.path.cluster isEqualToNumber:@(1026)] && [report.path.attribute isEqualToNumber:@(0)]) { - [self updateTempInUI:((NSNumber *) report.value).shortValue]; + if (report.error != nil) { + NSLog(@"Error reading temperature: %@", report.error); + } else { + [self updateTempInUI:((NSNumber *) report.value).shortValue]; + } } } } diff --git a/src/darwin/Framework/CHIP/CHIPDevice.h b/src/darwin/Framework/CHIP/CHIPDevice.h index d7617e767ed4a2..4af67485e7eade 100644 --- a/src/darwin/Framework/CHIP/CHIPDevice.h +++ b/src/darwin/Framework/CHIP/CHIPDevice.h @@ -55,11 +55,13 @@ extern NSString * const kCHIPStatusKey; * clusters, all attributes, all events) on the device. * * reportHandler will be called any time a data update is available (with a - * non-nil "value" and nil "error"), or any time there is an error (with a nil - * "value" and non-nil "error"). If it's called with an error, that will - * terminate the subscription. + * non-nil "value" and nil "error"), or any time there is an error for the + * entire subscription (with a nil "value" and non-nil "error"). If it's called + * with an error, that will terminate the subscription. * - * The array passed to reportHandler will contain CHIPAttributeReport instances. + * The array passed to reportHandler will contain CHIPAttributeReport + * instances. Errors for specific paths, not the whole subscription, will be + * reported via those objects. * * subscriptionEstablished block, if not nil, will be called once the * subscription is established. This will be _after_ the first (priming) call @@ -172,6 +174,10 @@ extern NSString * const kCHIPStatusKey; @property (nonatomic, readonly, strong, nonnull) CHIPAttributePath * path; // value is nullable because nullable attributes can have nil as value. @property (nonatomic, readonly, strong, nullable) id value; +// If this specific path resulted in an error, the error (in the +// MatterInteractionErrorDomain or CHIPErrorDomain) that corresponds to this +// path. +@property (nonatomic, readonly, strong, nullable) NSError * error; @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/CHIPDevice.mm b/src/darwin/Framework/CHIP/CHIPDevice.mm index cf7f68921a9c3e..58586695bd4d01 100644 --- a/src/darwin/Framework/CHIP/CHIPDevice.mm +++ b/src/darwin/Framework/CHIP/CHIPDevice.mm @@ -74,7 +74,7 @@ - (instancetype)initWithPath:(const ConcreteDataAttributePath &)path; @end @interface CHIPAttributeReport () -- (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(nullable id)value; +- (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(nullable id)value error:(nullable NSError *)error; @end @interface CHIPReadClientContainer : NSObject @@ -1193,11 +1193,12 @@ - (instancetype)initWithPath:(const ConcreteDataAttributePath &)path @end @implementation CHIPAttributeReport -- (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(nullable id)value +- (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(nullable id)value error:(nullable NSError *)error { if (self = [super init]) { _path = [[CHIPAttributePath alloc] initWithPath:path]; _value = value; + _error = error; } return self; } @@ -1226,35 +1227,33 @@ - (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(null return; } + id _Nullable value = nil; + NSError * _Nullable error = nil; if (aStatus.mStatus != Status::Success) { - ReportError(aStatus); - return; - } - - if (apData == nullptr) { - ReportError(CHIP_ERROR_INVALID_ARGUMENT); - return; - } - - CHIP_ERROR err; - id _Nullable value = CHIPDecodeAttributeValue(aPath, *apData, &err); - if (err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH) { - // We don't know this attribute; just skip it. - return; - } + error = [CHIPError errorForIMStatus:aStatus]; + } else if (apData == nullptr) { + error = [CHIPError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]; + } else { + CHIP_ERROR err; + value = CHIPDecodeAttributeValue(aPath, *apData, &err); + if (err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH) { + // We don't know this attribute; just skip it. + return; + } - if (err != CHIP_NO_ERROR) { - ReportError(err); - return; + if (err != CHIP_NO_ERROR) { + value = nil; + error = [CHIPError errorForCHIPErrorCode:err]; + } } if (mReports == nil) { - // Never got a OnReportBegin? + // Never got a OnReportBegin? Not much to do other than tear things down. ReportError(CHIP_ERROR_INCORRECT_STATE); return; } - [mReports addObject:[[CHIPAttributeReport alloc] initWithPath:aPath value:value]]; + [mReports addObject:[[CHIPAttributeReport alloc] initWithPath:aPath value:value error:error]]; } void SubscriptionCallback::OnError(CHIP_ERROR aError) { ReportError([CHIPError errorForCHIPErrorCode:aError]); }