Skip to content

Commit

Permalink
Isolate default MRP config from local MPR config (#19753)
Browse files Browse the repository at this point in the history
  • Loading branch information
kghost authored and pull[bot] committed Jul 22, 2022
1 parent 675097c commit 4124958
Show file tree
Hide file tree
Showing 46 changed files with 237 additions and 203 deletions.
4 changes: 2 additions & 2 deletions examples/chef/efr32/include/CHIPProjectConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
#define CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE (512)

/**
* @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL
* @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
*
* @brief
* Active retransmit interval, or time to wait before retransmission after
Expand All @@ -138,6 +138,6 @@
* needs (e.g. sleeping period) using Service Discovery TXT record CRA key.
*
*/
#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32)
#define CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL (2000_ms32)

#define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
6 changes: 4 additions & 2 deletions examples/chip-tool/commands/discover/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ class Resolve : public DiscoverCommand, public chip::AddressResolve::NodeListene
result.address.ToString(addrBuffer);

ChipLogProgress(chipTool, "NodeId Resolution: %" PRIu64 " at %s", peerId.GetNodeId(), addrBuffer);
ChipLogProgress(chipTool, " MRP retry interval (idle): %" PRIu32 "ms", result.mrpConfig.mIdleRetransTimeout.count());
ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms", result.mrpConfig.mActiveRetransTimeout.count());
ChipLogProgress(chipTool, " MRP retry interval (idle): %" PRIu32 "ms",
result.mrpRemoteConfig.mIdleRetransTimeout.count());
ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms",
result.mrpRemoteConfig.mActiveRetransTimeout.count());
ChipLogProgress(chipTool, " Supports TCP: %s", result.supportsTcp ? "yes" : "no");
SetCommandExitStatus(CHIP_NO_ERROR);
}
Expand Down
4 changes: 2 additions & 2 deletions examples/light-switch-app/efr32/include/CHIPProjectConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
#define CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE (512)

/**
* @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL
* @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
*
* @brief
* Active retransmit interval, or time to wait before retransmission after
Expand All @@ -138,6 +138,6 @@
* needs (e.g. sleeping period) using Service Discovery TXT record CRA key.
*
*/
#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32)
#define CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL (2000_ms32)

#define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
4 changes: 2 additions & 2 deletions examples/lighting-app/efr32/include/CHIPProjectConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
#define CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE (512)

/**
* @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL
* @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
*
* @brief
* Active retransmit interval, or time to wait before retransmission after
Expand All @@ -138,6 +138,6 @@
* needs (e.g. sleeping period) using Service Discovery TXT record CRA key.
*
*/
#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32)
#define CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL (2000_ms32)

#define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
4 changes: 2 additions & 2 deletions examples/lock-app/efr32/include/CHIPProjectConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
#define CHIP_DEVICE_CONFIG_EVENT_LOGGING_DEBUG_BUFFER_SIZE (512)

/**
* @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL
* @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
*
* @brief
* Active retransmit interval, or time to wait before retransmission after
Expand All @@ -139,4 +139,4 @@
* needs (e.g. sleeping period) using Service Discovery TXT record CRA key.
*
*/
#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32)
#define CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL (2000_ms32)
4 changes: 2 additions & 2 deletions examples/window-app/efr32/include/CHIPProjectConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)

/**
* @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL
* @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
*
* @brief
* Active retransmit interval, or time to wait before retransmission after
Expand All @@ -175,6 +175,6 @@
* needs (e.g. sleeping period) using Service Discovery TXT record CRA key.
*
*/
#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32)
#define CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL (2000_ms32)

#define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
2 changes: 1 addition & 1 deletion src/app/DeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class DLL_EXPORT DeviceProxy
protected:
virtual bool IsSecureConnected() const = 0;

ReliableMessageProtocolConfig mRemoteMRPConfig = GetLocalMRPConfig();
ReliableMessageProtocolConfig mRemoteMRPConfig = GetDefaultMRPConfig();
};

} // namespace chip
2 changes: 1 addition & 1 deletion src/app/OperationalDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ CHIP_ERROR OperationalDeviceProxy::LookupPeerAddress()

void OperationalDeviceProxy::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
{
UpdateDeviceData(result.address, result.mrpConfig);
UpdateDeviceData(result.address, result.mrpRemoteConfig);
}

