[RSDK-9244] [RSDK-9245] Feature gate local signaling and adjust max c…

…onnections (#362)

examples/esp-idf-component/sdkconfig.defaults

@@ -4,6 +4,7 @@ CONFIG_ESP32_DEFAULT_CPU_FREQ_240=y
 CONFIG_ESP_SYSTEM_EVENT_TASK_STACK_SIZE=4096
 CONFIG_PTHREAD_TASK_STACK_SIZE_DEFAULT=8192
 CONFIG_ESP_TLS_SERVER=y
+CONFIG_LWIP_MAX_SOCKETS=13
 
 CONFIG_ESP32_SPIRAM_SUPPORT=y
 CONFIG_SPIRAM_IGNORE_NOTFOUND=y

micro-rdk-ffi/src/ffi/runtime.rs

@@ -226,24 +226,15 @@ pub unsafe extern "C" fn viam_server_start(ctx: *mut viam_server_context) -> via
         .unwrap();
     }
 
-    let max_connection = {
-        #[cfg(not(target_os = "espidf"))]
-        {
-            10
-        }
-        #[cfg(target_os = "espidf")]
-        {
-            use micro_rdk::esp32::esp_idf_svc::hal::sys::g_wifi_feature_caps;
-            use micro_rdk::esp32::esp_idf_svc::hal::sys::CONFIG_FEATURE_CACHE_TX_BUF_BIT;
-            if !g_spiram_ok {
-                log::info!("spiram not initialized disabling cache feature of the wifi driver");
-                g_wifi_feature_caps &= !(CONFIG_FEATURE_CACHE_TX_BUF_BIT as u64);
-                1
-            } else {
-                3
-            }
+    #[cfg(target_os = "espidf")]
+    {
+        use micro_rdk::esp32::esp_idf_svc::hal::sys::g_wifi_feature_caps;
+        use micro_rdk::esp32::esp_idf_svc::hal::sys::CONFIG_FEATURE_CACHE_TX_BUF_BIT;
+        if !g_spiram_ok {
+            log::info!("spiram not initialized disabling cache feature of the wifi driver");
+            g_wifi_feature_caps &= !(CONFIG_FEATURE_CACHE_TX_BUF_BIT as u64);
         }
-    };
+    }
 
     let network = {
         #[cfg(not(target_os = "espidf"))]
@@ -301,7 +292,6 @@ pub unsafe extern "C" fn viam_server_start(ctx: *mut viam_server_context) -> via
     builder
         .with_provisioning_info(ctx.provisioning_info)
         .with_component_registry(ctx.registry)
-        .with_max_concurrent_connection(max_connection)
         .with_default_tasks();
 
     #[cfg(not(target_os = "espidf"))]

micro-rdk-server/esp32/main.rs

@@ -109,15 +109,12 @@ mod esp32 {
             }
         }
 
-        let max_connections = unsafe {
+        unsafe {
             if !g_spiram_ok {
                 log::info!("spiram not initialized disabling cache feature of the wifi driver");
                 g_wifi_feature_caps &= !(CONFIG_FEATURE_CACHE_TX_BUF_BIT as u64);
-                1
-            } else {
-                3
             }
-        };
+        }
 
         let mut info = ProvisioningInfo::default();
         info.set_manufacturer("viam".to_owned());
@@ -135,7 +132,6 @@ mod esp32 {
             .with_provisioning_info(info)
             .with_webrtc_configuration(webrtc_config)
             .with_http2_server(Esp32H2Connector::default(), 12346)
-            .with_max_concurrent_connection(max_connections)
             .with_default_tasks()
             .with_component_registry(registry);
         #[cfg(not(feature = "qemu"))]

micro-rdk-server/native/main.rs

@@ -72,7 +72,6 @@ mod native {
         builder
             .with_http2_server(NativeH2Connector::default(), 12346)
             .with_webrtc_configuration(webrtc_config)
-            .with_max_concurrent_connection(3)
             .with_provisioning_info(info)
             .with_component_registry(registry)
             .with_default_tasks();

micro-rdk-server/sdkconfig.defaults

@@ -20,6 +20,7 @@ CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=y
 CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_FULL=y
 
 CONFIG_ESP_TLS_SERVER=y
+CONFIG_LWIP_MAX_SOCKETS=13
 
 CONFIG_SPIRAM_SUPPORT=y
 CONFIG_ESP32_SPIRAM_SUPPORT=y

micro-rdk/Cargo.toml

@@ -15,7 +15,7 @@ links = "micro_rdk"
 crate-type = ["lib"]
 
 [features]
-default = ["builtin-components", "data"]
+default = ["builtin-components", "data", "local-signaling"]
 binstart = ["esp-idf-svc/binstart"]
 libstart = ["esp-idf-svc/libstart"]
 builtin-components = []
@@ -27,6 +27,7 @@ data = []
 qemu = []
 esp-idf-logs = ["esp32"]
 ota = []
+local-signaling = []
 
 [dev-dependencies]
 test-log.workspace = true

micro-rdk/src/common/conn/server.rs

@@ -116,6 +116,11 @@ impl IncomingConnectionManager {
         connections.resize_with(size, Default::default);
         Self { connections }
     }
+
+    pub(crate) fn max_connections(&self) -> usize {
+        self.connections.len()
+    }
+
     // return the lowest priority of active webrtc tasks or 0
     pub(crate) fn get_lowest_prio(&self) -> u32 {
         self.connections

micro-rdk/src/common/conn/viam.rs

@@ -184,7 +184,7 @@ pub struct ViamServerBuilder<Storage, State> {
     http2_server_port: u16,
     http2_server_insecure: bool,
     app_client_tasks: Vec<Box<dyn PeriodicAppClientTask>>,
-    max_concurrent_connections: u32,
+    max_concurrent_connections: usize,
     _state: PhantomData<State>,
 }
 
@@ -194,6 +194,39 @@ where
     <Storage as RobotConfigurationStorage>::Error: Debug,
     ServerError: From<<Storage as RobotConfigurationStorage>::Error>,
 {
+    /// Returns the computed default limit on the number of concurrent
+    /// connections the micro-rdk will permit.
+    pub fn get_default_max_concurrent_connections() -> usize {
+        // NOTE: If you adjust the logic in this method, please check
+        // the value in `Robot::Server::serve_http2_connection` to see
+        // if it needs a coordinated adjustment.
+
+        // By default, we get three, everywhere.
+        #[allow(unused_mut)]
+        let mut max = 3;
+
+        // If local signaling is enabled, grant two more
+        #[cfg(feature = "local-signaling")]
+        {
+            max += 2;
+        }
+
+        // But on an esp32 lacking SPIRAM, assume only one connection can be realized
+        #[cfg(target_os = "espidf")]
+        {
+            extern "C" {
+                pub static g_spiram_ok: bool;
+            }
+            unsafe {
+                if !g_spiram_ok {
+                    max = 1;
+                }
+            }
+        }
+
+        max
+    }
+
     pub fn new(storage: Storage) -> Self {
         ViamServerBuilder {
             storage,
@@ -205,7 +238,7 @@ where
             http2_server_port: 12346,
             http2_server_insecure: false,
             app_client_tasks: Default::default(),
-            max_concurrent_connections: 1,
+            max_concurrent_connections: Self::get_default_max_concurrent_connections(),
             _state: PhantomData,
         }
     }
@@ -235,7 +268,10 @@ where
     <Storage as RobotConfigurationStorage>::Error: Debug,
     ServerError: From<<Storage as RobotConfigurationStorage>::Error>,
 {
-    pub fn with_max_concurrent_connection(&mut self, max_concurrent_connections: u32) -> &mut Self {
+    pub fn with_max_concurrent_connection(
+        &mut self,
+        max_concurrent_connections: usize,
+    ) -> &mut Self {
         self.max_concurrent_connections = max_concurrent_connections;
         self
     }
@@ -376,7 +412,7 @@ pub struct ViamServer<Storage, C, M> {
     app_client_tasks: Vec<Box<dyn PeriodicAppClientTask>>,
     #[cfg(feature = "ota")]
     ota_service_task: Option<Task<()>>,
-    max_concurrent_connections: u32,
+    max_concurrent_connections: usize,
     network: Option<Box<dyn Network>>,
 }
 impl<Storage, C, M> ViamServer<Storage, C, M>
@@ -640,13 +676,16 @@ where
             webrtc_config: &self.webrtc_configuration,
             network,
             incomming_connection_manager: IncomingConnectionManager::new(
-                self.max_concurrent_connections as usize,
+                self.max_concurrent_connections,
             ),
             robot_config: &config,
-            local_signaling_server: Arc::new(SignalingServer::new(
+            #[cfg(feature = "local-signaling")]
+            local_signaling_server: Some(Arc::new(SignalingServer::new(
                 self.executor.clone(),
                 tx.clone(),
-            )),
+            ))),
+            #[cfg(not(feature = "local-signaling"))]
+            local_signaling_server: None,
         };
 
         if let Some(cfg) = config.cloud.as_ref() {
@@ -785,7 +824,8 @@ struct RobotServer<'a, M> {
     network: &'a dyn Network,
     incomming_connection_manager: IncomingConnectionManager,
     robot_config: &'a RobotConfig,
-    local_signaling_server: Arc<SignalingServer>,
+    #[allow(dead_code)]
+    local_signaling_server: Option<Arc<SignalingServer>>,
 }
 
 pub(crate) enum IncomingConnection {
@@ -803,10 +843,31 @@ where
     ) -> Task<Result<(), errors::ServerError>> {
         let exec = self.executor.clone();
         let robot = self.robot.clone();
-        let ss = self.local_signaling_server.clone();
+
+        // If the connection manager has a low limit on the number of
+        // concurrent connections, don't enable local signaling. This
+        // value may need to be adjusted if the logic in
+        // `ViamServerBuilder::get_default_max_concurrent_connections`
+        // changes.
+        #[cfg(feature = "local-signaling")]
+        let ss = self
+            .local_signaling_server
+            .clone()
+            .filter(|_| self.incomming_connection_manager.max_connections() >= 5)
+            .or_else(|| {
+                log::warn!(
+                    "Disabling local WebRTC signaling because the configured connection limit is less than 5",
+                );
+                None
+            });
+
         self.executor.spawn(async move {
+            #[allow(unused_mut)]
             let mut srv = GrpcServer::new(robot, GrpcBody::new());
-            srv.register_signaling_server(ss);
+            #[cfg(feature = "local-signaling")]
+            if let Some(ss) = ss {
+                srv.register_signaling_server(ss);
+            }
             http2::Builder::new(exec)
                 .initial_connection_window_size(2048)
                 .initial_stream_window_size(2048)

micro-rdk/src/common/grpc.rs

@@ -145,6 +145,7 @@ where
     }
 
     // TODO(RSDK-9242): Use the builder pattern instead.
+    #[allow(dead_code)]
     pub(crate) fn register_signaling_server(&mut self, signaling_server: Arc<SignalingServer>) {
         let _ = self.signaling_server.insert(signaling_server);
     }

micro-rdk/src/common/webrtc/signaling_server.rs

@@ -28,6 +28,7 @@ pub(crate) struct SignalingServer {
 }
 
 impl SignalingServer {
+    #[allow(dead_code)]
     pub fn new(executor: Executor, sender: Sender<Box<WebRtcSignalingChannel>>) -> Self {
         Self {
             executor,

templates/project/sdkconfig.defaults

@@ -18,6 +18,7 @@ CONFIG_ESPTOOLPY_FLASHMODE_DIO=n
 CONFIG_ESP_TLS_SERVER=y
 CONFIG_MBEDTLS_SSL_PROTO_DTLS=y
 CONFIG_MBEDTLS_DEFAULT_MEM_ALLOC=y
+CONFIG_LWIP_MAX_SOCKETS=13
 
 CONFIG_SPIRAM_SUPPORT=y
 CONFIG_ESP32_SPIRAM_SUPPORT=y

templates/project/src/main.rs

@@ -100,15 +100,12 @@ fn main() {
         }
     }
 
-    let max_connections = unsafe {
+    unsafe {
         if !g_spiram_ok {
             log::info!("spiram not initialized disabling cache feature of the wifi driver");
             g_wifi_feature_caps &= !(CONFIG_FEATURE_CACHE_TX_BUF_BIT as u64);
-            1
-        } else {
-            3
         }
-    };
+    }
 
     let mut info = ProvisioningInfo::default();
     info.set_manufacturer("viam".to_owned());
@@ -130,7 +127,6 @@ fn main() {
         .with_provisioning_info(info)
         .with_webrtc_configuration(webrtc_config)
         .with_http2_server(Esp32H2Connector::default(), 12346)
-        .with_max_concurrent_connection(max_connections)
         .with_default_tasks()
         .with_component_registry(registry);