Skip to content

Commit 80fee9c

Browse files
Merge branch 'master' into add/ncp_wakeup_source_config
2 parents 659b4ab + 89954f9 commit 80fee9c

21 files changed

+251
-58
lines changed

.github/workflows/examples-linux-standalone.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ jobs:
247247
build"
248248
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
249249
linux debug camera-controller \
250-
out/linux-x64-camera-controller/camera-controller \
250+
out/linux-x64-camera-controller/chip-camera-controller \
251251
/tmp/bloat_reports/
252252
- name: Uploading Size Reports
253253
uses: ./.github/actions/upload-size-reports

examples/camera-controller/BUILD.gn

+2-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ static_library("camera-controller-utils") {
107107
output_dir = root_out_dir
108108
}
109109

110-
executable("camera-controller") {
110+
executable("chip-camera-controller") {
111111
sources = [ "main.cpp" ]
112112

113113
deps = [
@@ -124,5 +124,5 @@ executable("camera-controller") {
124124
}
125125

126126
group("default") {
127-
deps = [ ":camera-controller" ]
127+
deps = [ ":chip-camera-controller" ]
128128
}

src/app/CommandSender.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ CHIP_ERROR CommandSender::SendCommandRequestInternal(const SessionHandle & sessi
125125
mExchangeCtx.Grab(exchange);
126126
VerifyOrReturnError(!mExchangeCtx->IsGroupExchangeContext(), CHIP_ERROR_INVALID_MESSAGE_TYPE);
127127

128-
mExchangeCtx->SetResponseTimeout(timeout.ValueOr(session->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime)));
128+
mExchangeCtx->SetResponseTimeout(
129+
timeout.ValueOr(session->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime, true /*isFirstMessageOnExchange*/)));
129130

130131
if (mTimedInvokeTimeoutMs.HasValue())
131132
{

src/app/ReadClient.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -989,12 +989,15 @@ CHIP_ERROR ReadClient::ComputeLivenessCheckTimerTimeout(System::Clock::Timeout *
989989
// So recompute the round-trip timeout directly. Assume MRP, since in practice that is likely what is happening.
990990
auto & peerMRPConfig = mReadPrepareParams.mSessionHolder->GetRemoteMRPConfig();
991991
// Peer will assume we are idle (hence we pass kZero to GetMessageReceiptTimeout()), but will assume we treat it as active
992-
// for the response, so to match the retransmission timeout computation for the message back to the peeer, we should treat
993-
// it as active.
994-
auto roundTripTimeout = mReadPrepareParams.mSessionHolder->GetMessageReceiptTimeout(System::Clock::kZero) +
992+
// for the response, so to match the retransmission timeout computation for the message back to the peer, we should treat
993+
// it as active and handling non-initial message, isFirstMessageOnExchange needs to be set as false for
994+
// GetRetransmissionTimeout.
995+
auto roundTripTimeout =
996+
mReadPrepareParams.mSessionHolder->GetMessageReceiptTimeout(System::Clock::kZero, true /*isFirstMessageOnExchange*/) +
995997
kExpectedIMProcessingTime +
996998
GetRetransmissionTimeout(peerMRPConfig.mActiveRetransTimeout, peerMRPConfig.mIdleRetransTimeout,
997-
System::SystemClock().GetMonotonicTimestamp(), peerMRPConfig.mActiveThresholdTime);
999+
System::SystemClock().GetMonotonicTimestamp(), peerMRPConfig.mActiveThresholdTime,
1000+
false /*isFirstMessageOnExchange*/);
9981001
*aTimeout = System::Clock::Seconds16(mMaxInterval) + roundTripTimeout;
9991002
return CHIP_NO_ERROR;
10001003
}

src/app/TimedHandler.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,9 @@ CHIP_ERROR TimedHandler::HandleTimedRequestAction(Messaging::ExchangeContext * a
127127
// will send nothing and the other side will have to time out to realize
128128
// it's missed its window).
129129
auto delay = System::Clock::Milliseconds32(timeoutMs);
130-
aExchangeContext->SetResponseTimeout(
131-
std::max(delay, aExchangeContext->GetSessionHandle()->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime)));
130+
aExchangeContext->SetResponseTimeout(std::max(delay,
131+
aExchangeContext->GetSessionHandle()->ComputeRoundTripTimeout(
132+
app::kExpectedIMProcessingTime, false /*isFirstMessageOnExchange*/)));
132133
ReturnErrorOnFailure(StatusResponse::Send(Status::Success, aExchangeContext, /* aExpectResponse = */ true));
133134

134135
// Now just wait for the client.

src/controller/AutoCommissioner.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ Optional<System::Clock::Timeout> AutoCommissioner::GetCommandTimeout(DeviceProxy
623623
auto sessionHandle = device->GetSecureSession();
624624
if (sessionHandle.HasValue())
625625
{
626-
timeout = sessionHandle.Value()->ComputeRoundTripTimeout(timeout);
626+
timeout = sessionHandle.Value()->ComputeRoundTripTimeout(timeout, true /*isFirstMessageOnExchange*/);
627627
}
628628

629629
// Enforce the spec minimal timeout. Maybe this enforcement should live in

src/controller/python/chip/utils/DeviceProxyUtils.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ uint32_t pychip_DeviceProxy_ComputeRoundTripTimeout(DeviceProxy * device, uint32
7171

7272
return deviceProxy->GetSecureSession()
7373
.Value()
74-
->ComputeRoundTripTimeout(System::Clock::Milliseconds32(upperLayerProcessingTimeoutMs))
74+
->ComputeRoundTripTimeout(System::Clock::Milliseconds32(upperLayerProcessingTimeoutMs), true /*isFirstMessageOnExchange*/)
7575
.count();
7676
}
7777

src/controller/tests/TestWriteChunking.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -571,9 +571,10 @@ void TestWriteChunking::RunTest(Instructions instructions)
571571
err = writeClient->SendWriteRequest(sessionHandle);
572572
EXPECT_EQ(err, CHIP_NO_ERROR);
573573

574-
GetIOContext().DriveIOUntil(sessionHandle->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime) +
575-
System::Clock::Seconds16(1),
576-
[&]() { return GetExchangeManager().GetNumActiveExchanges() == 0; });
574+
GetIOContext().DriveIOUntil(
575+
sessionHandle->ComputeRoundTripTimeout(app::kExpectedIMProcessingTime, true /*isFirstMessageOnExchange*/) +
576+
System::Clock::Seconds16(1),
577+
[&]() { return GetExchangeManager().GetNumActiveExchanges() == 0; });
577578

578579
EXPECT_EQ(onGoingPath, app::ConcreteAttributePath());
579580
EXPECT_EQ(status.size(), instructions.expectedStatus.size());

src/controller/tests/data_model/TestRead.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -4681,10 +4681,10 @@ System::Clock::Timeout TestRead::ComputeSubscriptionTimeout(System::Clock::Secon
46814681
// Add 1000ms of slack to our max interval to make sure we hit the
46824682
// subscription liveness timer. 100ms was tried in the past and is not
46834683
// sufficient: our process can easily lose the timeslice for 100ms.
4684-
const auto & ourMrpConfig = GetDefaultMRPConfig();
4685-
auto publisherTransmissionTimeout =
4686-
GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout,
4687-
System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime);
4684+
const auto & ourMrpConfig = GetDefaultMRPConfig();
4685+
auto publisherTransmissionTimeout = GetRetransmissionTimeout(
4686+
ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout, System::SystemClock().GetMonotonicTimestamp(),
4687+
ourMrpConfig.mActiveThresholdTime, true /* isFirstMessageOnExchange */);
46884688

46894689
return publisherTransmissionTimeout + aMaxInterval + System::Clock::Milliseconds32(1000);
46904690
}

src/darwin/Framework/CHIP/MTRBaseDevice.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -1526,7 +1526,7 @@ - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID
15261526
// Clamp to a number of seconds that will not overflow 32-bit
15271527
// int when converted to ms.
15281528
auto serverTimeoutInSeconds = System::Clock::Seconds16(serverSideProcessingTimeout.unsignedShortValue);
1529-
invokeTimeout.SetValue(session->ComputeRoundTripTimeout(serverTimeoutInSeconds));
1529+
invokeTimeout.SetValue(session->ComputeRoundTripTimeout(serverTimeoutInSeconds, true /*isFirstMessageOnExchange*/));
15301530
}
15311531
ReturnErrorOnFailure(commandSender->SendCommandRequest(session, invokeTimeout));
15321532