void OperationalDeviceProxy::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason)
Expand Down
8 changes: 3 additions & 5 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,8 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
if (mUseECM)
{
ReturnErrorOnFailure(SetTemporaryDiscriminator(mECMDiscriminator));
ReturnErrorOnFailure(mPairingSession.WaitForPairing(
mServer->GetSecureSessionManager(), mECMPASEVerifier, mECMIterations, ByteSpan(mECMSalt, mECMSaltLength),
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), this));
ReturnErrorOnFailure(mPairingSession.WaitForPairing(mServer->GetSecureSessionManager(), mECMPASEVerifier, mECMIterations,
ByteSpan(mECMSalt, mECMSaltLength), GetLocalMRPConfig(), this));
}
else
{
Expand All @@ -243,8 +242,7 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
ReturnErrorOnFailure(verifier.Deserialize(ByteSpan(serializedVerifier)));

ReturnErrorOnFailure(mPairingSession.WaitForPairing(mServer->GetSecureSessionManager(), verifier, iterationCount, saltSpan,
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
this));
GetLocalMRPConfig(), this));
}

ReturnErrorOnFailure(StartAdvertisement());
Expand Down
4 changes: 2 additions & 2 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
.SetMac(mac)
.SetPort(GetSecuredPort())
.SetInterfaceId(GetInterfaceId())
.SetMRPConfig(GetLocalMRPConfig())
.SetLocalMRPConfig(GetLocalMRPConfig())
.SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT))
.EnableIpV4(true);

Expand Down Expand Up @@ -351,7 +351,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
advertiseParameters.SetRotatingDeviceId(chip::Optional<const char *>::Value(rotatingDeviceIdHexBuffer));
#endif

advertiseParameters.SetMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));
advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));

if (!HaveOperationalCredentials())
{
Expand Down
2 changes: 1 addition & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
.fabricTable = &mFabrics,
.clientPool = &mCASEClientPool,
.groupDataProvider = mGroupsProvider,
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>(GetLocalMRPConfig()),
.mrpLocalConfig = GetLocalMRPConfig(),
},
.devicePool = &mDevicePool,
};
Expand Down
3 changes: 1 addition & 2 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,7 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
exchangeCtxt = mSystemState->ExchangeMgr()->NewContext(session.Value(), &device->GetPairing());
VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL);

err = device->GetPairing().Pair(*mSystemState->SessionMgr(), params.GetSetupPINCode(),
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
err = device->GetPairing().Pair(*mSystemState->SessionMgr(), params.GetSetupPINCode(), GetLocalMRPConfig(), exchangeCtxt, this);
SuccessOrExit(err);

exit:
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
.fabricTable = stateParams.fabricTable,
.clientPool = stateParams.caseClientPool,
.groupDataProvider = stateParams.groupDataProvider,
.mrpLocalConfig = Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()),
.mrpLocalConfig = GetLocalMRPConfig(),
};

