Address review comments.

project-chip · Jan 14, 2025 · e9c2011 · e9c2011
1 parent 3153888
commit e9c2011
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 33 deletions.
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -116,6 +116,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
     mUserDirectedCommissioningPort = initParams.userDirectedCommissioningPort;
     mInterfaceId                   = initParams.interfaceId;
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    auto tcpListenParams = TcpListenParameters(DeviceLayer::TCPEndPointManager())
+                               .SetAddressType(IPAddressType::kIPv6)
+                               .SetListenPort(mOperationalServicePort);
+#endif
+
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     VerifyOrExit(initParams.persistentStorageDelegate != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
@@ -226,12 +232,10 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
 #endif
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
                                ,
-                           TcpListenParameters(DeviceLayer::TCPEndPointManager())
-                               .SetAddressType(IPAddressType::kIPv6)
-                               .SetListenPort(mOperationalServicePort)
+                           tcpListenParams
 #endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
-                               ,
+                           ,
                            Transport::WiFiPAFListenParameters(DeviceLayer::ConnectivityMgr().GetWiFiPAF())
 #endif
     );
@@ -330,8 +334,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
-    // Enable the TCP Server based on the TCPListenParameters setting in TransportMgr Init.
-    app::DnssdServer::Instance().SetTCPServerEnabled(true);
+    // Enable the TCP Server based on the TCPListenParameters setting.
+    app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
     if (GetFabricTable().FabricCount() != 0)

diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -132,6 +132,13 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
     ChipLogError(Controller, "Warning: Device Controller Factory should be with a CHIP Device Layer...");
 #endif // CONFIG_DEVICE_LAYER
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    auto tcpListenParams = Transport::TcpListenParameters(stateParams.tcpEndPointManager)
+                               .SetAddressType(IPAddressType::kIPv6)
+                               .SetListenPort(params.listenPort)
+                               .SetServerListenEnabled(false); // Initialize as a TCP Client
+#endif
+
     if (params.dataModelProvider == nullptr)
     {
         ChipLogError(AppServer, "Device Controller Factory requires a `dataModelProvider` value.");
@@ -179,10 +186,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
 #endif
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
                                                             ,
-                                                        Transport::TcpListenParameters(stateParams.tcpEndPointManager)
-                                                            .SetAddressType(IPAddressType::kIPv6)
-                                                            .SetListenPort(params.listenPort)
-                                                            .SetServerListenEnabled(false) // Initialize as a TCP Client
+                                                        tcpListenParams
 #endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
                                                         ,
@@ -286,8 +290,8 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
         //
         app::DnssdServer::Instance().SetFabricTable(stateParams.fabricTable);
 
-        // Disable the TCP Server based on the TCPListenParameters setting in TransportMgr Init.
-        app::DnssdServer::Instance().SetTCPServerEnabled(false);
+        // Disable the TCP Server based on the TCPListenParameters setting.
+        app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
     }
 
     stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();

diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp
@@ -78,14 +78,12 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
 
     mEndpointType = params.GetAddressType();
 
-    mIsServerListenEnabled = params.IsServerListenEnabled();
-
     // Primary socket endpoint created to help get EndPointManager handle for creating multiple
     // connection endpoints at runtime.
     err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
     SuccessOrExit(err);
 
-    if (mIsServerListenEnabled)
+    if (params.IsServerListenEnabled())
     {
         err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
                                   params.GetInterfaceId().IsPresent());
@@ -677,11 +675,6 @@ void TCPBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool sho
     }
 }
 
-bool TCPBase::IsServerListenEnabled()
-{
-    return mIsServerListenEnabled;
-}
-
 bool TCPBase::HasActiveConnections() const
 {
     for (size_t i = 0; i < mActiveConnectionsSize; i++)

diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h
@@ -187,8 +187,6 @@ class DLL_EXPORT TCPBase : public Base
     // and release from the pool.
     void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = false) override;
 
-    bool IsServerListenEnabled();
-
     bool CanSendToPeer(const PeerAddress & address) override
     {
         return (mState == TCPState::kInitialized) && (address.GetTransportType() == Type::kTcp) &&
@@ -312,7 +310,6 @@ class DLL_EXPORT TCPBase : public Base
     Inet::TCPEndPoint * mListenSocket = nullptr;                       ///< TCP socket used by the transport
     Inet::IPAddressType mEndpointType = Inet::IPAddressType::kUnknown; ///< Socket listening type
     TCPState mState                   = TCPState::kNotReady;           ///< State of the TCP transport
-    bool mIsServerListenEnabled       = true;                          ///< TCP Server mode enabled
 
     // The configured timeout for the connection attempt to the peer, before
     // giving up.

diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp
@@ -555,29 +555,26 @@ TEST_F(TestTCP, CheckSimpleInitTest6)
 TEST_F(TestTCP, InitializeAsTCPClient)
 {
     TCPImpl tcp;
-
-    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
-                                  .SetAddressType(IPAddressType::kIPv6)
-                                  .SetListenPort(gChipTCPPort)
-                                  .SetServerListenEnabled(false));
+    auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
+    CHIP_ERROR err =
+        tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort).SetServerListenEnabled(false));
 
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
-    bool isServerEnabled = tcp.IsServerListenEnabled();
+    bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
     EXPECT_EQ(isServerEnabled, false);
 }
 
 TEST_F(TestTCP, InitializeAsTCPClientServer)
 {
     TCPImpl tcp;
+    auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
 
-    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
-                                  .SetAddressType(IPAddressType::kIPv6)
-                                  .SetListenPort(gChipTCPPort));
+    CHIP_ERROR err = tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort));
 
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
-    bool isServerEnabled = tcp.IsServerListenEnabled();
+    bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
     EXPECT_EQ(isServerEnabled, true);
 }