src/darwin/Framework/CHIP/MTRCommandWithRequiredResponse.mm

+27-3
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
* limitations under the License.
1515
*/
1616

17+
#import <Matter/Matter.h>
18+
1719
#import "MTRDeviceDataValidation.h"
1820
#import "MTRLogging_Internal.h"
19-
#import <Matter/Matter.h>
21+
#import "MTRUtilities.h"
2022

2123
@implementation MTRCommandWithRequiredResponse
2224
- (instancetype)initWithPath:(MTRCommandPath *)path
@@ -66,7 +68,14 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder
6668
return nil;
6769
}
6870

69-
_commandFields = [decoder decodeObjectOfClasses:[NSSet setWithArray:@[ [NSDictionary class], [NSString class], [NSNumber class], [NSArray class], [NSData class] ]] forKey:sFieldsKey];
71+
// The classes of things that can appear in a data-value dictionary.
72+
static NSSet * const sDataValueClasses = [NSSet setWithArray:@[ NSDictionary.class, NSArray.class, NSData.class, NSString.class, NSNumber.class ]];
73+
74+
// Unfortunately, decodeDictionaryWithKeysOfClasses:objectsOfClasses:forKey:
75+
// does not work when the objects stored in the dictionary can include
76+
// collections, so we have to use decodeObjectOfClasses: and then manually
77+
// validate we got a dictionary.
78+
_commandFields = [decoder decodeObjectOfClasses:sDataValueClasses forKey:sFieldsKey];
7079
if (_commandFields) {
7180
if (![_commandFields isKindOfClass:NSDictionary.class]) {
7281
MTR_LOG_ERROR("MTRCommandWithRequiredResponse decoded %@ for commandFields, not NSDictionary.", _commandFields);
@@ -79,7 +88,7 @@ - (nullable instancetype)initWithCoder:(NSCoder *)decoder
7988
}
8089
}
8190