CASESessionManagerConfig sessionManagerConfig = {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/address_resolve/AddressResolve.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ namespace AddressResolve {
struct ResolveResult
{
Transport::PeerAddress address;
ReliableMessageProtocolConfig mrpConfig;
ReliableMessageProtocolConfig mrpRemoteConfig;
bool supportsTcp = false;

ResolveResult() : address(Transport::Type::kUdp), mrpConfig(GetLocalMRPConfig()) {}
ResolveResult() : address(Transport::Type::kUdp), mrpRemoteConfig(GetDefaultMRPConfig()) {}
};

/// Represents an object interested in callbacks for a resolve operation.
Expand Down
4 changes: 2 additions & 2 deletions src/lib/address_resolve/AddressResolve_DefaultImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat

result.address.SetPort(nodeData.resolutionData.port);
result.address.SetInterface(nodeData.resolutionData.interfaceId);
result.mrpConfig = nodeData.resolutionData.GetMRPConfig();
result.supportsTcp = nodeData.resolutionData.supportsTcp;
result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
result.supportsTcp = nodeData.resolutionData.supportsTcp;

for (size_t i = 0; i < nodeData.resolutionData.numIPs; i++)
{
Expand Down
4 changes: 2 additions & 2 deletions src/lib/address_resolve/tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ class PrintOutNodeListener : public chip::AddressResolve::NodeListener

ChipLogProgress(Discovery, "Resolve completed: %s", addr_string);
ChipLogProgress(Discovery, " Supports TCP: %s", result.supportsTcp ? "YES" : "NO");
ChipLogProgress(Discovery, " MRP IDLE retransmit timeout: %u ms", result.mrpConfig.mIdleRetransTimeout.count());
ChipLogProgress(Discovery, " MRP ACTIVE retransmit timeout: %u ms", result.mrpConfig.mActiveRetransTimeout.count());
ChipLogProgress(Discovery, " MRP IDLE retransmit timeout: %u ms", result.mrpRemoteConfig.mIdleRetransTimeout.count());
ChipLogProgress(Discovery, " MRP ACTIVE retransmit timeout: %u ms", result.mrpRemoteConfig.mActiveRetransTimeout.count());
NotifyDone();
}

Expand Down
8 changes: 4 additions & 4 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ class BaseAdvertisingParams
const chip::ByteSpan GetMac() const { return chip::ByteSpan(mMacStorage, mMacLength); }

// Common Flags
Derived & SetMRPConfig(const ReliableMessageProtocolConfig & config)
Derived & SetLocalMRPConfig(const Optional<ReliableMessageProtocolConfig> & config)
{
mMRPConfig.SetValue(config);
mLocalMRPConfig = config;
return *reinterpret_cast<Derived *>(this);
}
const Optional<ReliableMessageProtocolConfig> & GetMRPConfig() const { return mMRPConfig; }
const Optional<ReliableMessageProtocolConfig> & GetLocalMRPConfig() const { return mLocalMRPConfig; }
Derived & SetTcpSupported(Optional<bool> tcpSupported)
{
mTcpSupported = tcpSupported;
Expand All @@ -107,7 +107,7 @@ class BaseAdvertisingParams
bool mEnableIPv4 = true;
uint8_t mMacStorage[kMaxMacSize] = {};
size_t mMacLength = 0;
Optional<ReliableMessageProtocolConfig> mMRPConfig;
Optional<ReliableMessageProtocolConfig> mLocalMRPConfig;
Optional<bool> mTcpSupported;
};

Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
CHIP_ERROR AddCommonTxtEntries(const BaseAdvertisingParams<Derived> & params, CommonTxtEntryStorage & storage,
char ** txtFields, size_t & numTxtFields)
{
auto optionalMrp = params.GetMRPConfig();
auto optionalMrp = params.GetLocalMRPConfig();

if (optionalMrp.HasValue())
{
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const
return CopyTextRecordValue(buffer, bufferLen, params.GetTcpSupported());
case TxtFieldKey::kSleepyIdleInterval:
case TxtFieldKey::kSleepyActiveInterval:
return CopyTextRecordValue(buffer, bufferLen, params.GetMRPConfig(), key == TxtFieldKey::kSleepyIdleInterval);
return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key == TxtFieldKey::kSleepyIdleInterval);
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ struct CommonResolutionData

bool IsValid() const { return !IsHost("") && (numIPs > 0) && (ipAddress[0] != chip::Inet::IPAddress::Any); }

ReliableMessageProtocolConfig GetMRPConfig() const
ReliableMessageProtocolConfig GetRemoteMRPConfig() const
{
const ReliableMessageProtocolConfig defaultConfig = GetLocalMRPConfig();
const ReliableMessageProtocolConfig defaultConfig = GetDefaultMRPConfig();
return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(defaultConfig.mIdleRetransTimeout),
GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout));
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ OperationalAdvertisingParameters operationalParams1 =
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetTcpSupported(chip::Optional<bool>(false))
.SetMRPConfig(ReliableMessageProtocolConfig(32_ms32, 30_ms32)); // Match SII, SAI below
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(32_ms32, 30_ms32)); // Match SII, SAI below
OperationalAdvertisingParameters operationalParams2 =
OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
OperationalAdvertisingParameters operationalParams3 =
Expand Down Expand Up @@ -180,7 +180,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
.SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"))
.SetTcpSupported(chip::Optional<bool>(true))
// 3600005 is more than the max so should be adjusted down
.SetMRPConfig(ReliableMessageProtocolConfig(3600000_ms32, 3600005_ms32));
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32));
QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000",
"DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3",
"SAI=3600000", "SII=3600000", "T=1" };
Expand Down
17 changes: 9 additions & 8 deletions src/lib/dnssd/platform/tests/TestPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ test::ExpectedCall operationalCall1 = test::ExpectedCall()
.SetInstanceName("BEEFBEEFF00DF00D-1111222233334444")
.SetHostName(host)
.AddSubtype("_IBEEFBEEFF00DF00D");
OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters()
.SetPeerId(kPeerId2)
.SetMac(ByteSpan(kMac))
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetMRPConfig({ 32_ms32, 30_ms32 }) // SII and SAI to match below
.SetTcpSupported(Optional<bool>(true));
OperationalAdvertisingParameters operationalParams2 =
OperationalAdvertisingParameters()
.SetPeerId(kPeerId2)
.SetMac(ByteSpan(kMac))
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(32_ms32, 30_ms32)) // SII and SAI to match below
.SetTcpSupported(Optional<bool>(true));
test::ExpectedCall operationalCall2 = test::ExpectedCall()
.SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp)
.SetServiceName("_matter")
Expand Down Expand Up @@ -95,7 +96,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic =
.SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"))
.SetTcpSupported(chip::Optional<bool>(true))
// 3600005 is over the max, so this should be adjusted by the platform
.SetMRPConfig({ 3600000_ms32, 3600005_ms32 });
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32));

test::ExpectedCall commissionableLargeBasic = test::ExpectedCall()
.SetProtocol(DnssdServiceProtocol::kDnssdProtocolUdp)
Expand Down
Loading

0 comments on commit 4124958

Please sign in to comment.