From 1a74993ee2365040529fc96b835c35a886b0757e Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 6 Aug 2024 00:10:53 -0700 Subject: [PATCH] selfdrive/car: ban params (#33198) * ban params too with a callback * all sorts of messed up * use cloudlog * consistent order * order * better type hint * format * this is a bit nicer * hmm * fix PLR1704 * no carvin * fix process replay old-commit-hash: 5a1596a32299d6f78d89c89469ab6893b8d5e083 --- .importlinter | 6 ++++++ selfdrive/car/car_helpers.py | 15 ++++++--------- selfdrive/car/card.py | 6 +++++- selfdrive/car/fw_versions.py | 1 - selfdrive/test/process_replay/process_replay.py | 5 +++-- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/.importlinter b/.importlinter index e884230a76..3cd74f2b9b 100644 --- a/.importlinter +++ b/.importlinter @@ -8,6 +8,7 @@ type = forbidden source_modules = openpilot.selfdrive.car forbidden_modules = + openpilot.common.params openpilot.system openpilot.body openpilot.docs @@ -34,4 +35,9 @@ ignore_imports = openpilot.selfdrive.car.tests.test_car_interfaces -> openpilot.selfdrive.controls.lib.longcontrol openpilot.selfdrive.car.tests.test_car_interfaces -> openpilot.selfdrive.controls.lib.latcontrol_torque openpilot.selfdrive.car.tests.test_car_interfaces -> openpilot.selfdrive.controls.lib.latcontrol_pid + openpilot.selfdrive.car.card -> openpilot.common.params + openpilot.selfdrive.car.tests.test_models -> openpilot.common.params + # these two will still live in openpilot, but require some modification + openpilot.selfdrive.car.fw_versions -> openpilot.common.params + openpilot.selfdrive.car.ecu_addrs -> openpilot.common.params unmatched_ignore_imports_alerting = warn diff --git a/selfdrive/car/car_helpers.py b/selfdrive/car/car_helpers.py index 4392a6853a..72c2930f32 100644 --- a/selfdrive/car/car_helpers.py +++ b/selfdrive/car/car_helpers.py @@ -3,7 +3,6 @@ import time from collections.abc import Callable from cereal import car -from openpilot.common.params import Params from openpilot.selfdrive.car import carlog from openpilot.selfdrive.car.interfaces import get_interface_attr from openpilot.selfdrive.car.fingerprints import eliminate_incompatible_cars, all_legacy_fingerprint_cars @@ -90,18 +89,17 @@ def can_fingerprint(next_can: Callable) -> tuple[str | None, dict[int, dict]]: # **** for use live only **** -def fingerprint(logcan, sendcan, set_obd_multiplexing, num_pandas): +def fingerprint(logcan, sendcan, set_obd_multiplexing, num_pandas, cached_params_raw): fixed_fingerprint = os.environ.get('FINGERPRINT', "") skip_fw_query = os.environ.get('SKIP_FW_QUERY', False) disable_fw_cache = os.environ.get('DISABLE_FW_CACHE', False) ecu_rx_addrs = set() - params = Params() start_time = time.monotonic() if not skip_fw_query: - cached_params = params.get("CarParamsCache") - if cached_params is not None: - with car.CarParams.from_bytes(cached_params) as cached_params: + cached_params = None + if cached_params_raw is not None: + with car.CarParams.from_bytes(cached_params_raw) as cached_params: if cached_params.carName == "mock": cached_params = None @@ -135,7 +133,6 @@ def fingerprint(logcan, sendcan, set_obd_multiplexing, num_pandas): # disable OBD multiplexing for CAN fingerprinting and potential ECU knockouts set_obd_multiplexing(False) - params.put_bool("FirmwareQueryDone", True) fw_query_time = time.monotonic() - start_time @@ -169,8 +166,8 @@ def get_car_interface(CP): return CarInterface(CP, CarController, CarState) -def get_car(logcan, sendcan, set_obd_multiplexing, experimental_long_allowed, num_pandas=1): - candidate, fingerprints, vin, car_fw, source, exact_match = fingerprint(logcan, sendcan, set_obd_multiplexing, num_pandas) +def get_car(logcan, sendcan, set_obd_multiplexing, experimental_long_allowed, num_pandas=1, cached_params=None): + candidate, fingerprints, vin, car_fw, source, exact_match = fingerprint(logcan, sendcan, set_obd_multiplexing, num_pandas, cached_params) if candidate is None: carlog.error({"event": "car doesn't match any fingerprints", "fingerprints": repr(fingerprints)}) diff --git a/selfdrive/car/card.py b/selfdrive/car/card.py index 2aa5012dc2..74801ccbeb 100755 --- a/selfdrive/car/card.py +++ b/selfdrive/car/card.py @@ -62,7 +62,11 @@ class Car: experimental_long_allowed = self.params.get_bool("ExperimentalLongitudinalEnabled") num_pandas = len(messaging.recv_one_retry(self.sm.sock['pandaStates']).pandaStates) - self.CI, self.CP = get_car(self.can_sock, self.pm.sock['sendcan'], obd_callback(self.params), experimental_long_allowed, num_pandas) + cached_params = self.params.get("CarParamsCache") + self.CI, self.CP = get_car(self.can_sock, self.pm.sock['sendcan'], obd_callback(self.params), experimental_long_allowed, num_pandas, cached_params) + + # continue onto next fingerprinting step in pandad + self.params.put_bool("FirmwareQueryDone", True) else: self.CI, self.CP = CI, CI.CP diff --git a/selfdrive/car/fw_versions.py b/selfdrive/car/fw_versions.py index ef9a1e24d4..87f5f3defd 100755 --- a/selfdrive/car/fw_versions.py +++ b/selfdrive/car/fw_versions.py @@ -8,7 +8,6 @@ import capnp import panda.python.uds as uds from cereal import car -from openpilot.common.params import Params from openpilot.selfdrive.car import carlog from openpilot.selfdrive.car.ecu_addrs import get_ecu_addrs from openpilot.selfdrive.car.fingerprints import FW_VERSIONS diff --git a/selfdrive/test/process_replay/process_replay.py b/selfdrive/test/process_replay/process_replay.py index 0c9f98e647..4eef771155 100755 --- a/selfdrive/test/process_replay/process_replay.py +++ b/selfdrive/test/process_replay/process_replay.py @@ -348,14 +348,15 @@ def get_car_params_callback(rc, pm, msgs, fingerprint): sendcan = DummySocket() canmsgs = [msg for msg in msgs if msg.which() == "can"] - has_cached_cp = params.get("CarParamsCache") is not None + cached_params = params.get("CarParamsCache") + has_cached_cp = cached_params is not None assert len(canmsgs) != 0, "CAN messages are required for fingerprinting" assert os.environ.get("SKIP_FW_QUERY", False) or has_cached_cp, \ "CarParamsCache is required for fingerprinting. Make sure to keep carParams msgs in the logs." for m in canmsgs[:300]: can.send(m.as_builder().to_bytes()) - _, CP = get_car(can, sendcan, lambda obd: None, Params().get_bool("ExperimentalLongitudinalEnabled")) + _, CP = get_car(can, sendcan, lambda obd: None, Params().get_bool("ExperimentalLongitudinalEnabled"), cached_params=cached_params) if not params.get_bool("DisengageOnAccelerator"): CP.alternativeExperience |= ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS