Clarify TraCI connection issues.

CHANGELOG.md

@@ -13,10 +13,12 @@ Copy and pasting the git commit messages is __NOT__ enough.
 ### Changed
 - `VehicleIndex.build_agent_vehicle()` no longer has `filename` and `surface_patches` parameters.
 - The following modules have been renamed: `envision.types` -> `envision.etypes`, `smarts.core.utils.logging` -> `smarts.core.utils.core_logging`, `smarts.core.utils.math` -> `smarts.core.utils.core_math`, `smarts.sstudio.types` -> `smarts.sstudio.sstypes`. For compatibility reasons they can still be imported by their original module name.
+- Exposed `traffic:traci_retries`/`SMARTS_TRAFFIC_TRACI_RETRIES` to control how many times the `SumoTrafficSimulation` will try to restart when using default configuration.
+
 ### Deprecated
 ### Fixed
+- `SumoTrafficSimulation` gives clearer reasons as to why it failed to connect to the TraCI server.
 - Suppressed an issue where `pybullet_utils.pybullet.BulletClient` would cause an error because it was catching a non `BaseException` type.
-
 - Fixed an issue where `AgentInterface.vehicle_type` would not affect agent vehicles when attempting to take over an existing vehicle.
 - Fixed a case where newly created agent vehicles would have a constant `"sedan"` size instead of the size of `AgentInterface.vehicle_type`.
 - Fixed a case where if vehicles are replaced they would not respect controller and vehicle parameters.

smarts/core/sumo_traffic_simulation.py

@@ -74,6 +74,8 @@ class SumoTrafficSimulation(TrafficProvider):
         remove_agents_only_mode:
             Remove only agent vehicles used by SMARTS and not delete other SUMO
             vehicles when the traffic simulation calls to tear-down
+        traci_retries:
+            The number of times to retry acquisition of a TraCI server before erroring.
     """
 
     _HAS_DYNAMIC_ATTRIBUTES = True
@@ -86,7 +88,7 @@ def __init__(
         sumo_port: Optional[int] = None,
         auto_start: bool = True,
         allow_reload: bool = True,
-        debug: bool = True,
+        debug: bool = False,
         remove_agents_only_mode: bool = False,
         traci_retries: Optional[int] = None,
     ):
@@ -205,14 +207,15 @@ def _initialize_traci_conn(self, num_retries=5):
                 base_params=self._base_sumo_load_params(),
                 sumo_binary=sumo_binary,
             )
-            # Ensure there has been enough time for sumo to start
+
             try:
                 while self._traci_conn.viable and not self._traci_conn.connected:
                     try:
                         self._traci_conn.connect(
                             timeout=5,
                             minimum_traci_version=20,
                             minimum_sumo_version=(1, 10, 0),
+                            debug=self._debug,
                         )
                     except traci.exceptions.FatalTraCIError:
                         # Could not connect in time just retry connection
@@ -225,32 +228,28 @@ def _initialize_traci_conn(self, num_retries=5):
                 self._traci_conn.close_traci_and_pipes()
                 continue
             except ConnectionRefusedError:
-                # Some other process owns the port... sumo did not die just retry
-                self._traci_conn.close_traci_and_pipes()
+                # Some other process somehow owns the port... sumo needs to be restarted.
                 continue
             except OSError:
+                # TraCI or SUMO version are not at the minimum required version.
                 raise
             except KeyboardInterrupt:
                 self._log.debug("Keyboard interrupted TraCI connection.")
                 self._traci_conn.close_traci_and_pipes()
                 raise
             break
+        else:
+            exception = traci.exceptions.FatalTraCIError(
+                f"Unable to connect to TraCI server after `{num_retries=}`."
+            )
+            self._handle_traci_exception(exception, actors_relinquishable=False)
+            raise exception
 
         try:
-            assert self._traci_conn is not None
             # It is mandatory to set order when using multiple clients.
             self._traci_conn.setOrder(0)
             self._traci_conn.getVersion()
         except (traci.exceptions.FatalTraCIError, TypeError) as err:
-            logging.error(
-                """Failed to initialize SUMO
-                Your scenario might not be configured correctly or
-                you were trying to initialize many SUMO instances at
-                once and we were not able to assign unique port
-                numbers to all SUMO processes.
-                Check %s for hints""",
-                self._log_file,
-            )
             self._handle_traci_exception(err, actors_relinquishable=False)
             self.teardown()
             raise

smarts/core/utils/sumo.py

@@ -20,6 +20,7 @@
 """Importing this module "redirects" the import to the "real" sumolib. This is available
 for convenience and to reduce code duplication as sumolib lives under SUMO_HOME.
 """
+from __future__ import annotations
 
 import functools
 import inspect
@@ -115,18 +116,15 @@ def __del__(self) -> None:
 
     def connect(
         self,
-        timeout: float = 5,
-        minimum_traci_version=20,
-        minimum_sumo_version=(
-            1,
-            10,
-            0,
-        ),
+        timeout: float,
+        minimum_traci_version: int,
+        minimum_sumo_version: Tuple[int, ...],
+        debug: bool = False,
     ):
         """Attempt a connection with the SUMO process."""
         traci_conn = None
         try:
-            with suppress_output(stdout=False):
+            with suppress_output(stderr=not debug, stdout=False):
                 traci_conn = traci.connect(
                     self._sumo_port,
                     numRetries=max(0, int(20 * timeout)),
@@ -215,7 +213,7 @@ def __safe_close(conn):
                 # TraCI connection is already dead.
                 pass
             except AttributeError:
-                # Socket was destroyed internally by a fatal error somehow.
+                # Socket was destroyed internally, likely due to an error.
                 pass
 
         if self._traci_conn:
@@ -236,14 +234,13 @@ def teardown(self):
 
 
 def _wrap_traci_method(*args, method, sumo_process: TraciConn, **kwargs):
-    # Argument order must be `*args` first so keyword arguments are required for `method` and `sumo_process`.
+    # Argument order must be `*args` first so `method` and `sumo_process` are keyword only arguments.
     try:
         return method(*args, **kwargs)
     except traci.exceptions.FatalTraCIError:
-        # Traci cannot continue
+        # TraCI cannot continue
         sumo_process.close_traci_and_pipes()
         raise
     except traci.exceptions.TraCIException:
-        # Case where SUMO can continue
-        # TAI: consider closing the process even with a non fatal error
+        # Case where TraCI/SUMO can theoretically continue
         raise

smarts/engine.ini

@@ -1,3 +1,4 @@
+; For syntax see https://docs.python.org/3/library/configparser.html#supported-ini-file-structure
 [benchmark]
 [core]
 debug = False