82-
_requiredResponse = [decoder decodeObjectOfClasses:[NSSet setWithArray:@[ [NSDictionary class], [NSString class], [NSNumber class], [NSArray class], [NSData class] ]] forKey:sExpectedResultKey];
91+
_requiredResponse = [decoder decodeObjectOfClasses:sDataValueClasses forKey:sExpectedResultKey];
8392
if (_requiredResponse) {
8493
if (![_requiredResponse isKindOfClass:NSDictionary.class]) {
8594
MTR_LOG_ERROR("MTRCommandWithRequiredResponse decoded %@ for requiredResponse, not NSDictionary.", _requiredResponse);
@@ -116,4 +125,19 @@ - (void)encodeWithCoder:(NSCoder *)coder
116125
}
117126
}
118127

128+
- (BOOL)_isEqualToOther:(MTRCommandWithRequiredResponse *)other
129+
{
130+
return MTREqualObjects(_path, other.path)
131+
&& MTREqualObjects(_commandFields, other.commandFields)
132+
&& MTREqualObjects(_requiredResponse, other.requiredResponse);
133+
}
134+
135+
- (BOOL)isEqual:(id)object
136+
{
137+
if ([object class] != [self class]) {
138+
return NO;
139+
}
140+
return [self _isEqualToOther:object];
141+
}
142+
119143
@end

src/darwin/Framework/CHIPTests/MTRDeviceTests.m

+138-1
Original file line numberDiff line numberDiff line change
@@ -3350,14 +3350,23 @@ - (void)test031_MTRDeviceAttributeCacheLocalTestStorage
33503350
XCTAssertTrue(storedAttributeCountDifferenceFromMTRDeviceReport > 300);
33513351
}
33523352

3353-
- (void)doEncodeDecodeRoundTrip:(id<NSSecureCoding>)encodable
3353+
- (NSData *)_encodeEncodable:(id<NSSecureCoding>)encodable
33543354
{
33553355
// We know all our encodables are in fact NSObject.
33563356
NSObject * obj = (NSObject *) encodable;
33573357

33583358
NSError * encodeError;
33593359
NSData * encodedData = [NSKeyedArchiver archivedDataWithRootObject:encodable requiringSecureCoding:YES error:&encodeError];
33603360
XCTAssertNil(encodeError, @"Failed to encode %@", NSStringFromClass(obj.class));
3361+
return encodedData;
3362+
}
3363+
3364+
- (void)doEncodeDecodeRoundTrip:(id<NSSecureCoding>)encodable
3365+
{
3366+
NSData * encodedData = [self _encodeEncodable:encodable];
3367+
3368+
// We know all our encodables are in fact NSObject.
3369+
NSObject * obj = (NSObject *) encodable;
33613370

33623371
NSError * decodeError;
33633372
id decodedValue = [NSKeyedUnarchiver unarchivedObjectOfClasses:[NSSet setWithObject:obj.class] fromData:encodedData error:&decodeError];
@@ -3367,6 +3376,19 @@ - (void)doEncodeDecodeRoundTrip:(id<NSSecureCoding>)encodable
33673376
XCTAssertEqualObjects(obj, decodedValue, @"Decoding for %@ did not round-trip correctly", NSStringFromClass([obj class]));
33683377
}
33693378

3379+
- (void)_ensureDecodeFails:(id<NSSecureCoding>)encodable
3380+
{
3381+
NSData * encodedData = [self _encodeEncodable:encodable];
3382+
3383+
// We know all our encodables are in fact NSObject.
3384+
NSObject * obj = (NSObject *) encodable;
3385+
3386+
NSError * decodeError;
3387+
id decodedValue = [NSKeyedUnarchiver unarchivedObjectOfClasses:[NSSet setWithObject:obj.class] fromData:encodedData error:&decodeError];
3388+
XCTAssertNil(decodedValue);
3389+
XCTAssertNotNil(decodeError);
3390+
}
3391+
33703392
- (void)test032_MTRPathClassesEncoding
33713393
{
33723394
// Test attribute path encode / decode
@@ -6023,6 +6045,121 @@ - (void)test045_MTRDeviceInvokeGroups
60236045
[self waitForExpectations:@[ updateFabricLabelExpectingWrongValueExpectation ] timeout:(2 * kTimeoutInSeconds)];
60246046
}
60256047

6048+
- (void)test046_MTRCommandWithRequiredResponseEncoding
6049+
{
6050+
// Basic test with no command fields or required response.
6051+
__auto_type * onPath = [MTRCommandPath commandPathWithEndpointID:@(1)
6052+
clusterID:@(MTRClusterIDTypeOnOffID)
6053+
commandID:@(MTRCommandIDTypeClusterOnOffCommandOnID)];
6054+
__auto_type * onCommand = [[MTRCommandWithRequiredResponse alloc] initWithPath:onPath commandFields:nil requiredResponse:nil];
6055+
[self doEncodeDecodeRoundTrip:onCommand];
6056+
6057+
// Test with both command fields and an interesting required response.
6058+
//
6059+
// NSSecureCoding tracks object identity, so we need to create new objects
6060+
// for every instance of a thing we decode/encode with a given coder to make
6061+
// sure all codepaths are exercised. Use a block that returns a new
6062+
// dictionary each time to handle this.
6063+
__auto_type structureWithAllTypes = ^{
6064+
return @{
6065+
MTRTypeKey : MTRStructureValueType,
6066+
MTRValueKey : @[
6067+
@{
6068+
MTRContextTagKey : @(0),
6069+
MTRDataKey : @ {
6070+
MTRTypeKey : MTRSignedIntegerValueType,
6071+
MTRValueKey : @(5),
6072+
},
6073+
},
6074+
@{
6075+
MTRContextTagKey : @(1),
6076+
MTRDataKey : @ {
6077+
MTRTypeKey : MTRUnsignedIntegerValueType,
6078+
MTRValueKey : @(5),
6079+
},
6080+
},
6081+
@{
6082+
MTRContextTagKey : @(2),
6083+
MTRDataKey : @ {
6084+
MTRTypeKey : MTRBooleanValueType,
6085+
MTRValueKey : @(YES),
6086+
},
6087+
},
6088+
@{
6089+
MTRContextTagKey : @(3),
6090+
MTRDataKey : @ {
6091+
MTRTypeKey : MTRUTF8StringValueType,
6092+
MTRValueKey : @("abc"),
6093+
},
6094+
},
6095+
@{
6096+
MTRContextTagKey : @(4),
6097+
MTRDataKey : @ {
6098+
MTRTypeKey : MTROctetStringValueType,
6099+
MTRValueKey : [[NSData alloc] initWithBase64EncodedString:@"APJj" options:0],
6100+
},
6101+
},
6102+
@{
6103+
MTRContextTagKey : @(5),
6104+
MTRDataKey : @ {
6105+
MTRTypeKey : MTRFloatValueType,
6106+
MTRValueKey : @(1.0),
6107+
},
6108+
},
6109+
@{
6110+
MTRContextTagKey : @(6),
6111+
MTRDataKey : @ {
6112+
MTRTypeKey : MTRDoubleValueType,
6113+
MTRValueKey : @(5.0),
6114+
},
6115+
},
6116+
@{
6117+
MTRContextTagKey : @(7),
6118+
MTRDataKey : @ {
6119+
MTRTypeKey : MTRNullValueType,
6120+
},
6121+
},
6122+
@{
6123+
MTRContextTagKey : @(8),
6124+
MTRDataKey : @ {
6125+
MTRTypeKey : MTRArrayValueType,
6126+
MTRValueKey : @[
6127+
@{
6128+
MTRDataKey : @ {
6129+
MTRTypeKey : MTRUnsignedIntegerValueType,
6130+
MTRValueKey : @(9),
6131+
},
6132+
},
6133+
],
6134+
}
6135+
},
6136+
],
6137+
};
6138+
};
6139+
6140+
// Invalid commandFields (not a dictionary)
6141+
onCommand.commandFields = (id) @[];
6142+
[self _ensureDecodeFails:onCommand];
6143+
6144+
// Invalid required response (not a dictionary)
6145+
onCommand.commandFields = nil;
6146+
onCommand.requiredResponse = (id) @[];
6147+
[self _ensureDecodeFails:onCommand];
6148+
6149+
// Invalid required response (key is not NSNumber)
6150+
onCommand.requiredResponse = @{
6151+
@("abc") : structureWithAllTypes(),
6152+
};
6153+
[self _ensureDecodeFails:onCommand];
6154+
6155+
onCommand.commandFields = structureWithAllTypes();
6156+
onCommand.requiredResponse = @{
6157+
@(1) : structureWithAllTypes(),
6158+
@(13) : structureWithAllTypes(),
6159+
};
6160+
[self doEncodeDecodeRoundTrip:onCommand];
6161+
}
6162+
60266163
@end
60276164

