From 0ceb423ccc385e2e4d78b9c8a39f77486c7b5aaa Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 12 Jan 2023 12:25:24 -0800 Subject: [PATCH] Car interface: require fingerprint and FW versions to get params (#26932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * require fingerprint and FW versions * add get_non_essential_params() * comment * all required * classmethod, need to allow subclasses to override _get_params * fix that * fix * fix * wrong fix 🤦 old-commit-hash: b00bc4f57e2c9a3acb9e3b10528b7e30cbde43f1 --- selfdrive/car/docs.py | 2 +- selfdrive/car/interfaces.py | 13 +++++++------ selfdrive/car/tests/test_car_interfaces.py | 2 +- selfdrive/car/tests/test_fw_fingerprint.py | 2 +- selfdrive/car/tests/test_lateral_limits.py | 2 +- selfdrive/controls/lib/tests/test_latcontrol.py | 2 +- selfdrive/controls/lib/tests/test_vehicle_model.py | 2 +- selfdrive/controls/tests/test_state_machine.py | 2 +- selfdrive/debug/cycle_alerts.py | 2 +- selfdrive/debug/internal/test_paramsd.py | 2 +- selfdrive/test/longitudinal_maneuvers/plant.py | 2 +- selfdrive/test/process_replay/process_replay.py | 2 +- 12 files changed, 18 insertions(+), 17 deletions(-) diff --git a/selfdrive/car/docs.py b/selfdrive/car/docs.py index 03313e2ff..bc03619d0 100755 --- a/selfdrive/car/docs.py +++ b/selfdrive/car/docs.py @@ -29,7 +29,7 @@ def get_all_car_info() -> List[CarInfo]: all_car_info: List[CarInfo] = [] footnotes = get_all_footnotes() for model, car_info in get_interface_attr("CAR_INFO", combine_brands=True).items(): - CP = interfaces[model][0].get_params(model, fingerprint=gen_empty_fingerprint(), car_fw=[car.CarParams.CarFw(ecu="unknown")]) + CP = interfaces[model][0].get_params(model, fingerprint=gen_empty_fingerprint(), car_fw=[car.CarParams.CarFw(ecu="unknown")], experimental_long=False) if CP.dashcamOnly or car_info is None: continue diff --git a/selfdrive/car/interfaces.py b/selfdrive/car/interfaces.py index ad1fd22b9..7192f5252 100644 --- a/selfdrive/car/interfaces.py +++ b/selfdrive/car/interfaces.py @@ -88,13 +88,14 @@ class CarInterfaceBase(ABC): return ACCEL_MIN, ACCEL_MAX @classmethod - def get_params(cls, candidate: str, fingerprint: Optional[Dict[int, Dict[int, int]]] = None, car_fw: Optional[List[car.CarParams.CarFw]] = None, experimental_long: bool = False): - if fingerprint is None: - fingerprint = gen_empty_fingerprint() - - if car_fw is None: - car_fw = list() + def get_non_essential_params(cls, candidate: str): + """ + Parameters essential to controlling the car may be incomplete or wrong without FW versions or fingerprints. + """ + return cls.get_params(candidate, gen_empty_fingerprint(), list(), False) + @classmethod + def get_params(cls, candidate: str, fingerprint: Dict[int, Dict[int, int]], car_fw: List[car.CarParams.CarFw], experimental_long: bool): ret = CarInterfaceBase.get_std_params(candidate) ret = cls._get_params(ret, candidate, fingerprint, car_fw, experimental_long) diff --git a/selfdrive/car/tests/test_car_interfaces.py b/selfdrive/car/tests/test_car_interfaces.py index 11e7be7f4..deb0454b9 100755 --- a/selfdrive/car/tests/test_car_interfaces.py +++ b/selfdrive/car/tests/test_car_interfaces.py @@ -25,7 +25,7 @@ class TestCarInterfaces(unittest.TestCase): car_fw = [] - car_params = CarInterface.get_params(car_name, fingerprints, car_fw) + car_params = CarInterface.get_params(car_name, fingerprints, car_fw, experimental_long=False) car_interface = CarInterface(car_params, CarController, CarState) assert car_params assert car_interface diff --git a/selfdrive/car/tests/test_fw_fingerprint.py b/selfdrive/car/tests/test_fw_fingerprint.py index 2e4310385..e9f23145c 100755 --- a/selfdrive/car/tests/test_fw_fingerprint.py +++ b/selfdrive/car/tests/test_fw_fingerprint.py @@ -67,7 +67,7 @@ class TestFwFingerprint(unittest.TestCase): blacklisted_addrs = (0x7c4, 0x7d0) # includes A/C ecu and an unknown ecu for car_model, ecus in FW_VERSIONS.items(): with self.subTest(car_model=car_model): - CP = interfaces[car_model][0].get_params(car_model) + CP = interfaces[car_model][0].get_non_essential_params(car_model) if CP.carName == 'subaru': for ecu in ecus.keys(): self.assertNotIn(ecu[1], blacklisted_addrs, f'{car_model}: Blacklisted ecu: (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])})') diff --git a/selfdrive/car/tests/test_lateral_limits.py b/selfdrive/car/tests/test_lateral_limits.py index a82c20ce0..3efdd7404 100755 --- a/selfdrive/car/tests/test_lateral_limits.py +++ b/selfdrive/car/tests/test_lateral_limits.py @@ -40,7 +40,7 @@ class TestLateralLimits(unittest.TestCase): @classmethod def setUpClass(cls): CarInterface, _, _ = interfaces[cls.car_model] - CP = CarInterface.get_params(cls.car_model) + CP = CarInterface.get_non_essential_params(cls.car_model) if CP.dashcamOnly: raise unittest.SkipTest("Platform is behind dashcamOnly") diff --git a/selfdrive/controls/lib/tests/test_latcontrol.py b/selfdrive/controls/lib/tests/test_latcontrol.py index f15ab2fa5..993774f71 100755 --- a/selfdrive/controls/lib/tests/test_latcontrol.py +++ b/selfdrive/controls/lib/tests/test_latcontrol.py @@ -20,7 +20,7 @@ class TestLatControl(unittest.TestCase): @parameterized.expand([(HONDA.CIVIC, LatControlPID), (TOYOTA.RAV4, LatControlTorque), (TOYOTA.PRIUS, LatControlINDI), (NISSAN.LEAF, LatControlAngle)]) def test_saturation(self, car_name, controller): CarInterface, CarController, CarState = interfaces[car_name] - CP = CarInterface.get_params(car_name) + CP = CarInterface.get_non_essential_params(car_name) CI = CarInterface(CP, CarController, CarState) VM = VehicleModel(CP) diff --git a/selfdrive/controls/lib/tests/test_vehicle_model.py b/selfdrive/controls/lib/tests/test_vehicle_model.py index 3e08cb0aa..03d97a7e3 100755 --- a/selfdrive/controls/lib/tests/test_vehicle_model.py +++ b/selfdrive/controls/lib/tests/test_vehicle_model.py @@ -12,7 +12,7 @@ from selfdrive.controls.lib.vehicle_model import VehicleModel, dyn_ss_sol, creat class TestVehicleModel(unittest.TestCase): def setUp(self): - CP = CarInterface.get_params(CAR.CIVIC) + CP = CarInterface.get_non_essential_params(CAR.CIVIC) self.VM = VehicleModel(CP) def test_round_trip_yaw_rate(self): diff --git a/selfdrive/controls/tests/test_state_machine.py b/selfdrive/controls/tests/test_state_machine.py index 8f263a98e..d5f468f21 100755 --- a/selfdrive/controls/tests/test_state_machine.py +++ b/selfdrive/controls/tests/test_state_machine.py @@ -31,7 +31,7 @@ class TestStateMachine(unittest.TestCase): def setUp(self): CarInterface, CarController, CarState = interfaces["mock"] - CP = CarInterface.get_params("mock") + CP = CarInterface.get_non_essential_params("mock") CI = CarInterface(CP, CarController, CarState) self.controlsd = Controls(CI=CI) diff --git a/selfdrive/debug/cycle_alerts.py b/selfdrive/debug/cycle_alerts.py index b40c8e304..71c7b34be 100755 --- a/selfdrive/debug/cycle_alerts.py +++ b/selfdrive/debug/cycle_alerts.py @@ -51,7 +51,7 @@ def cycle_alerts(duration=200, is_metric=False): cameras = ['roadCameraState', 'wideRoadCameraState', 'driverCameraState'] CS = car.CarState.new_message() - CP = CarInterface.get_params("HONDA CIVIC 2016") + CP = CarInterface.get_non_essential_params("HONDA CIVIC 2016") sm = messaging.SubMaster(['deviceState', 'pandaStates', 'roadCameraState', 'modelV2', 'liveCalibration', 'driverMonitoringState', 'longitudinalPlan', 'lateralPlan', 'liveLocationKalman', 'managerState'] + cameras) diff --git a/selfdrive/debug/internal/test_paramsd.py b/selfdrive/debug/internal/test_paramsd.py index 3d8e422c3..81524496f 100755 --- a/selfdrive/debug/internal/test_paramsd.py +++ b/selfdrive/debug/internal/test_paramsd.py @@ -20,7 +20,7 @@ T_SIM = 5 * 60 # s DT = 0.01 -CP = CarInterface.get_params(CAR.CIVIC) +CP = CarInterface.get_non_essential_params(CAR.CIVIC) VM = VehicleModel(CP) x, y = 0, 0 # m, m diff --git a/selfdrive/test/longitudinal_maneuvers/plant.py b/selfdrive/test/longitudinal_maneuvers/plant.py index 59cb0e1fe..bd0556aa0 100755 --- a/selfdrive/test/longitudinal_maneuvers/plant.py +++ b/selfdrive/test/longitudinal_maneuvers/plant.py @@ -49,7 +49,7 @@ class Plant: from selfdrive.car.honda.values import CAR from selfdrive.car.honda.interface import CarInterface - self.planner = LongitudinalPlanner(CarInterface.get_params(CAR.CIVIC), init_v=self.speed) + self.planner = LongitudinalPlanner(CarInterface.get_non_essential_params(CAR.CIVIC), init_v=self.speed) @property def current_time(self): diff --git a/selfdrive/test/process_replay/process_replay.py b/selfdrive/test/process_replay/process_replay.py index c5465fc02..22ec09928 100755 --- a/selfdrive/test/process_replay/process_replay.py +++ b/selfdrive/test/process_replay/process_replay.py @@ -180,7 +180,7 @@ def fingerprint(msgs, fsm, can_sock, fingerprint): def get_car_params(msgs, fsm, can_sock, fingerprint): if fingerprint: CarInterface, _, _ = interfaces[fingerprint] - CP = CarInterface.get_params(fingerprint) + CP = CarInterface.get_non_essential_params(fingerprint) else: can = FakeSocket(wait=False) sendcan = FakeSocket(wait=False)