Fix bug where instance attach properties were actually attached to th…

…e class.

smarts/core/sensor_manager.py

@@ -44,6 +44,7 @@ class SensorManager:
 
     def __init__(self):
         self._logger = logging.getLogger(self.__class__.__name__)
+        self._logger.setLevel(logging.INFO)
         self._sensors: Dict[str, Sensor] = {}
 
         # {actor_id: <SensorState>}
@@ -184,7 +185,7 @@ def add_sensor_state(self, actor_id: str, sensor_state: SensorState):
     def remove_sensor_state_by_actor_id(self, actor_id: str):
         """Add a sensor state associated with a given actor."""
         self._logger.info("Sensor state removed for actor '%s'.", actor_id)
-        del self._sensor_states[actor_id]
+        return self._sensor_states.pop(actor_id, None)
 
     def remove_actor_sensors_by_actor_id(
         self, actor_id: str, schedule_teardown: bool = True

smarts/core/tests/test_vehicle.py

@@ -20,20 +20,20 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 import math
+from functools import partial
 
 import numpy as np
 import pytest
 
 from smarts.core.chassis import BoxChassis
 from smarts.core.coordinates import Dimensions, Heading, Pose
 from smarts.core.utils import pybullet
-from smarts.core.utils.pybullet import bullet_client as bc
 from smarts.core.vehicle import VEHICLE_CONFIGS, Vehicle, VehicleState
 
 
 @pytest.fixture
 def bullet_client():
-    client = bc.BulletClient(pybullet.DIRECT)
+    client = pybullet.SafeBulletClient(pybullet.DIRECT)
     yield client
     client.disconnect()
 
@@ -141,3 +141,39 @@ def test_vehicle_bounding_box(bullet_client):
         vehicle.bounding_box, [[0.5, 2.5], (1.5, 2.5), (1.5, -0.5), (0.5, -0.5)]
     ):
         assert np.array_equal(coordinates[0], coordinates[1])
+
+
+def validate_vehicle(vehicle: Vehicle):
+    def check_attr(sensor_control_name):
+        if hasattr(vehicle, sensor_control_name):
+            sensor_attach = getattr(vehicle, sensor_control_name)
+            if sensor_attach is None:
+                return
+            if isinstance(sensor_attach, property):
+                sensor_attach = sensor_attach.fget
+            assert isinstance(sensor_attach, partial)
+            assert (
+                vehicle.id == sensor_attach.keywords["self"].id
+            ), f"{vehicle.id} | {sensor_attach.keywords['self'].id}"
+
+    for sensor_name in vehicle.sensor_names:
+        check_attr(f"attach_{sensor_name}")
+        check_attr(f"detach_{sensor_name}")
+
+
+def test_vehicle_meta_methods(bullet_client):
+    pose = Pose.from_center((1, 1, 0), Heading(0))
+    chassis = BoxChassis(
+        pose=pose,
+        speed=0,
+        dimensions=Dimensions(length=3, width=1, height=1),
+        bullet_client=bullet_client,
+    )
+
+    for i in range(2):
+        vehicle = Vehicle(
+            id=f"vehicle-{i}",
+            chassis=chassis,
+            vehicle_config_type="passenger",
+        )
+        validate_vehicle(vehicle)

smarts/core/utils/class_factory.py

@@ -115,15 +115,15 @@ def find_factory(self, locator):
                 # Import the module so that the agent may register itself in the index
                 # it is assumed that a `register(name=..., entry_point=...)` exists in the target module.
                 module = importlib.import_module(mod_name)
-            except ImportError:
+            except ImportError as exc:
                 import sys
 
                 raise ImportError(
                     f"Ensure that `{mod_name}` module can be found from your "
                     f"PYTHONPATH and name=`{locator}` exists (e.g. was registered "
-                    "manually or downloaded.\n"
+                    "manually or downloaded).\n"
                     f"`PYTHONPATH`: `{sys.path}`"
-                )
+                ) from exc
         else:
             # There is no module component.
             name = mod_name
@@ -132,8 +132,8 @@ def find_factory(self, locator):
             # See if `register()` has been called.
             # return the builder if it exists.
             return self.index[name]