60286165
@interface MTRDeviceEncoderTests : XCTestCase

src/messaging/ExchangeContext.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ void ExchangeContext::SetResponseExpected(bool inResponseExpected)
7777

7878
void ExchangeContext::UseSuggestedResponseTimeout(Timeout applicationProcessingTimeout)
7979
{
80-
SetResponseTimeout(mSession->ComputeRoundTripTimeout(applicationProcessingTimeout));
80+
SetResponseTimeout(mSession->ComputeRoundTripTimeout(applicationProcessingTimeout, !HasReceivedAtLeastOneMessage()));
8181
}
8282

8383
void ExchangeContext::SetResponseTimeout(Timeout timeout)

src/messaging/ReliableMessageProtocolConfig.cpp

+10-2
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
126126
}
127127

128128
System::Clock::Timeout GetRetransmissionTimeout(System::Clock::Timeout activeInterval, System::Clock::Timeout idleInterval,
129-
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold)
129+
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold,
130+
bool isFirstMessageOnExchange)
130131
{
131132
auto timeSinceLastActivity = (System::SystemClock().GetMonotonicTimestamp() - lastActivityTime);
132133

@@ -135,7 +136,14 @@ System::Clock::Timeout GetRetransmissionTimeout(System::Clock::Timeout activeInt
135136
System::Clock::Timestamp timeout(0);
136137
for (uint8_t i = 0; i < CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1; i++)
137138
{
138-
auto baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
139+
auto baseInterval = activeInterval;
140+
// If we are calculating the timeout for the initial message, we never know whether the peer is active or not, choose
141+
// active/idle interval from PeerActiveMode of session per 4.11.2.1. Retransmissions.
142+
// If we are calculating the timeout for response message, we know the peer is active, always choose active interval.
143+
if (isFirstMessageOnExchange)
144+
{
145+
baseInterval = ((timeSinceLastActivity + timeout) < activityThreshold) ? activeInterval : idleInterval;
146+
}
139147
timeout += Messaging::ReliableMessageMgr::GetBackoff(baseInterval, i, /* computeMaxPossible */ true);
140148
}
141149

src/messaging/ReliableMessageProtocolConfig.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,12 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig();
257257
* @param[in] idleInterval The idle interval to use for the backoff calculation.
258258
* @param[in] lastActivityTime The last time some activity has been recorded.
259259
* @param[in] activityThreshold The activity threshold for a node to be considered active.
260-
*
260+
* @param[in] isFirstMessageOnExchange Indicates whether this is for the initial message on an exchange.
261261
* @return The maximum transmission time
262262
*/
263263
System::Clock::Timeout GetRetransmissionTimeout(System::Clock::Timeout activeInterval, System::Clock::Timeout idleInterval,
264-
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold);
264+
System::Clock::Timeout lastActivityTime, System::Clock::Timeout activityThreshold,
265+
bool isFirstMessageOnExchange);
265266

266267
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
267268

0 commit comments

Comments
 (0)