From 49365792808931316f5f7451ba3f84f725794c80 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Thu, 2 Dec 2021 15:20:06 +0800 Subject: [PATCH] Revisit MRP implementation (#11988) --- config/esp32/components/chip/Kconfig | 11 - .../chip-tool/commands/discover/Commands.h | 4 +- .../efr32/include/CHIPProjectConfig.h | 2 +- .../efr32/include/CHIPProjectConfig.h | 2 +- .../main/include/CHIPProjectConfig.h | 2 +- .../k32w/k32w0/include/CHIPProjectConfig.h | 4 +- .../main/include/CHIPProjectConfig.h | 4 +- .../main/include/CHIPProjectConfig.h | 4 +- .../efr32/include/CHIPProjectConfig.h | 2 +- src/app/OperationalDeviceProxy.cpp | 1 - src/controller/CommissioneeDeviceProxy.cpp | 1 - ...issionableNodeController-ScriptBinding.cpp | 4 +- .../ChipDeviceController-ScriptBinding.cpp | 4 +- src/include/platform/CHIPDeviceConfig.h | 4 +- src/include/platform/ConnectivityManager.h | 14 +- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 23 +- src/lib/dnssd/Discovery_ImplPlatform.cpp | 23 +- src/lib/dnssd/Resolver.h | 24 +- src/lib/dnssd/TxtFields.cpp | 6 +- src/lib/dnssd/TxtFields.h | 11 +- .../minimal_mdns/tests/TestAdvertiser.cpp | 21 +- src/lib/dnssd/platform/tests/TestPlatform.cpp | 23 +- src/lib/dnssd/tests/TestTxtFields.cpp | 8 +- src/messaging/ExchangeContext.cpp | 3 +- src/messaging/ExchangeMgr.cpp | 6 +- src/messaging/ReliableMessageContext.cpp | 40 +-- src/messaging/ReliableMessageContext.h | 21 +- src/messaging/ReliableMessageMgr.cpp | 259 +++--------------- src/messaging/ReliableMessageMgr.h | 79 +----- .../ReliableMessageProtocolConfig.cpp | 5 +- src/messaging/ReliableMessageProtocolConfig.h | 38 +-- .../tests/TestReliableMessageProtocol.cpp | 159 +++++------ src/platform/Darwin/CHIPPlatformConfig.h | 6 +- src/platform/EFR32/CHIPPlatformConfig.h | 4 - src/platform/ESP32/CHIPPlatformConfig.h | 1 - src/platform/Linux/CHIPPlatformConfig.h | 4 - ...nericThreadStackManagerImpl_OpenThread.cpp | 18 +- src/platform/Tizen/CHIPPlatformConfig.h | 4 - src/platform/android/CHIPPlatformConfig.h | 4 - src/platform/cc13x2_26x2/CHIPPlatformConfig.h | 4 - src/platform/mbed/CHIPPlatformConfig.h | 4 - src/platform/nrfconnect/CHIPPlatformConfig.h | 4 - .../nxp/k32w/k32w0/CHIPPlatformConfig.h | 4 - src/platform/telink/CHIPPlatformConfig.h | 4 - .../secure_channel/tests/TestPASESession.cpp | 39 +-- 45 files changed, 256 insertions(+), 656 deletions(-) diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index 3ab41d517b8a50..3e59822a63553b 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -67,17 +67,6 @@ menu "CHIP Core" message handlers with the CHIP message layer to direct incoming messages to their code. - config RMP_TIMER_DEFAULT_PERIOD_SHIFT - int "Default WRMP Timer Tick Interval Shift (ms)" - range 0 16 - default 6 - help - The default interval shift, in milliseconds (e.g. 6 bits shift - = 64ms), at which items in the WRMP pending message list are - processed for the purpose of retransmission or timeout. - - This value can be overridden by the application at runtime. - config ENABLE_PW_RPC bool "Enable Pigweed RPC library" default n diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h index c68ae26615b4ee..cbcdf2e27bd938 100644 --- a/examples/chip-tool/commands/discover/Commands.h +++ b/examples/chip-tool/commands/discover/Commands.h @@ -55,12 +55,12 @@ class Resolve : public DiscoverCommand, public chip::Dnssd::ResolverDelegate auto retryInterval = nodeData.GetMrpRetryIntervalIdle(); if (retryInterval.HasValue()) - ChipLogProgress(chipTool, " MRP retry interval (idle): %" PRIu32 "ms", retryInterval.Value()); + ChipLogProgress(chipTool, " MRP retry interval (idle): %" PRIu32 "ms", retryInterval.Value().count()); retryInterval = nodeData.GetMrpRetryIntervalActive(); if (retryInterval.HasValue()) - ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms", retryInterval.Value()); + ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms", retryInterval.Value().count()); ChipLogProgress(chipTool, " Supports TCP: %s", nodeData.mSupportsTcp ? "yes" : "no"); SetCommandExitStatus(CHIP_NO_ERROR); diff --git a/examples/lighting-app/efr32/include/CHIPProjectConfig.h b/examples/lighting-app/efr32/include/CHIPProjectConfig.h index 230adaefeae985..e2bfb2d63d60a3 100644 --- a/examples/lighting-app/efr32/include/CHIPProjectConfig.h +++ b/examples/lighting-app/efr32/include/CHIPProjectConfig.h @@ -128,4 +128,4 @@ * needs (e.g. sleeping period) using Service Discovery TXT record CRA key. * */ -#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000) +#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32) diff --git a/examples/lock-app/efr32/include/CHIPProjectConfig.h b/examples/lock-app/efr32/include/CHIPProjectConfig.h index 4fdc0865b0e22c..0c9c9295d0577c 100644 --- a/examples/lock-app/efr32/include/CHIPProjectConfig.h +++ b/examples/lock-app/efr32/include/CHIPProjectConfig.h @@ -125,4 +125,4 @@ * needs (e.g. sleeping period) using Service Discovery TXT record CRA key. * */ -#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000) +#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32) diff --git a/examples/lock-app/nrfconnect/main/include/CHIPProjectConfig.h b/examples/lock-app/nrfconnect/main/include/CHIPProjectConfig.h index 46afec16824f5d..8c7fa93baa72ac 100644 --- a/examples/lock-app/nrfconnect/main/include/CHIPProjectConfig.h +++ b/examples/lock-app/nrfconnect/main/include/CHIPProjectConfig.h @@ -38,4 +38,4 @@ #define CHIP_DEVICE_CONFIG_USE_TEST_SETUP_PIN_CODE 20202021 #define CHIP_DEVICE_CONFIG_USE_TEST_SETUP_DISCRIMINATOR 0xF00 -#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 2000 +#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 2000_ms32 diff --git a/examples/lock-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h b/examples/lock-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h index 620662f80ca98a..8da35444dca534 100644 --- a/examples/lock-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h +++ b/examples/lock-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h @@ -185,8 +185,8 @@ #define CHIP_CONFIG_MAX_DEVICE_ADMINS 4 // 3 fabrics + 1 for rotation slack #define CHIP_DEVICE_CONFIG_ENABLE_SED 1 -#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 1000 -#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 100 +#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 1000_ms32 +#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 100_ms32 /** * CHIP_CONFIG_EVENT_LOGGING_DEFAULT_IMPORTANCE diff --git a/examples/pump-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h b/examples/pump-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h index 074e0401ab222f..6620df3ce102ea 100644 --- a/examples/pump-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h +++ b/examples/pump-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h @@ -124,8 +124,8 @@ #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 1 #define CHIP_DEVICE_CONFIG_ENABLE_SED 1 -#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000 -#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 5000 +#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000_ms32 +#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 5000_ms32 /** * CHIP_CONFIG_EVENT_LOGGING_DEFAULT_IMPORTANCE diff --git a/examples/pump-controller-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h b/examples/pump-controller-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h index 074e0401ab222f..6620df3ce102ea 100644 --- a/examples/pump-controller-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h +++ b/examples/pump-controller-app/cc13x2x7_26x2x7/main/include/CHIPProjectConfig.h @@ -124,8 +124,8 @@ #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 1 #define CHIP_DEVICE_CONFIG_ENABLE_SED 1 -#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000 -#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 5000 +#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000_ms32 +#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 5000_ms32 /** * CHIP_CONFIG_EVENT_LOGGING_DEFAULT_IMPORTANCE diff --git a/examples/window-app/efr32/include/CHIPProjectConfig.h b/examples/window-app/efr32/include/CHIPProjectConfig.h index 68afab33fad73e..6902c38d625b0e 100644 --- a/examples/window-app/efr32/include/CHIPProjectConfig.h +++ b/examples/window-app/efr32/include/CHIPProjectConfig.h @@ -165,4 +165,4 @@ * needs (e.g. sleeping period) using Service Discovery TXT record CRA key. * */ -#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000) +#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32) diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalDeviceProxy.cpp index 015a0a95233694..b6b6a46cc89732 100644 --- a/src/app/OperationalDeviceProxy.cpp +++ b/src/app/OperationalDeviceProxy.cpp @@ -120,7 +120,6 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress if (secureSession != nullptr) { secureSession->SetPeerAddress(addr); - secureSession->SetMRPConfig(mMRPConfig); } } diff --git a/src/controller/CommissioneeDeviceProxy.cpp b/src/controller/CommissioneeDeviceProxy.cpp index 89cb5c08322772..84128527e6d9d3 100644 --- a/src/controller/CommissioneeDeviceProxy.cpp +++ b/src/controller/CommissioneeDeviceProxy.cpp @@ -134,7 +134,6 @@ CHIP_ERROR CommissioneeDeviceProxy::UpdateDeviceData(const Transport::PeerAddres Transport::SecureSession * secureSession = mSessionManager->GetSecureSession(mSecureSession.Value()); secureSession->SetPeerAddress(addr); - secureSession->SetMRPConfig(config); return CHIP_NO_ERROR; } diff --git a/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp b/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp index 5c2b76296b7e61..c32eab8abc6aa1 100644 --- a/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp +++ b/src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp @@ -100,7 +100,7 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners( ChipLogProgress(Discovery, "\tPairing Hint\t\t%u", dnsSdInfo->pairingHint); if (dnsSdInfo->GetMrpRetryIntervalIdle().HasValue()) { - ChipLogProgress(Discovery, "\tMrp Interval idle\t%u", dnsSdInfo->GetMrpRetryIntervalIdle().Value()); + ChipLogProgress(Discovery, "\tMrp Interval idle\t%u", dnsSdInfo->GetMrpRetryIntervalIdle().Value().count()); } else { @@ -108,7 +108,7 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners( } if (dnsSdInfo->GetMrpRetryIntervalActive().HasValue()) { - ChipLogProgress(Discovery, "\tMrp Interval active\t%u", dnsSdInfo->GetMrpRetryIntervalActive().Value()); + ChipLogProgress(Discovery, "\tMrp Interval active\t%u", dnsSdInfo->GetMrpRetryIntervalActive().Value().count()); } else { diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp index 35e85bc3b8a055..789e5855d92289 100644 --- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp +++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp @@ -425,7 +425,7 @@ void pychip_DeviceController_PrintDiscoveredDevices(chip::Controller::DeviceComm ChipLogProgress(Discovery, "\tPairing Hint\t\t%u", dnsSdInfo->pairingHint); if (dnsSdInfo->GetMrpRetryIntervalIdle().HasValue()) { - ChipLogProgress(Discovery, "\tMrp Interval idle\t%u", dnsSdInfo->GetMrpRetryIntervalIdle().Value()); + ChipLogProgress(Discovery, "\tMrp Interval idle\t%u", dnsSdInfo->GetMrpRetryIntervalIdle().Value().count()); } else { @@ -433,7 +433,7 @@ void pychip_DeviceController_PrintDiscoveredDevices(chip::Controller::DeviceComm } if (dnsSdInfo->GetMrpRetryIntervalActive().HasValue()) { - ChipLogProgress(Discovery, "\tMrp Interval active\t%u", dnsSdInfo->GetMrpRetryIntervalActive().Value()); + ChipLogProgress(Discovery, "\tMrp Interval active\t%u", dnsSdInfo->GetMrpRetryIntervalActive().Value().count()); } else { diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index 318f634a2da4d2..b7e40bd6dec042 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/src/include/platform/CHIPDeviceConfig.h @@ -132,7 +132,7 @@ * This interval is used by the device to periodically wake up and poll the data in the idle mode. */ #ifndef CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL -#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000 +#define CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL 5000_ms32 #endif /** @@ -142,7 +142,7 @@ * This interval is used by the device to periodically wake up and poll the data in the active mode. */ #ifndef CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL -#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 200 +#define CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL 200_ms32 #endif // -------------------- Device Identification Configuration -------------------- diff --git a/src/include/platform/ConnectivityManager.h b/src/include/platform/ConnectivityManager.h index 26056f71dc1589..694d064da9720c 100644 --- a/src/include/platform/ConnectivityManager.h +++ b/src/include/platform/ConnectivityManager.h @@ -275,15 +275,13 @@ class ConnectivityManager */ struct ConnectivityManager::SEDPollingConfig { - uint32_t FastPollingIntervalMS; /**< Interval at which the device polls its parent - when there are active chip exchanges in progress. Only meaningful - when the device is acting as a sleepy end node. */ + /** Interval at which the device polls its parent when there are active chip exchanges in progress. Only meaningful when the + * device is acting as a sleepy end node. */ + System::Clock::Milliseconds32 FastPollingIntervalMS; - uint32_t SlowPollingIntervalMS; /**< Interval at which the device polls its parent - when there are NO active chip exchanges in progress. Only meaningful - when the device is acting as a sleepy end node. */ - - void Clear() { memset(this, 0, sizeof(*this)); } + /** Interval at which the device polls its parent when there are NO active chip exchanges in progress. Only meaningful when the + * device is acting as a sleepy end node. */ + System::Clock::Milliseconds32 SlowPollingIntervalMS; }; /** diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 4925bee60e33c1..a06ebe26559ffa 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -161,33 +161,28 @@ class AdvertiserMinMdns : public ServiceAdvertiser, // TODO: Issue #5833 - MRP retry intervals should be updated on the poll period value change or device type change. #if CHIP_DEVICE_CONFIG_ENABLE_SED chip::DeviceLayer::ConnectivityManager::SEDPollingConfig sedPollingConfig; - sedPollingConfig.Clear(); ReturnErrorOnFailure(chip::DeviceLayer::ConnectivityMgr().GetSEDPollingConfig(sedPollingConfig)); // Increment default MRP retry intervals by SED poll period to be on the safe side // and avoid unnecessary retransmissions. if (optionalMrp.HasValue()) { auto mrp = optionalMrp.Value(); - optionalMrp.SetValue(ReliableMessageProtocolConfig( - mrp.mIdleRetransTimeoutTick + - (sedPollingConfig.SlowPollingIntervalMS >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT), - mrp.mActiveRetransTimeoutTick + - (sedPollingConfig.FastPollingIntervalMS >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT))); + optionalMrp.SetValue(ReliableMessageProtocolConfig(mrp.mIdleRetransTimeout + sedPollingConfig.SlowPollingIntervalMS, + mrp.mActiveRetransTimeout + sedPollingConfig.FastPollingIntervalMS)); } #endif if (optionalMrp.HasValue()) { auto mrp = optionalMrp.Value(); { - if ((mrp.mIdleRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) > kMaxRetryInterval) + if (mrp.mIdleRetransTimeout > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval idle value exceeds allowed range of 1 hour, using maximum available"); - mrp.mIdleRetransTimeoutTick = kMaxRetryInterval >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT; + mrp.mIdleRetransTimeout = kMaxRetryInterval; } - size_t writtenCharactersNumber = - snprintf(storage.mrpRetryIntervalIdleBuf, sizeof(storage.mrpRetryIntervalIdleBuf), "CRI=%" PRIu32, - mrp.mIdleRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); + size_t writtenCharactersNumber = snprintf(storage.mrpRetryIntervalIdleBuf, sizeof(storage.mrpRetryIntervalIdleBuf), + "CRI=%" PRIu32, mrp.mIdleRetransTimeout.count()); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.mrpRetryIntervalIdleBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); @@ -196,15 +191,15 @@ class AdvertiserMinMdns : public ServiceAdvertiser, } { - if ((mrp.mActiveRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) > kMaxRetryInterval) + if (mrp.mActiveRetransTimeout > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval active value exceeds allowed range of 1 hour, using maximum available"); - mrp.mActiveRetransTimeoutTick = kMaxRetryInterval >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT; + mrp.mActiveRetransTimeout = kMaxRetryInterval; } size_t writtenCharactersNumber = snprintf(storage.mrpRetryIntervalActiveBuf, sizeof(storage.mrpRetryIntervalActiveBuf), "CRA=%" PRIu32, - mrp.mActiveRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); + mrp.mActiveRetransTimeout.count()); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.mrpRetryIntervalActiveBuf)), CHIP_ERROR_INVALID_STRING_LENGTH); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 109bbc69abbb39..0f564488f1d126 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -136,45 +136,42 @@ CHIP_ERROR AddCommonTxtElements(const BaseAdvertisingParams & params, c // in with the correct values and set one level up from here. #if CHIP_DEVICE_CONFIG_ENABLE_SED chip::DeviceLayer::ConnectivityManager::SEDPollingConfig sedPollingConfig; - sedPollingConfig.Clear(); ReturnErrorOnFailure(chip::DeviceLayer::ConnectivityMgr().GetSEDPollingConfig(sedPollingConfig)); // Increment default MRP retry intervals by SED poll period to be on the safe side // and avoid unnecessary retransmissions. if (optionalMrp.HasValue()) { auto mrp = optionalMrp.Value(); - optionalMrp.SetValue(ReliableMessageProtocolConfig( - mrp.mIdleRetransTimeoutTick + (sedPollingConfig.SlowPollingIntervalMS >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT), - mrp.mActiveRetransTimeoutTick + - (sedPollingConfig.FastPollingIntervalMS >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT))); + optionalMrp.SetValue(ReliableMessageProtocolConfig(mrp.mIdleRetransTimeout + sedPollingConfig.SlowPollingIntervalMS, + mrp.mActiveRetransTimeout + sedPollingConfig.FastPollingIntervalMS)); } #endif if (optionalMrp.HasValue()) { auto mrp = optionalMrp.Value(); { - if ((mrp.mIdleRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) > kMaxRetryInterval) + if (mrp.mIdleRetransTimeout > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval idle value exceeds allowed range of 1 hour, using maximum available"); - mrp.mIdleRetransTimeoutTick = kMaxRetryInterval >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT; + mrp.mIdleRetransTimeout = kMaxRetryInterval; } - size_t writtenCharactersNumber = snprintf(mrpRetryIdleStorage, sizeof(mrpRetryIdleStorage), "%" PRIu32, - mrp.mIdleRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); + size_t writtenCharactersNumber = + snprintf(mrpRetryIdleStorage, sizeof(mrpRetryIdleStorage), "%" PRIu32, mrp.mIdleRetransTimeout.count()); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber <= kTxtRetryIntervalIdleMaxLength), CHIP_ERROR_INVALID_STRING_LENGTH); txtEntryStorage[txtEntryIdx++] = { "CRI", Uint8::from_const_char(mrpRetryIdleStorage), strlen(mrpRetryIdleStorage) }; } { - if ((mrp.mActiveRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) > kMaxRetryInterval) + if (mrp.mActiveRetransTimeout > kMaxRetryInterval) { ChipLogProgress(Discovery, "MRP retry interval active value exceeds allowed range of 1 hour, using maximum available"); - mrp.mActiveRetransTimeoutTick = kMaxRetryInterval >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT; + mrp.mActiveRetransTimeout = kMaxRetryInterval; } - size_t writtenCharactersNumber = snprintf(mrpRetryActiveStorage, sizeof(mrpRetryActiveStorage), "%" PRIu32, - mrp.mActiveRetransTimeoutTick << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); + size_t writtenCharactersNumber = + snprintf(mrpRetryActiveStorage, sizeof(mrpRetryActiveStorage), "%" PRIu32, mrp.mActiveRetransTimeout.count()); VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber <= kTxtRetryIntervalActiveMaxLength), CHIP_ERROR_INVALID_STRING_LENGTH); txtEntryStorage[txtEntryIdx++] = { "CRA", Uint8::from_const_char(mrpRetryActiveStorage), diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 6512ada84f34f0..f4ef2ac059c483 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -56,15 +56,11 @@ struct ResolvedNodeData ReliableMessageProtocolConfig GetMRPConfig() const { - return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(gDefaultMRPConfig.mIdleRetransTimeoutTick - << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) >> - CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - GetMrpRetryIntervalActive().ValueOr(gDefaultMRPConfig.mActiveRetransTimeoutTick - << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) >> - CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); + return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(gDefaultMRPConfig.mIdleRetransTimeout), + GetMrpRetryIntervalActive().ValueOr(gDefaultMRPConfig.mActiveRetransTimeout)); } - Optional GetMrpRetryIntervalIdle() const { return mMrpRetryIntervalIdle; } - Optional GetMrpRetryIntervalActive() const { return mMrpRetryIntervalActive; } + Optional GetMrpRetryIntervalIdle() const { return mMrpRetryIntervalIdle; } + Optional GetMrpRetryIntervalActive() const { return mMrpRetryIntervalActive; } PeerId mPeerId; size_t mNumIPs = 0; @@ -73,8 +69,8 @@ struct ResolvedNodeData uint16_t mPort = 0; char mHostName[kHostNameMaxLength + 1] = {}; bool mSupportsTcp = false; - Optional mMrpRetryIntervalIdle; - Optional mMrpRetryIntervalActive; + Optional mMrpRetryIntervalIdle; + Optional mMrpRetryIntervalActive; System::Clock::Timestamp mExpiryTime; }; @@ -100,8 +96,8 @@ struct DiscoveredNodeData uint16_t pairingHint; char pairingInstruction[kMaxPairingInstructionLen + 1]; bool supportsTcp; - Optional mrpRetryIntervalIdle; - Optional mrpRetryIntervalActive; + Optional mrpRetryIntervalIdle; + Optional mrpRetryIntervalActive; uint16_t port; int numIPs; Inet::InterfaceId interfaceId[kMaxIPAddresses]; @@ -135,8 +131,8 @@ struct DiscoveredNodeData bool IsHost(const char * host) const { return strcmp(host, hostName) == 0; } bool IsInstanceName(const char * instance) const { return strcmp(instance, instanceName) == 0; } bool IsValid() const { return !IsHost("") && ipAddress[0] != chip::Inet::IPAddress::Any; } - Optional GetMrpRetryIntervalIdle() const { return mrpRetryIntervalIdle; } - Optional GetMrpRetryIntervalActive() const { return mrpRetryIntervalActive; } + Optional GetMrpRetryIntervalIdle() const { return mrpRetryIntervalIdle; } + Optional GetMrpRetryIntervalActive() const { return mrpRetryIntervalActive; } void LogDetail() const { diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp index 3cf23a2d300c62..0e13ff92ab606b 100644 --- a/src/lib/dnssd/TxtFields.cpp +++ b/src/lib/dnssd/TxtFields.cpp @@ -171,13 +171,13 @@ void GetPairingInstruction(const ByteSpan & value, char * pairingInstruction) Platform::CopyString(pairingInstruction, kMaxPairingInstructionLen + 1, value); } -Optional GetRetryInterval(const ByteSpan & value) +Optional GetRetryInterval(const ByteSpan & value) { const auto undefined = std::numeric_limits::max(); const auto retryInterval = MakeU32FromAsciiDecimal(value, undefined); - if (retryInterval != undefined && retryInterval <= kMaxRetryInterval) - return MakeOptional(retryInterval); + if (retryInterval != undefined && retryInterval <= kMaxRetryInterval.count()) + return MakeOptional(System::Clock::Milliseconds32(retryInterval)); return NullOptional; } diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h index dbee250e486348..18accfcfff0dce 100644 --- a/src/lib/dnssd/TxtFields.h +++ b/src/lib/dnssd/TxtFields.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -27,11 +28,13 @@ namespace chip { namespace Dnssd { +using namespace System::Clock::Literals; + // Operational node TXT entries -static constexpr size_t kTxtRetryIntervalIdleMaxLength = 7; // [CRI] 0-3600000 -static constexpr size_t kTxtRetryIntervalActiveMaxLength = 7; // [CRA] 0-3600000 -static constexpr size_t kMaxRetryInterval = 3600000; -static constexpr size_t kKeyTcpSupportMaxLength = 1; +static constexpr size_t kTxtRetryIntervalIdleMaxLength = 7; // [CRI] 0-3600000 +static constexpr size_t kTxtRetryIntervalActiveMaxLength = 7; // [CRA] 0-3600000 +static constexpr System::Clock::Milliseconds32 kMaxRetryInterval = 3600000_ms32; +static constexpr size_t kKeyTcpSupportMaxLength = 1; // Commissionable/commissioner node TXT entries static constexpr size_t kKeyDiscriminatorMaxLength = 5; diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp index efc12cee33f1ca..5cde77f9f90d4b 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp @@ -71,15 +71,13 @@ FullQName kCompressedIdSubName2 = FullQName(kCompressedIdSubPart PtrResourceRecord ptrServiceSubCompressedId1 = PtrResourceRecord(kDnsSdQueryName, kCompressedIdSubName1); PtrResourceRecord ptrServiceSubCompressedId2 = PtrResourceRecord(kDnsSdQueryName, kCompressedIdSubName2); -OperationalAdvertisingParameters operationalParams1 = - OperationalAdvertisingParameters() - .SetPeerId(kPeerId1) - .SetMac(ByteSpan(kMac)) - .SetPort(CHIP_PORT) - .EnableIpV4(true) - .SetTcpSupported(chip::Optional(false)) - .SetMRPConfig(ReliableMessageProtocolConfig(64 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - 128 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT)); +OperationalAdvertisingParameters operationalParams1 = OperationalAdvertisingParameters() + .SetPeerId(kPeerId1) + .SetMac(ByteSpan(kMac)) + .SetPort(CHIP_PORT) + .EnableIpV4(true) + .SetTcpSupported(chip::Optional(false)) + .SetMRPConfig(ReliableMessageProtocolConfig(32_ms32, 33_ms32)); OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams3 = @@ -90,7 +88,7 @@ OperationalAdvertisingParameters operationalParams5 = OperationalAdvertisingParameters().SetPeerId(kPeerId5).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); OperationalAdvertisingParameters operationalParams6 = OperationalAdvertisingParameters().SetPeerId(kPeerId6).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true); -const QNamePart txtOperational1Parts[] = { "CRI=64", "CRA=128", "T=0" }; +const QNamePart txtOperational1Parts[] = { "CRI=32", "CRA=33", "T=0" }; PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName); PtrResourceRecord ptrOperational1 = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1); SrvResourceRecord srvOperational1 = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT); @@ -180,8 +178,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced = .SetRotatingId(chip::Optional("id_that_spins")) .SetTcpSupported(chip::Optional(true)) // 3600005 is more than the max so should be adjusted down - .SetMRPConfig(ReliableMessageProtocolConfig(3600000 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - 3600064 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT)); + .SetMRPConfig(ReliableMessageProtocolConfig(3600000_ms32, 3600005_ms32)); QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=25", "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3", "CRA=3600000", "CRI=3600000", "T=1" }; diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp index acbde364d97ae5..1e7ea27b1e23e9 100644 --- a/src/lib/dnssd/platform/tests/TestPlatform.cpp +++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp @@ -46,23 +46,21 @@ 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(ReliableMessageProtocolConfig(64 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - 128 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT)) - .SetTcpSupported(Optional(true)); +OperationalAdvertisingParameters operationalParams2 = OperationalAdvertisingParameters() + .SetPeerId(kPeerId2) + .SetMac(ByteSpan(kMac)) + .SetPort(CHIP_PORT) + .EnableIpV4(true) + .SetMRPConfig({ 32_ms32, 33_ms32 }) + .SetTcpSupported(Optional(true)); test::ExpectedCall operationalCall2 = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp) .SetServiceName("_matter") .SetInstanceName("5555666677778888-1212343456567878") .SetHostName(host) .AddSubtype("_I5555666677778888") - .AddTxt("CRI", "64") - .AddTxt("CRA", "128") + .AddTxt("CRI", "32") + .AddTxt("CRA", "33") .AddTxt("T", "1"); CommissionAdvertisingParameters commissionableNodeParamsSmall = @@ -97,8 +95,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic = .SetRotatingId(chip::Optional("id_that_spins")) .SetTcpSupported(chip::Optional(true)) // 3600005 is over the max, so this should be adjusted by the platform - .SetMRPConfig(ReliableMessageProtocolConfig(3600000 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - 3600064 >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT)); + .SetMRPConfig({ 3600000_ms32, 3600005_ms32 }); test::ExpectedCall commissionableLargeBasic = test::ExpectedCall() .SetProtocol(DnssdServiceProtocol::kDnssdProtocolUdp) diff --git a/src/lib/dnssd/tests/TestTxtFields.cpp b/src/lib/dnssd/tests/TestTxtFields.cpp index aa3a28d9b78b3d..0745132543c4bc 100644 --- a/src/lib/dnssd/tests/TestTxtFields.cpp +++ b/src/lib/dnssd/tests/TestTxtFields.cpp @@ -416,14 +416,14 @@ void TxtFieldMrpRetryIntervalIdle(nlTestSuite * inSuite, void * inContext) sprintf(val, "1"); FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData); NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().HasValue()); - NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().Value() == 1); + NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().Value() == 1_ms32); // Maximum sprintf(key, "CRI"); sprintf(val, "3600000"); FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData); NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().HasValue()); - NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().Value() == 3600000); + NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalIdle().Value() == 3600000_ms32); // Test no other fields were populated ResetRetryIntervalIdle(nodeData); @@ -479,14 +479,14 @@ void TxtFieldMrpRetryIntervalActive(nlTestSuite * inSuite, void * inContext) sprintf(val, "1"); FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData); NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().HasValue()); - NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().Value() == 1); + NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().Value() == 1_ms32); // Maximum sprintf(key, "CRA"); sprintf(val, "3600000"); FillNodeDataFromTxt(GetSpan(key), GetSpan(val), nodeData); NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().HasValue()); - NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().Value() == 3600000); + NL_TEST_ASSERT(inSuite, nodeData.GetMrpRetryIntervalActive().Value() == 3600000_ms32); // Test no other fields were populated ResetRetryIntervalActive(nodeData); diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index de7760bd8c813e..0b98068018305c 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -529,8 +529,7 @@ System::Clock::Milliseconds32 ExchangeContext::GetAckTimeout() System::Clock::Timeout timeout; if (IsUDPTransport()) { - timeout = System::Clock::Milliseconds32((CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1) * - (GetIdleRetransmitTimeoutTick() << CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT)); + timeout = GetMRPConfig().mIdleRetransTimeout * (CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS + 1); } else if (IsTCPTransport()) { diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 7ccb0ce7b0db8e..eb3bc0a0d305f6 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -86,7 +86,7 @@ CHIP_ERROR ExchangeManager::Init(SessionManager * sessionManager) sessionManager->RegisterReleaseDelegate(*this); sessionManager->SetMessageDelegate(this); - mReliableMessageMgr.Init(sessionManager->SystemLayer(), sessionManager); + mReliableMessageMgr.Init(sessionManager->SystemLayer()); ReturnErrorOnFailure(mDefaultExchangeDispatch.Init(mSessionManager)); mState = State::kState_Initialized; @@ -118,9 +118,7 @@ CHIP_ERROR ExchangeManager::Shutdown() ExchangeContext * ExchangeManager::NewContext(SessionHandle session, ExchangeDelegate * delegate) { - ExchangeContext * context = mContextPool.CreateObject(this, mNextExchangeId++, session, true, delegate); - context->GetReliableMessageContext()->SetConfig(session.GetMRPConfig(GetSessionManager())); - return context; + return mContextPool.CreateObject(this, mNextExchangeId++, session, true, delegate); } CHIP_ERROR ExchangeManager::RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId, ExchangeDelegate * delegate) diff --git a/src/messaging/ReliableMessageContext.cpp b/src/messaging/ReliableMessageContext.cpp index baabb4acc4ef20..62f1bd75531975 100644 --- a/src/messaging/ReliableMessageContext.cpp +++ b/src/messaging/ReliableMessageContext.cpp @@ -39,8 +39,7 @@ namespace chip { namespace Messaging { -ReliableMessageContext::ReliableMessageContext() : mConfig(gDefaultMRPConfig), mNextAckTimeTick(0), mPendingPeerAckMessageCounter(0) -{} +ReliableMessageContext::ReliableMessageContext() : mNextAckTime(0), mPendingPeerAckMessageCounter(0) {} bool ReliableMessageContext::AutoRequestAck() const { @@ -108,17 +107,14 @@ CHIP_ERROR ReliableMessageContext::FlushAcks() if (IsAckPending()) { - // Send the acknowledgment as a SecureChannel::StandStandaloneAck message err = SendStandaloneAckMessage(); if (err == CHIP_NO_ERROR) { -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Flushed pending ack for MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange, mPendingPeerAckMessageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif } } @@ -130,16 +126,6 @@ bool ReliableMessageContext::HasPiggybackAckPending() const return mFlags.Has(Flags::kFlagAckMessageCounterIsValid); } -uint64_t ReliableMessageContext::GetIdleRetransmitTimeoutTick() -{ - return mConfig.mIdleRetransTimeoutTick; -} - -uint64_t ReliableMessageContext::GetActiveRetransmitTimeoutTick() -{ - return mConfig.mActiveRetransTimeoutTick; -} - /** * Process received Ack. Remove the corresponding message context from the RetransTable and execute the application * callback @@ -156,20 +142,16 @@ void ReliableMessageContext::HandleRcvdAck(uint32_t ackMessageCounter) { // This can happen quite easily due to a packet with a piggyback ack // being lost and retransmitted. -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "CHIP MessageCounter:" ChipLogFormatMessageCounter " not in RetransTable on exchange " ChipLogFormatExchange, ackMessageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif } else { -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Removed CHIP MessageCounter:" ChipLogFormatMessageCounter " from RetransTable on exchange " ChipLogFormatExchange, ackMessageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif } } @@ -180,9 +162,6 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAck(uint32_t messageCounter, BitFl if (ShouldDropAckDebug()) return CHIP_NO_ERROR; - // Expire any virtual ticks that have expired so all wakeup sources reflect the current time - GetReliableMessageMgr()->ExpireTicks(); - CHIP_ERROR err = HandleNeedsAckInner(messageCounter, messageFlags); // Schedule next physical wakeup on function exit @@ -198,13 +177,11 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageCounter, // should not wait for one and just immediately send a standalone ack. if (messageFlags.Has(MessageFlagValues::kDuplicateMessage)) { -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Forcing tx of solitary ack for duplicate MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange, messageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif - // Is there pending ack for a different message counter. + bool wasAckPending = IsAckPending() && mPendingPeerAckMessageCounter != messageCounter; bool messageCounterWasValid = HasPiggybackAckPending(); @@ -212,13 +189,9 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageCounter, // Temporary store currently pending ack message counter (even if there is none). uint32_t tempAckMessageCounter = mPendingPeerAckMessageCounter; - // Set the pending ack message counter. SetPendingPeerAckMessageCounter(messageCounter); - - // Send the Ack for the duplication message in a SecureChannel::StandaloneAck message. CHIP_ERROR err = SendStandaloneAckMessage(); - // If there was pending ack for a different message counter. if (wasAckPending) { // Restore previously pending ack message counter. @@ -242,21 +215,18 @@ CHIP_ERROR ReliableMessageContext::HandleNeedsAckInner(uint32_t messageCounter, { if (IsAckPending()) { -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Pending ack queue full; forcing tx of solitary ack for MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange, mPendingPeerAckMessageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif // Send the Ack for the currently pending Ack in a SecureChannel::StandaloneAck message. ReturnErrorOnFailure(SendStandaloneAckMessage()); } // Replace the Pending ack message counter. SetPendingPeerAckMessageCounter(messageCounter); - mNextAckTimeTick = static_cast( - CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK + - GetReliableMessageMgr()->GetTickCounterFromTimeDelta(System::SystemClock().GetMonotonicTimestamp())); + using namespace System::Clock::Literals; + mNextAckTime = System::SystemClock().GetMonotonicTimestamp() + CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT; return CHIP_NO_ERROR; } } @@ -271,11 +241,9 @@ CHIP_ERROR ReliableMessageContext::SendStandaloneAckMessage() } // Send the null message -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Sending Standalone Ack for MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange, mPendingPeerAckMessageCounter, ChipLogValueExchange(GetExchangeContext())); -#endif CHIP_ERROR err = GetExchangeContext()->SendMessage(Protocols::SecureChannel::MsgType::StandaloneAck, std::move(msgBuf), BitFlags{ SendMessageFlags::kNoAutoRequestAck }); diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 2869724d1d1f6a..b0999fec69d6d2 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -47,8 +47,6 @@ class ReliableMessageContext public: ReliableMessageContext(); - void SetConfig(ReliableMessageProtocolConfig config) { mConfig = config; } - /** * Flush the pending Ack for current exchange. * @@ -74,22 +72,6 @@ class ReliableMessageContext */ bool HasPiggybackAckPending() const; - /** - * Get the idle retransmission interval. It would be the time to wait before - * retransmission after first failure. - * - * @return the idle retransmission interval. - */ - uint64_t GetIdleRetransmitTimeoutTick(); - - /** - * Get the active retransmit interval. It would be the time to wait before - * retransmission after subsequent failures. - * - * @return the active retransmission interval. - */ - uint64_t GetActiveRetransmitTimeoutTick(); - /** * Send a SecureChannel::StandaloneAck message. * @@ -241,8 +223,7 @@ class ReliableMessageContext friend class ExchangeContext; friend class ExchangeMessageDispatch; - ReliableMessageProtocolConfig mConfig; - uint16_t mNextAckTimeTick; // Next time for triggering Solo Ack + System::Clock::Timestamp mNextAckTime; // Next time for triggering Solo Ack uint32_t mPendingPeerAckMessageCounter; }; diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index 8594d080bdadda..675f9bda754ca1 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -39,7 +39,7 @@ namespace chip { namespace Messaging { ReliableMessageMgr::RetransTableEntry::RetransTableEntry(ReliableMessageContext * rc) : - ec(*rc->GetExchangeContext()), retainedBuf(EncryptedPacketBufferHandle()), nextRetransTimeTick(0), sendCount(0) + ec(*rc->GetExchangeContext()), retainedBuf(EncryptedPacketBufferHandle()), nextRetransTime(0), sendCount(0) { ec->SetMessageNotAcked(true); } @@ -50,17 +50,14 @@ ReliableMessageMgr::RetransTableEntry::~RetransTableEntry() } ReliableMessageMgr::ReliableMessageMgr(BitMapObjectPool & contextPool) : - mContextPool(contextPool), mSystemLayer(nullptr), mCurrentTimerExpiry(0), - mTimerIntervalShift(CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) + mContextPool(contextPool), mSystemLayer(nullptr) {} ReliableMessageMgr::~ReliableMessageMgr() {} -void ReliableMessageMgr::Init(chip::System::Layer * systemLayer, SessionManager * sessionManager) +void ReliableMessageMgr::Init(chip::System::Layer * systemLayer) { - mSystemLayer = systemLayer; - mTimeStampBase = System::SystemClock().GetMonotonicTimestamp(); - mCurrentTimerExpiry = System::Clock::kZero; + mSystemLayer = systemLayer; } void ReliableMessageMgr::Shutdown() @@ -69,23 +66,13 @@ void ReliableMessageMgr::Shutdown() // Clear the retransmit table mRetransTable.ForEachActiveObject([&](auto * entry) { - ClearRetransTable(*entry); + mRetransTable.ReleaseObject(entry); return true; }); mSystemLayer = nullptr; } -uint64_t ReliableMessageMgr::GetTickCounterFromTimePeriod(System::Clock::Milliseconds64 period) -{ - return (period.count() >> mTimerIntervalShift); -} - -uint64_t ReliableMessageMgr::GetTickCounterFromTimeDelta(System::Clock::Timestamp newTime) -{ - return GetTickCounterFromTimePeriod(newTime - mTimeStampBase); -} - #if defined(RMP_TICKLESS_DEBUG) void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) { @@ -93,8 +80,9 @@ void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) mRetransTable.ForEachActiveObject([&](auto * entry) { ChipLogDetail(ExchangeManager, - "EC:" ChipLogFormatExchange " MessageCounter:" ChipLogFormatMessageCounter " NextRetransTimeCtr:%04" PRIX16, - ChipLogValueExchange(&entry->ec.Get()), entry->retainedBuf.GetMessageCounter(), entry->nextRetransTimeTick); + "EC:" ChipLogFormatExchange " MessageCounter:" ChipLogFormatMessageCounter " NextRetransTimeCtr:%" PRIu64, + ChipLogValueExchange(&entry->ec.Get()), entry->retainedBuf.GetMessageCounter(), + entry->nextRetransTime.count()); return true; }); } @@ -107,76 +95,55 @@ void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) void ReliableMessageMgr::ExecuteActions() { + System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions"); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions at % " PRIu64 "ms", now.count()); #endif - ExecuteForAllContext([](ReliableMessageContext * rc) { + ExecuteForAllContext([&](ReliableMessageContext * rc) { if (rc->IsAckPending()) { - if (0 == rc->mNextAckTimeTick) + if (rc->mNextAckTime <= now) { #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions sending ACK"); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions sending ACK %p", rc); #endif - // Send the Ack in a SecureChannel::StandaloneAck message rc->SendStandaloneAckMessage(); } } }); - TicklessDebugDumpRetransTable("ReliableMessageMgr::ExecuteActions Dumping mRetransTable entries before processing"); - - // Retransmit / cancel anything in the retrans table whose retrans timeout - // has expired + // Retransmit / cancel anything in the retrans table whose retrans timeout has expired mRetransTable.ForEachActiveObject([&](auto * entry) { - CHIP_ERROR err = CHIP_NO_ERROR; - - if (entry->nextRetransTimeTick != 0) + if (entry->nextRetransTime > now) return true; - if (entry->retainedBuf.IsNull()) - { - // We generally try to prevent entries with a null buffer being in a table, but it could happen - // if the message dispatch (which is supposed to fill in the buffer) fails to do so _and_ returns - // success (so its caller doesn't clear out the bogus table entry). - // - // If that were to happen, we would crash in the code below. Guard against it, just in case. - ClearRetransTable(*entry); - return true; - } + VerifyOrDie(!entry->retainedBuf.IsNull()); uint8_t sendCount = entry->sendCount; uint32_t messageCounter = entry->retainedBuf.GetMessageCounter(); if (sendCount == CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS) { - err = CHIP_ERROR_MESSAGE_NOT_ACKNOWLEDGED; - ChipLogError(ExchangeManager, "Failed to Send CHIP MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange " sendCount: %" PRIu8 " max retries: %d", messageCounter, ChipLogValueExchange(&entry->ec.Get()), sendCount, CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS); - // Remove from Table - ClearRetransTable(*entry); + // Do not StartTimer, we will schedule the timer at the end of the timer handler. + mRetransTable.ReleaseObject(entry); + return true; } - // Resend from Table (if the operation fails, the entry is cleared) - if (err == CHIP_NO_ERROR) - err = SendFromRetransTable(entry); - - if (err == CHIP_NO_ERROR) - { - // If the retransmission was successful, update the passive timer - entry->nextRetransTimeTick = static_cast(entry->ec->GetActiveRetransmitTimeoutTick()); -#if !defined(NDEBUG) - ChipLogDetail(ExchangeManager, - "Retransmitted MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange - " Send Cnt %d", - messageCounter, ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); -#endif - } + ChipLogDetail(ExchangeManager, + "Retransmitting MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange + " Send Cnt %d", + messageCounter, ChipLogValueExchange(&entry->ec.Get()), entry->sendCount); + // TODO: Choose active/idle timeout corresponding to the activity of exchanges of the session. + entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + entry->ec->GetMRPConfig().mActiveRetransTimeout; + SendFromRetransTable(entry); + // For test not using async IO loop, the entry may have been removed after send, do not use entry below return true; }); @@ -184,61 +151,6 @@ void ReliableMessageMgr::ExecuteActions() TicklessDebugDumpRetransTable("ReliableMessageMgr::ExecuteActions Dumping mRetransTable entries after processing"); } -static void TickProceed(uint16_t & time, uint64_t ticks) -{ - if (time >= ticks) - { - time = static_cast(time - ticks); - } - else - { - time = 0; - } -} - -void ReliableMessageMgr::ExpireTicks() -{ - System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); - - // Number of full ticks elapsed since last timer processing. We always round down - // to the previous tick. If we are between tick boundaries, the extra time since the - // last virtual tick is not accounted for here (it will be accounted for when resetting - // the ReliableMessageProtocol timer) - uint64_t deltaTicks = GetTickCounterFromTimeDelta(now); - -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks at %" PRIu64 ", %" PRIu64 ", %" PRIu64, now, mTimeStampBase, - deltaTicks); -#endif - - ExecuteForAllContext([deltaTicks](ReliableMessageContext * rc) { - if (rc->IsAckPending()) - { - // Decrement counter of Ack timestamp by the elapsed timer ticks - TickProceed(rc->mNextAckTimeTick, deltaTicks); -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks set mNextAckTimeTick to %u", rc->mNextAckTimeTick); -#endif - } - }); - - mRetransTable.ForEachActiveObject([&](auto * entry) { - // Decrement Retransmit timeout by elapsed timeticks - TickProceed(entry->nextRetransTimeTick, deltaTicks); -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks set nextRetransTimeTick to %u", entry->nextRetransTimeTick); -#endif - return true; - }); - - // Re-Adjust the base time stamp to the most recent tick boundary - mTimeStampBase += System::Clock::Milliseconds32(deltaTicks << mTimerIntervalShift); - -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExpireTicks mTimeStampBase to %" PRIu64, mTimeStampBase.count()); -#endif -} - void ReliableMessageMgr::Timeout(System::Layer * aSystemLayer, void * aAppState) { ReliableMessageMgr * manager = reinterpret_cast(aAppState); @@ -249,9 +161,6 @@ void ReliableMessageMgr::Timeout(System::Layer * aSystemLayer, void * aAppState) ChipLogDetail(ExchangeManager, "ReliableMessageMgr::Timeout\n"); #endif - // Make sure all tick counts are sync'd to the current time - manager->ExpireTicks(); - // Execute any actions that are due this tick manager->ExecuteActions(); @@ -263,11 +172,7 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re { VerifyOrDie(!rc->IsMessageNotAcked()); - // Expire any virtual ticks that have expired so all wakeup sources reflect the current time - ExpireTicks(); - *rEntry = mRetransTable.CreateObject(rc); - if (*rEntry == nullptr) { ChipLogError(ExchangeManager, "mRetransTable Already Full"); @@ -279,38 +184,11 @@ CHIP_ERROR ReliableMessageMgr::AddToRetransTable(ReliableMessageContext * rc, Re void ReliableMessageMgr::StartRetransmision(RetransTableEntry * entry) { - entry->nextRetransTimeTick = static_cast(entry->ec->GetIdleRetransmitTimeoutTick() + - GetTickCounterFromTimeDelta(System::SystemClock().GetMonotonicTimestamp())); - - // Check if the timer needs to be started and start it. + // TODO: Choose active/idle timeout corresponding to the activity of exchanges of the session. + entry->nextRetransTime = System::SystemClock().GetMonotonicTimestamp() + entry->ec->GetMRPConfig().mIdleRetransTimeout; StartTimer(); } -void ReliableMessageMgr::PauseRetransmision(ReliableMessageContext * rc, uint32_t PauseTimeMillis) -{ - mRetransTable.ForEachActiveObject([&](auto * entry) { - if (entry->ec->GetReliableMessageContext() == rc) - { - entry->nextRetransTimeTick = - static_cast(entry->nextRetransTimeTick + (PauseTimeMillis >> mTimerIntervalShift)); - return false; - } - return true; - }); -} - -void ReliableMessageMgr::ResumeRetransmision(ReliableMessageContext * rc) -{ - mRetransTable.ForEachActiveObject([&](auto * entry) { - if (entry->ec->GetReliableMessageContext() == rc) - { - entry->nextRetransTimeTick = 0; - return false; - } - return true; - }); -} - bool ReliableMessageMgr::CheckAndRemRetransTable(ReliableMessageContext * rc, uint32_t ackMessageCounter) { bool removed = false; @@ -320,12 +198,10 @@ bool ReliableMessageMgr::CheckAndRemRetransTable(ReliableMessageContext * rc, ui // Clear the entry from the retransmision table. ClearRetransTable(*entry); -#if !defined(NDEBUG) ChipLogDetail(ExchangeManager, "Rxd Ack; Removing MessageCounter:" ChipLogFormatMessageCounter " from Retrans Table on exchange " ChipLogFormatExchange, ackMessageCounter, ChipLogValueExchange(rc->GetExchangeContext())); -#endif removed = true; return false; } @@ -385,106 +261,57 @@ CHIP_ERROR ReliableMessageMgr::SendFromRetransTable(RetransTableEntry * entry) void ReliableMessageMgr::ClearRetransTable(ReliableMessageContext * rc) { - RetransTableEntry * result = nullptr; mRetransTable.ForEachActiveObject([&](auto * entry) { if (entry->ec->GetReliableMessageContext() == rc) { - result = entry; + ClearRetransTable(*entry); return false; } return true; }); - if (result != nullptr) - { - ClearRetransTable(*result); - } } void ReliableMessageMgr::ClearRetransTable(RetransTableEntry & entry) { mRetransTable.ReleaseObject(&entry); // Expire any virtual ticks that have expired so all wakeup sources reflect the current time - ExpireTicks(); StartTimer(); } -void ReliableMessageMgr::FailRetransTableEntries(ReliableMessageContext * rc, CHIP_ERROR err) -{ - ClearRetransTable(rc); -} - void ReliableMessageMgr::StartTimer() { - CHIP_ERROR res = CHIP_NO_ERROR; - uint64_t nextWakeTimeTick = UINT64_MAX; - bool foundWake = false; - // When do we need to next wake up to send an ACK? + System::Clock::Timestamp nextWakeTime = System::Clock::Timestamp::max(); - ExecuteForAllContext([&nextWakeTimeTick, &foundWake](ReliableMessageContext * rc) { - if (rc->IsAckPending() && rc->mNextAckTimeTick < nextWakeTimeTick) + ExecuteForAllContext([&](ReliableMessageContext * rc) { + if (rc->IsAckPending() && rc->mNextAckTime < nextWakeTime) { - nextWakeTimeTick = rc->mNextAckTimeTick; - foundWake = true; -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer next ACK time %" PRIu64, nextWakeTimeTick); -#endif + nextWakeTime = rc->mNextAckTime; } }); + // When do we need to next wake up for ReliableMessageProtocol retransmit? mRetransTable.ForEachActiveObject([&](auto * entry) { - // When do we need to next wake up for ReliableMessageProtocol retransmit? - if (entry->nextRetransTimeTick < nextWakeTimeTick) + if (entry->nextRetransTime < nextWakeTime) { - nextWakeTimeTick = entry->nextRetransTimeTick; - foundWake = true; -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer RetransTime %" PRIu64, nextWakeTimeTick); -#endif + nextWakeTime = entry->nextRetransTime; } return true; }); - if (foundWake) + if (nextWakeTime != System::Clock::Timestamp::max()) { - // Set timer for next tick boundary - subtract the elapsed time from the current tick - System::Clock::Timestamp timerExpiry = - System::Clock::Milliseconds32(nextWakeTimeTick << mTimerIntervalShift) + mTimeStampBase; - -#if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer wake at %" PRIu64 " ms (%" PRIu64 " %" PRIu64 ")", - timerExpiry, nextWakeTimeTick, mTimeStampBase); -#endif - if (timerExpiry != mCurrentTimerExpiry) - { - // If the tick boundary has expired in the past (delayed processing of event due to other system activity), - // expire the timer immediately - System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); - System::Clock::Timeout timerArmValue = (timerExpiry > now) ? timerExpiry - now : System::Clock::kZero; - #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer set timer for %" PRIu32 " ms", - System::Clock::Milliseconds32(timerArmValue).count()); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer wake at %" PRIu64 "ms", nextWakeTime); #endif - StopTimer(); - res = mSystemLayer->StartTimer(timerArmValue, Timeout, this); - VerifyOrDieWithMsg(res == CHIP_NO_ERROR, ExchangeManager, - "Cannot start ReliableMessageMgr::Timeout %" CHIP_ERROR_FORMAT, res.Format()); - mCurrentTimerExpiry = timerExpiry; -#if defined(RMP_TICKLESS_DEBUG) - } - else - { - ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer timer already set for %" PRIu64, timerExpiry); -#endif - } + StopTimer(); + VerifyOrDie(mSystemLayer->StartTimer(nextWakeTime, Timeout, this) == CHIP_NO_ERROR); } else { #if defined(RMP_TICKLESS_DEBUG) - ChipLogDetail(ExchangeManager, "Not setting ReliableMessageProtocol timeout at %" PRIu64, - System::SystemClock().GetMonotonicTimestamp().count()); + ChipLogDetail(ExchangeManager, "ReliableMessageMgr skipped timer"); #endif StopTimer(); } diff --git a/src/messaging/ReliableMessageMgr.h b/src/messaging/ReliableMessageMgr.h index 617007a2f81b45..b1326b9b95747f 100644 --- a/src/messaging/ReliableMessageMgr.h +++ b/src/messaging/ReliableMessageMgr.h @@ -26,12 +26,11 @@ #include #include -#include -#include - #include #include #include +#include +#include #include #include #include @@ -63,37 +62,20 @@ class ReliableMessageMgr RetransTableEntry(ReliableMessageContext * rc); ~RetransTableEntry(); - ExchangeHandle ec; /**< The context for the stored CHIP message. */ - EncryptedPacketBufferHandle retainedBuf; /**< The packet buffer holding the CHIP message. */ - uint16_t nextRetransTimeTick; /**< A counter representing the next retransmission time for the message. */ - uint8_t sendCount; /**< A counter representing the number of times the message has been sent. */ + ExchangeHandle ec; /**< The context for the stored CHIP message. */ + EncryptedPacketBufferHandle retainedBuf; /**< The packet buffer holding the CHIP message. */ + System::Clock::Timestamp nextRetransTime; /**< A counter representing the next retransmission time for the message. */ + uint8_t sendCount; /**< The number of times we have tried to send this entry, + including both successfully and failure send. */ }; public: ReliableMessageMgr(BitMapObjectPool & contextPool); ~ReliableMessageMgr(); - void Init(chip::System::Layer * systemLayer, SessionManager * sessionManager); + void Init(chip::System::Layer * systemLayer); void Shutdown(); - /** - * Return a tick counter value given a time period. - * - * @param[in] period Timestamp value of in milliseconds. - * - * @return Tick count for the time period. - */ - uint64_t GetTickCounterFromTimePeriod(System::Clock::Milliseconds64 period); - - /** - * Return a tick counter value between the given time and the stored time. - * - * @param[in] newTime Timestamp value of in milliseconds. - * - * @return Tick count of the difference between the given time and the stored time. - */ - uint64_t GetTickCounterFromTimeDelta(System::Clock::Timestamp newTime); - /** * Iterate through active exchange contexts and retrans table entries. If an * action needs to be triggered by ReliableMessageProtocol time facilities, @@ -129,26 +111,6 @@ class ReliableMessageMgr */ void StartRetransmision(RetransTableEntry * entry); - /** - * Pause retranmisttion of current exchange for specified period. - * - * @param[in] rc A pointer to the ExchangeContext object. - * - * @param[in] PauseTimeMillis Pause period in milliseconds. - * - * @retval #CHIP_NO_ERROR On success. - */ - void PauseRetransmision(ReliableMessageContext * rc, uint32_t PauseTimeMillis); - - /** - * Re-start retranmisttion of cached encryped packets for the given ReliableMessageContext. - * - * @param[in] rc The ReliableMessageContext to resume retransmission for. - * - * @retval #CHIP_NO_ERROR On success. - */ - void ResumeRetransmision(ReliableMessageContext * rc); - /** * Iterate through active exchange contexts and retrans table entries. Clear the entry matching * the specified ExchangeContext and the message ID from the retransmision table. @@ -185,16 +147,6 @@ class ReliableMessageMgr */ void ClearRetransTable(RetransTableEntry & rEntry); - /** - * Fail entries matching a specified ExchangeContext. - * - * @param[in] rc A pointer to the ExchangeContext object. - * - * @param[in] err The error for failing table entries. - * - */ - void FailRetransTableEntries(ReliableMessageContext * rc, CHIP_ERROR err); - /** * Iterate through active exchange contexts and retrans table entries. * Determine how many ReliableMessageProtocol ticks we need to sleep before we @@ -210,29 +162,14 @@ class ReliableMessageMgr */ void StopTimer(); - /** - * Calculate number of virtual ReliableMessageProtocol ticks that have expired - * since we last called this function. Iterate through active exchange contexts - * and retrans table entries, subtracting expired virtual ticks to synchronize - * wakeup times with the current system time. Do not perform any actions beyond - * updating tick counts, actions will be performed by the physical - * ReliableMessageProtocol timer tick expiry. - * - */ - void ExpireTicks(); - #if CHIP_CONFIG_TEST // Functions for testing int TestGetCountRetransTable(); - void TestSetIntervalShift(uint16_t value) { mTimerIntervalShift = value; } #endif // CHIP_CONFIG_TEST private: BitMapObjectPool & mContextPool; chip::System::Layer * mSystemLayer; - System::Clock::Timestamp mTimeStampBase; // ReliableMessageProtocol timer base value to add offsets to evaluate timeouts - System::Clock::Timestamp mCurrentTimerExpiry; // Tracks when the ReliableMessageProtocol timer will next expire - uint16_t mTimerIntervalShift; // ReliableMessageProtocol Timer tick period shift /* Placeholder function to run a function for all exchanges */ template diff --git a/src/messaging/ReliableMessageProtocolConfig.cpp b/src/messaging/ReliableMessageProtocolConfig.cpp index e373b0c4f3dff5..76c56a8b5a96ac 100644 --- a/src/messaging/ReliableMessageProtocolConfig.cpp +++ b/src/messaging/ReliableMessageProtocolConfig.cpp @@ -30,8 +30,7 @@ namespace chip { using namespace System::Clock::Literals; -const ReliableMessageProtocolConfig - gDefaultMRPConfig(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT, - CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL >> CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT); +const ReliableMessageProtocolConfig gDefaultMRPConfig(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL, + CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); } // namespace chip diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h index d2bb05ffe20a84..f03641f0139ed9 100644 --- a/src/messaging/ReliableMessageProtocolConfig.h +++ b/src/messaging/ReliableMessageProtocolConfig.h @@ -25,24 +25,11 @@ */ #pragma once +#include #include -#include - namespace chip { -/** - * @def CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - * - * @brief - * The default ReliableMessageProtocol timer tick interval shift in - * milliseconds. 6 bit equals 64 milliseconds - * - */ -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - /** * @def CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL * @@ -55,7 +42,7 @@ namespace chip { * */ #ifndef CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL -#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (300) +#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (300_ms32) #endif // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL /** @@ -69,19 +56,19 @@ namespace chip { * needs (e.g. sleeping period) using Service Discovery TXT record CRI key. */ #ifndef CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL -#define CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL (5000) +#define CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL (5000_ms32) #endif // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL /** - * @def CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK + * @def CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT * * @brief * The default acknowledgment timeout in milliseconds. * */ -#ifndef CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK -#define CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK (1) -#endif // CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT_TICK +#ifndef CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT +#define CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT (200_ms32) +#endif // CHIP_CONFIG_RMP_DEFAULT_ACK_TIMEOUT /** * @def CHIP_CONFIG_RMP_RETRANS_TABLE_SIZE @@ -119,12 +106,15 @@ namespace chip { */ struct ReliableMessageProtocolConfig { - ReliableMessageProtocolConfig(uint32_t idleInterval, uint32_t activeInterval) : - mIdleRetransTimeoutTick(idleInterval), mActiveRetransTimeoutTick(activeInterval) + ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, System::Clock::Milliseconds32 activeInterval) : + mIdleRetransTimeout(idleInterval), mActiveRetransTimeout(activeInterval) {} - uint32_t mIdleRetransTimeoutTick; /**< Configurable timeout in msec for retransmission of the first sent message. */ - uint32_t mActiveRetransTimeoutTick; /**< Configurable timeout in msec for retransmission of all subsequent messages. */ + // Configurable timeout in msec for retransmission of the first sent message. + System::Clock::Milliseconds32 mIdleRetransTimeout; + + // Configurable timeout in msec for retransmission of all subsequent messages. + System::Clock::Milliseconds32 mActiveRetransTimeout; }; extern const ReliableMessageProtocolConfig gDefaultMRPConfig; diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index 994170f11396d7..279be96b67b3ac 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -53,6 +53,7 @@ using namespace chip::Inet; using namespace chip::Transport; using namespace chip::Messaging; using namespace chip::Protocols; +using namespace chip::System::Clock::Literals; using TestContext = chip::Test::MessagingContext; @@ -190,28 +191,6 @@ void CheckAddClearRetrans(nlTestSuite * inSuite, void * inContext) exchange->Close(); } -void CheckFailRetrans(nlTestSuite * inSuite, void * inContext) -{ - TestContext & ctx = *reinterpret_cast(inContext); - - MockAppDelegate mockAppDelegate; - ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockAppDelegate); - NL_TEST_ASSERT(inSuite, exchange != nullptr); - - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); - NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - - ReliableMessageMgr::RetransTableEntry * entry; - rm->AddToRetransTable(rc, &entry); - NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); - rm->FailRetransTableEntries(rc, CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); - - exchange->Close(); -} - void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext) { TestContext & ctx = *reinterpret_cast(inContext); @@ -226,15 +205,14 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext) ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's drop the initial message gLoopback.mSentMessageCount = 0; @@ -244,8 +222,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext) // Ensure the retransmit table is empty right now NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); - // Ensure the exchange stays open after we send (unlike the - // CheckCloseExchangeAndResendApplicationMessage case), by claiming to + // Ensure the exchange stays open after we send (unlike the CheckCloseExchangeAndResendApplicationMessage case), by claiming to // expect a response. err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendMessageFlags::kExpectResponse); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); @@ -255,7 +232,7 @@ void CheckResendApplicationMessage(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, gLoopback.mDroppedMessageCount == 1); NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -291,15 +268,14 @@ void CheckCloseExchangeAndResendApplicationMessage(nlTestSuite * inSuite, void * ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's drop the initial message gLoopback.mSentMessageCount = 0; @@ -317,7 +293,7 @@ void CheckCloseExchangeAndResendApplicationMessage(nlTestSuite * inSuite, void * NL_TEST_ASSERT(inSuite, gLoopback.mDroppedMessageCount == 1); NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -353,15 +329,14 @@ void CheckFailedMessageRetainOnSend(nlTestSuite * inSuite, void * inContext) ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); mockSender.mMessageDispatch.mRetainMessageOnSend = false; @@ -378,7 +353,7 @@ void CheckFailedMessageRetainOnSend(nlTestSuite * inSuite, void * inContext) // Ensure the message was dropped NL_TEST_ASSERT(inSuite, gLoopback.mDroppedMessageCount == 1); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -410,10 +385,8 @@ void CheckUnencryptedMessageReceiveFailure(nlTestSuite * inSuite, void * inConte ExchangeContext * exchange = ctx.NewUnauthenticatedExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); gLoopback.mSentMessageCount = 0; gLoopback.mNumMessagesToDrop = 0; @@ -449,15 +422,14 @@ void CheckResendApplicationMessageWithPeerExchange(nlTestSuite * inSuite, void * ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's drop the initial message gLoopback.mSentMessageCount = 0; @@ -476,7 +448,7 @@ void CheckResendApplicationMessageWithPeerExchange(nlTestSuite * inSuite, void * NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -511,15 +483,14 @@ void CheckDuplicateMessageClosedExchange(nlTestSuite * inSuite, void * inContext ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_RMP_DEFAULT_INITIAL_RETRY_INTERVAL - 1, // CHIP_CONFIG_RMP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_RMP_DEFAULT_INITIAL_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_RMP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's not drop the message. Expectation is that it is received by the peer, but the ack is dropped gLoopback.mSentMessageCount = 0; @@ -548,7 +519,7 @@ void CheckDuplicateMessageClosedExchange(nlTestSuite * inSuite, void * inContext err = ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -586,15 +557,14 @@ void CheckResendSessionEstablishmentMessageWithPeerExchange(nlTestSuite * inSuit ExchangeContext * exchange = ctx.NewUnauthenticatedExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's drop the initial message gLoopback.mSentMessageCount = 0; @@ -613,7 +583,7 @@ void CheckResendSessionEstablishmentMessageWithPeerExchange(nlTestSuite * inSuit NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); NL_TEST_ASSERT(inSuite, !mockReceiver.IsOnMessageReceivedCalled); - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -659,15 +629,14 @@ void CheckDuplicateMessage(nlTestSuite * inSuite, void * inContext) ExchangeContext * exchange = ctx.NewExchangeToAlice(&mockSender); NL_TEST_ASSERT(inSuite, exchange != nullptr); - ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); + ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); NL_TEST_ASSERT(inSuite, rm != nullptr); - NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_RMP_DEFAULT_INITIAL_RETRY_INTERVAL - 1, // CHIP_CONFIG_RMP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_RMP_DEFAULT_INITIAL_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_RMP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Let's not drop the message. Expectation is that it is received by the peer, but the ack is dropped gLoopback.mSentMessageCount = 0; @@ -697,7 +666,7 @@ void CheckDuplicateMessage(nlTestSuite * inSuite, void * inContext) mockReceiver.mDropAckResponse = false; mockReceiver.mRetainExchange = false; - // 1 tick is 64 ms, sleep 65 ms to trigger first re-transmit + // sleep 65 ms to trigger first re-transmit chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -1144,11 +1113,11 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); // Make sure that we resend our message before the other side does. - ReliableMessageContext * rc = exchange->GetReliableMessageContext(); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + exchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // We send a message, the other side sends an application-level response // (which is lost), then we do a retransmit that is acked, then the other @@ -1181,10 +1150,11 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext) // Make sure receiver resends after sender does, and there's enough of a gap // that we are very unlikely to actually trigger the resends on the receiver // when we trigger the resends on the sender. - receiverRc->SetConfig({ - 4, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 4, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + mockReceiver.mExchange->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 256_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 256_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); // Now send a message from the other side, but drop it. gLoopback.mNumMessagesToDrop = 1; @@ -1215,7 +1185,7 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext) mockSender.IsOnMessageReceivedCalled = false; mockSender.mReceivedPiggybackAck = false; - // 1 tick is 64 ms, sleep 65 ms to trigger re-transmit from sender + // sleep 65 ms to trigger re-transmit from sender chip::test_utils::SleepMillis(65); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -1244,7 +1214,7 @@ void CheckLostResponseWithPiggyback(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0); } - // 1 tick is 64 ms, sleep 65*3 ms to trigger re-transmit from receiver + // sleep 65*3 ms to trigger re-transmit from receiver chip::test_utils::SleepMillis(65 * 3); ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), rm); @@ -1402,7 +1372,6 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext) const nlTest sTests[] = { NL_TEST_DEF("Test ReliableMessageMgr::CheckAddClearRetrans", CheckAddClearRetrans), - NL_TEST_DEF("Test ReliableMessageMgr::CheckFailRetrans", CheckFailRetrans), NL_TEST_DEF("Test ReliableMessageMgr::CheckResendApplicationMessage", CheckResendApplicationMessage), NL_TEST_DEF("Test ReliableMessageMgr::CheckCloseExchangeAndResendApplicationMessage", CheckCloseExchangeAndResendApplicationMessage), NL_TEST_DEF("Test ReliableMessageMgr::CheckFailedMessageRetainOnSend", CheckFailedMessageRetainOnSend), diff --git a/src/platform/Darwin/CHIPPlatformConfig.h b/src/platform/Darwin/CHIPPlatformConfig.h index d16acc0e7d1cd2..f1778c437dc984 100644 --- a/src/platform/Darwin/CHIPPlatformConfig.h +++ b/src/platform/Darwin/CHIPPlatformConfig.h @@ -94,10 +94,6 @@ #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 32 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 1 #endif // CHIP_LOG_FILTERING @@ -108,7 +104,7 @@ // TODO - Fine tune MRP default parameters for Darwin platform #define CHIP_CONFIG_MRP_DEFAULT_INITIAL_RETRY_INTERVAL (15000) -#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000) +#define CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL (2000_ms32) // ==================== Security Configuration Overrides ==================== diff --git a/src/platform/EFR32/CHIPPlatformConfig.h b/src/platform/EFR32/CHIPPlatformConfig.h index 0e924de8e8fef2..bf6de056644859 100644 --- a/src/platform/EFR32/CHIPPlatformConfig.h +++ b/src/platform/EFR32/CHIPPlatformConfig.h @@ -99,10 +99,6 @@ #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 16 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/ESP32/CHIPPlatformConfig.h b/src/platform/ESP32/CHIPPlatformConfig.h index 33bca2b5d3725c..aadc79461d67ef 100644 --- a/src/platform/ESP32/CHIPPlatformConfig.h +++ b/src/platform/ESP32/CHIPPlatformConfig.h @@ -80,7 +80,6 @@ #define CHIP_CONFIG_MAX_PEER_NODES CONFIG_MAX_PEER_NODES #define CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS #define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS CONFIG_MAX_EXCHANGE_CONTEXTS -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT #define CHIP_CONFIG_MAX_SESSION_KEYS CONFIG_MAX_SESSION_KEYS #define CHIP_CONFIG_USE_APP_GROUP_KEYS_FOR_MSG_ENC CONFIG_USE_APP_GROUP_KEYS_FOR_MSG_ENC #define CHIP_CONFIG_MAX_CACHED_MSG_ENC_APP_KEYS CONFIG_MAX_CACHED_MSG_ENC_APP_KEYS diff --git a/src/platform/Linux/CHIPPlatformConfig.h b/src/platform/Linux/CHIPPlatformConfig.h index 0659d4359b1723..fa8cf092f3c390 100644 --- a/src/platform/Linux/CHIPPlatformConfig.h +++ b/src/platform/Linux/CHIPPlatformConfig.h @@ -98,10 +98,6 @@ using CHIP_CONFIG_PERSISTED_STORAGE_KEY_TYPE = const char *; #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 32 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index c01a2ab101dcab..b5198cf85ed701 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -1450,8 +1450,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::DoInit(otInstanc #if CHIP_DEVICE_CONFIG_ENABLE_SED ConnectivityManager::SEDPollingConfig sedPollingConfig; - mPollingConfig.Clear(); - sedPollingConfig.Clear(); + using namespace System::Clock::Literals; sedPollingConfig.FastPollingIntervalMS = CHIP_DEVICE_CONFIG_SED_FAST_POLLING_INTERVAL; sedPollingConfig.SlowPollingIntervalMS = CHIP_DEVICE_CONFIG_SED_SLOW_POLLING_INTERVAL; err = _SetSEDPollingConfig(sedPollingConfig); @@ -1524,8 +1523,9 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetSEDPollingConfig( const ConnectivityManager::SEDPollingConfig & pollingConfig) { - if ((pollingConfig.SlowPollingIntervalMS < pollingConfig.FastPollingIntervalMS) || (pollingConfig.SlowPollingIntervalMS == 0) || - (pollingConfig.FastPollingIntervalMS == 0)) + using namespace System::Clock::Literals; + if ((pollingConfig.SlowPollingIntervalMS < pollingConfig.FastPollingIntervalMS) || + (pollingConfig.SlowPollingIntervalMS == 0_ms32) || (pollingConfig.FastPollingIntervalMS == 0_ms32)) { return CHIP_ERROR_INVALID_ARGUMENT; } @@ -1547,7 +1547,7 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::SetSEDPollingMode(ConnectivityManager::SEDPollingMode pollingType) { CHIP_ERROR err = CHIP_NO_ERROR; - uint32_t interval; + System::Clock::Milliseconds32 interval; if (pollingType == ConnectivityManager::SEDPollingMode::Idle) { @@ -1567,17 +1567,17 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::SetSEDPollingMod uint32_t curPollingIntervalMS = otLinkGetPollPeriod(mOTInst); - if (interval != curPollingIntervalMS) + if (interval.count() != curPollingIntervalMS) { - otError otErr = otLinkSetPollPeriod(mOTInst, interval); + otError otErr = otLinkSetPollPeriod(mOTInst, interval.count()); err = MapOpenThreadError(otErr); } Impl()->UnlockThreadStack(); - if (interval != curPollingIntervalMS) + if (interval.count() != curPollingIntervalMS) { - ChipLogProgress(DeviceLayer, "OpenThread polling interval set to %" PRId32 "ms", interval); + ChipLogProgress(DeviceLayer, "OpenThread polling interval set to %" PRId32 "ms", interval.count()); } return err; diff --git a/src/platform/Tizen/CHIPPlatformConfig.h b/src/platform/Tizen/CHIPPlatformConfig.h index 7067e0876fcded..e1b95a71fb0a82 100644 --- a/src/platform/Tizen/CHIPPlatformConfig.h +++ b/src/platform/Tizen/CHIPPlatformConfig.h @@ -80,10 +80,6 @@ #define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS 8 #endif // CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/android/CHIPPlatformConfig.h b/src/platform/android/CHIPPlatformConfig.h index e05ad640a9ff35..97ae7648bdbfe1 100644 --- a/src/platform/android/CHIPPlatformConfig.h +++ b/src/platform/android/CHIPPlatformConfig.h @@ -96,10 +96,6 @@ using CHIP_CONFIG_PERSISTED_STORAGE_KEY_TYPE = const char *; #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 32 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/cc13x2_26x2/CHIPPlatformConfig.h b/src/platform/cc13x2_26x2/CHIPPlatformConfig.h index 62d7637405fd0a..08c4c4039cc14f 100644 --- a/src/platform/cc13x2_26x2/CHIPPlatformConfig.h +++ b/src/platform/cc13x2_26x2/CHIPPlatformConfig.h @@ -97,10 +97,6 @@ #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 16 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/mbed/CHIPPlatformConfig.h b/src/platform/mbed/CHIPPlatformConfig.h index 86ec46bb3b1fba..5e2fdb071d885a 100644 --- a/src/platform/mbed/CHIPPlatformConfig.h +++ b/src/platform/mbed/CHIPPlatformConfig.h @@ -90,10 +90,6 @@ #define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS 8 #endif // CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 1 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/nrfconnect/CHIPPlatformConfig.h b/src/platform/nrfconnect/CHIPPlatformConfig.h index 716992dc23d871..2c6e5054a5b091 100644 --- a/src/platform/nrfconnect/CHIPPlatformConfig.h +++ b/src/platform/nrfconnect/CHIPPlatformConfig.h @@ -91,10 +91,6 @@ #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 16 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/nxp/k32w/k32w0/CHIPPlatformConfig.h b/src/platform/nxp/k32w/k32w0/CHIPPlatformConfig.h index 87d0f7d1446c49..0f1fd54a4c4b5b 100644 --- a/src/platform/nxp/k32w/k32w0/CHIPPlatformConfig.h +++ b/src/platform/nxp/k32w/k32w0/CHIPPlatformConfig.h @@ -100,10 +100,6 @@ #define CHIP_CONFIG_MAX_CHANNEL_HANDLES 16 #endif // CHIP_CONFIG_MAX_CHANNEL_HANDLES -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/platform/telink/CHIPPlatformConfig.h b/src/platform/telink/CHIPPlatformConfig.h index 37945a0a435157..954f3d42f15af1 100644 --- a/src/platform/telink/CHIPPlatformConfig.h +++ b/src/platform/telink/CHIPPlatformConfig.h @@ -82,10 +82,6 @@ #define CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS 8 #endif // CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS -#ifndef CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT -#define CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT 6 -#endif // CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT - #ifndef CHIP_LOG_FILTERING #define CHIP_LOG_FILTERING 0 #endif // CHIP_LOG_FILTERING diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index ea1409047d6387..a3d2316de750b3 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -45,16 +45,7 @@ using TestContext = chip::Test::MessagingContext; class PASETestLoopbackTransport : public Test::LoopbackTransport { - void MessageDropped() override - { - // Trigger a retransmit. - if (mContext != nullptr) - { - chip::test_utils::SleepMillis(65); - ReliableMessageMgr * rm = mContext->GetExchangeManager().GetReliableMessageMgr(); - ReliableMessageMgr::Timeout(&mContext->GetSystemLayer(), rm); - } - } + void MessageDropped() override { mMessageDropped = true; } public: bool CanSendToPeer(const PeerAddress & address) override { return true; } @@ -66,6 +57,7 @@ class PASETestLoopbackTransport : public Test::LoopbackTransport } TestContext * mContext = nullptr; + bool mMessageDropped = false; }; TransportMgrBase gTransportMgr; @@ -95,6 +87,8 @@ class MockAppDelegate : public ExchangeDelegate void OnResponseTimeout(ExchangeContext * ec) override {} }; +using namespace System::Clock::Literals; + void SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext) { // Test all combinations of invalid parameters @@ -179,10 +173,11 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, P NL_TEST_ASSERT(inSuite, rm != nullptr); NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + contextCommissioner->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); gLoopback.mContext = &ctx; } @@ -197,6 +192,13 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, P pairingCommissioner.Pair(Transport::PeerAddress(Transport::Type::kBle), 1234, 0, contextCommissioner, &delegateCommissioner) == CHIP_NO_ERROR); + while (gLoopback.mMessageDropped) + { + chip::test_utils::SleepMillis(65); + gLoopback.mMessageDropped = false; + ReliableMessageMgr::Timeout(&ctx.GetSystemLayer(), ctx.GetExchangeManager().GetReliableMessageMgr()); + }; + // Standalone acks also increment the mSentMessageCount. But some messages could be acked // via piggybacked acks. So we cannot check for a specific value of mSentMessageCount. // Let's make sure atleast number is >= than the minimum messages required to complete the @@ -249,10 +251,11 @@ void SecurePairingFailedHandshake(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, rm != nullptr); NL_TEST_ASSERT(inSuite, rc != nullptr); - rc->SetConfig({ - 1, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL - 1, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL - }); + contextCommissioner->GetSessionHandle().SetMRPConfig(&ctx.GetSecureSessionManager(), + { + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL + 64_ms32, // CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL + }); gLoopback.mContext = &ctx; NL_TEST_ASSERT(inSuite,