-        except KeyError:
-            raise NameError(f"Locator not registered in lookup: {locator}")
+        except KeyError as exc:
+            raise NameError(f"Locator not registered in lookup: {locator}") from exc
 
     def make(self, locator, **kwargs):
         """Calls the factory with `locator` name key supplying the keyword arguments as argument

smarts/core/vehicle.py

@@ -17,6 +17,8 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
+from __future__ import annotations
+
 import importlib.resources as pkg_resources
 import logging
 import os
@@ -59,7 +61,7 @@ class Vehicle:
     """Represents a single vehicle."""
 
     _HAS_DYNAMIC_ATTRIBUTES = True  # dynamic pytype attribute
-    _sensor_names = [
+    _sensor_names = (
         "ogm_sensor",
         "rgb_sensor",
         "lidar_sensor",
@@ -73,7 +75,7 @@ class Vehicle:
         "lane_position_sensor",
         "via_sensor",
         "signals_sensor",
-    ]
+    )
 
     def __init__(
         self,
@@ -262,6 +264,12 @@ def valid(self) -> bool:
         """Check if the vehicle still `exists` and is still operable."""
         return self._initialized
 
+    @classmethod
+    @property
+    def sensor_names(cls) -> Tuple[str]:
+        """The names of the sensors that are potentially available to this vehicle."""
+        return cls._sensor_names
+
     @staticmethod
     @lru_cache(maxsize=None)
     def vehicle_urdf_path(vehicle_type: str, override_path: Optional[str]) -> str:
@@ -413,11 +421,12 @@ def build_social_vehicle(sim, vehicle_id, vehicle_state: VehicleState) -> "Vehic
             dimensions=dims,
             bullet_client=sim.bc,
         )
-        return Vehicle(
+        vehicle = Vehicle(
             id=vehicle_id,
             chassis=chassis,
             vehicle_config_type=vehicle_state.vehicle_config_type,
         )
+        return vehicle
 
     @classmethod
     def attach_sensors_to_vehicle(
@@ -634,13 +643,16 @@ def attach_sensor(self, sensor, sensor_name):
         (for example, for observation collection from history vehicles),
         but if that vehicle gets hijacked, we want to use the sensors
         specified by the hijacking agent's interface."""
-        self.detach_sensor(sensor_name)
-        self._log.debug("replacing existing %s on vehicle %s", sensor_name, self.id)
+        detach = getattr(self, f"detach_{sensor_name}")
+        if detach:
+            self.detach_sensor(sensor_name)
+        self._log.debug("Replaced existing %s on vehicle %s", sensor_name, self.id)
         setattr(self, f"_{sensor_name}", sensor)
         self._sensors[sensor_name] = sensor
 
     def detach_sensor(self, sensor_name):
         """Detach a sensor by name."""
+        self._log.debug("Removed existing %s on vehicle %s", sensor_name, self.id)
         sensor = getattr(self, f"_{sensor_name}", None)
         if sensor is not None:
             setattr(self, f"_{sensor_name}", None)
@@ -655,48 +667,61 @@ def subscribed_to(self, sensor_name):
     def sensor_property(self, sensor_name):
         """Call a sensor by name."""
         sensor = getattr(self, f"_{sensor_name}", None)
-        assert sensor is not None, f"{sensor_name} is not attached to {self.id}"
+        assert sensor is not None, f"'{sensor_name}' is not attached to '{self.id}'"
         return sensor
 
-    def _meta_create_sensor_functions(self):
-        # Bit of metaprogramming to make sensor creation more DRY
-        sensor_names = self._sensor_names
-        for sensor_name in sensor_names:
-            setattr(Vehicle, f"_{sensor_name}", None)
+    def _meta_create_instance_sensor_functions(self):
+        for sensor_name in Vehicle._sensor_names:
+            setattr(self, f"_{sensor_name}", None)
             setattr(
-                Vehicle,
+                self,
                 f"attach_{sensor_name}",
-                partial(self.attach_sensor, sensor_name=sensor_name),
+                partial(
+                    self.__class__.attach_sensor, self=self, sensor_name=sensor_name
+                ),
             )
             setattr(
-                Vehicle,
+                self,
                 f"detach_{sensor_name}",
-                partial(self.detach_sensor, sensor_name=sensor_name),
+                partial(
+                    self.__class__.detach_sensor, self=self, sensor_name=sensor_name
+                ),
             )
+
+    @classmethod
+    @lru_cache(1)
+    def _meta_create_class_sensor_functions(cls):
+        for sensor_name in cls._sensor_names:
             setattr(
-                Vehicle,
+                cls,
                 f"subscribed_to_{sensor_name}",
                 property(
-                    partial(self.__class__.subscribed_to, sensor_name=sensor_name)
+                    partial(cls.subscribed_to, sensor_name=sensor_name)
                 ),
             )
             setattr(
                 Vehicle,
                 f"{sensor_name}",
                 property(
-                    partial(self.__class__.sensor_property, sensor_name=sensor_name)
+                    partial(cls.sensor_property, sensor_name=sensor_name)
                 ),
             )
 
         def detach_all_sensors_from_vehicle(vehicle):
             sensors = []
-            for sensor_name in sensor_names:
+            for sensor_name in cls._sensor_names:
                 detach_sensor_func = getattr(vehicle, f"detach_{sensor_name}")
                 sensors.append(detach_sensor_func())
             return sensors
 
         setattr(
-            Vehicle,
+            cls,
             "detach_all_sensors_from_vehicle",
             staticmethod(detach_all_sensors_from_vehicle),
         )
+
+    def _meta_create_sensor_functions(self):
+        # Bit of metaprogramming to make sensor creation more DRY
+        self._meta_create_instance_sensor_functions()
+        self._meta_create_class_sensor_functions()
+

smarts/core/vehicle_index.py

@@ -548,6 +548,8 @@ def relinquish_agent_control(
         # pytype: disable=attribute-error
         Vehicle.detach_all_sensors_from_vehicle(vehicle)
         # pytype: enable=attribute-error
+        sim.sensor_manager.remove_actor_sensors_by_actor_id(vehicle_id)
+        sim.sensor_manager.remove_sensor_state_by_actor_id(vehicle_id)
 
         vehicle = self._vehicles[v_id]
         box_chassis = BoxChassis(