From 0251742079b206ce40a3b4d8ae9cec1381494d4a Mon Sep 17 00:00:00 2001 From: Justin Newberry Date: Tue, 26 Sep 2023 13:44:18 -0700 Subject: [PATCH 1/8] CI: use "slow" marker for pytest (#30063) * skip to mark * fix quotes --- .github/workflows/selfdrive_tests.yaml | 3 +-- pyproject.toml | 2 +- selfdrive/car/tests/test_models.py | 5 ++--- selfdrive/locationd/test/test_laikad.py | 5 ++--- selfdrive/test/helpers.py | 1 - 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/.github/workflows/selfdrive_tests.yaml b/.github/workflows/selfdrive_tests.yaml index 2aae1350f3..b46b720b71 100644 --- a/.github/workflows/selfdrive_tests.yaml +++ b/.github/workflows/selfdrive_tests.yaml @@ -268,8 +268,7 @@ jobs: - name: Run unit tests timeout-minutes: 15 run: | - ${{ env.RUN }} "export SKIP_LONG_TESTS=1 && \ - $PYTEST -n auto --dist=loadscope --timeout 30 -o cpp_files=test_* && \ + ${{ env.RUN }} "$PYTEST -n auto --dist=loadscope --timeout 30 -o cpp_files=test_* -m 'not slow' && \ ./selfdrive/ui/tests/create_test_translations.sh && \ QT_QPA_PLATFORM=offscreen ./selfdrive/ui/tests/test_translations && \ ./selfdrive/ui/tests/test_translations.py && \ diff --git a/pyproject.toml b/pyproject.toml index d5302c1184..9cdc249d12 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ addopts = "--ignore=openpilot/ --ignore=cereal/ --ignore=opendbc/ --ignore=panda python_files = "test_*.py" #timeout = "30" # you get this long by default markers = [ - "parallel: mark tests as parallelizable (tests with no global state, so can be run in parallel)" + "slow: tests that take awhile to run and can be skipped with -m 'not slow'" ] testpaths = [ "common", diff --git a/selfdrive/car/tests/test_models.py b/selfdrive/car/tests/test_models.py index e6460d491e..b9622437a0 100755 --- a/selfdrive/car/tests/test_models.py +++ b/selfdrive/car/tests/test_models.py @@ -2,6 +2,7 @@ import capnp import os import importlib +import pytest import unittest from collections import defaultdict, Counter from typing import List, Optional, Tuple @@ -23,7 +24,6 @@ from openpilot.tools.lib.logreader import LogReader from openpilot.tools.lib.route import Route, SegmentName, RouteName from panda.tests.libpanda import libpanda_py -from openpilot.selfdrive.test.helpers import SKIP_ENV_VAR EventName = car.CarEvent.EventName PandaType = log.PandaState.PandaType @@ -67,7 +67,7 @@ def get_test_cases() -> List[Tuple[str, Optional[CarTestRoute]]]: return test_cases - +@pytest.mark.slow class TestCarModelBase(unittest.TestCase): car_model: Optional[str] = None test_route: Optional[CarTestRoute] = None @@ -76,7 +76,6 @@ class TestCarModelBase(unittest.TestCase): can_msgs: List[capnp.lib.capnp._DynamicStructReader] elm_frame: Optional[int] - @unittest.skipIf(SKIP_ENV_VAR in os.environ, f"Long running test skipped. Unset {SKIP_ENV_VAR} to run") @classmethod def setUpClass(cls): if cls.__name__ == 'TestCarModel' or cls.__name__.endswith('Base'): diff --git a/selfdrive/locationd/test/test_laikad.py b/selfdrive/locationd/test/test_laikad.py index 2c4a8a406d..db8802ed36 100755 --- a/selfdrive/locationd/test/test_laikad.py +++ b/selfdrive/locationd/test/test_laikad.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -import os +import pytest import time import unittest from cereal import log @@ -19,7 +19,6 @@ from openpilot.selfdrive.test.openpilotci import get_url from openpilot.tools.lib.logreader import LogReader from openpilot.selfdrive.test.process_replay.process_replay import get_process_config, replay_process -from openpilot.selfdrive.test.helpers import SKIP_ENV_VAR GPS_TIME_PREDICTION_ORBITS_RUSSIAN_SRC = GPSTime.from_datetime(datetime(2022, month=1, day=29, hour=12)) UBLOX_TEST_ROUTE = "4cf7a6ad03080c90|2021-09-29--13-46-36" @@ -94,7 +93,7 @@ def get_measurement_mock(gpstime, sat_ephemeris): return meas -@unittest.skipIf(SKIP_ENV_VAR in os.environ, f"Laika test skipped since it's long and not currently used. Unset {SKIP_ENV_VAR} to run") +@pytest.mark.slow class TestLaikad(unittest.TestCase): @classmethod diff --git a/selfdrive/test/helpers.py b/selfdrive/test/helpers.py index a6190b20b5..552070f024 100644 --- a/selfdrive/test/helpers.py +++ b/selfdrive/test/helpers.py @@ -9,7 +9,6 @@ from openpilot.selfdrive.manager.process_config import managed_processes from openpilot.system.hardware import PC from openpilot.system.version import training_version, terms_version -SKIP_ENV_VAR = "SKIP_LONG_TESTS" def set_params_enabled(): os.environ['PASSIVE'] = "0" From 6d5f4f149d819eed6a56112e9fba277b62613ff1 Mon Sep 17 00:00:00 2001 From: Vivek Aithal Date: Tue, 26 Sep 2023 15:31:40 -0700 Subject: [PATCH 2/8] locationd: Replace allAliveAndValid with allValid (#30064) * replace allaliveandvalid check with allvalid, all necessary time checks are already used * update refs --- selfdrive/locationd/locationd.cc | 2 +- selfdrive/test/process_replay/ref_commit | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/selfdrive/locationd/locationd.cc b/selfdrive/locationd/locationd.cc index 600ba46853..80882fc951 100644 --- a/selfdrive/locationd/locationd.cc +++ b/selfdrive/locationd/locationd.cc @@ -720,7 +720,7 @@ int Localizer::locationd_thread() { // 100Hz publish for notcars, 20Hz for cars const char* trigger_msg = sm["carParams"].getCarParams().getNotCar() ? "accelerometer" : "cameraOdometry"; if (sm.updated(trigger_msg)) { - bool inputsOK = sm.allAliveAndValid() && this->are_inputs_ok(); + bool inputsOK = sm.allValid() && this->are_inputs_ok(); bool gpsOK = this->is_gps_ok(); bool sensorsOK = sm.allAliveAndValid({"accelerometer", "gyroscope"}); diff --git a/selfdrive/test/process_replay/ref_commit b/selfdrive/test/process_replay/ref_commit index c6a2d58fec..ecf0412028 100644 --- a/selfdrive/test/process_replay/ref_commit +++ b/selfdrive/test/process_replay/ref_commit @@ -1 +1 @@ -b421ff389ce720b70a36dd2b3510af54eb484b5f \ No newline at end of file +7553d9123f418fd81ef811df4e130ab153ac489a \ No newline at end of file From fa06a691017f6f1b333bb2e6bcb99bf4361bcc33 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 26 Sep 2023 16:11:26 -0700 Subject: [PATCH 3/8] add extra debug info to onroad test (#30067) --- selfdrive/test/test_time_to_onroad.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/selfdrive/test/test_time_to_onroad.py b/selfdrive/test/test_time_to_onroad.py index 429feca344..501ce16b64 100755 --- a/selfdrive/test/test_time_to_onroad.py +++ b/selfdrive/test/test_time_to_onroad.py @@ -16,7 +16,7 @@ def test_time_to_onroad(): proc = subprocess.Popen(["python", manager_path]) start_time = time.monotonic() - sm = messaging.SubMaster(['controlsState', 'deviceState']) + sm = messaging.SubMaster(['controlsState', 'deviceState', 'carEvents']) try: # wait for onroad with Timeout(20, "timed out waiting to go onroad"): @@ -38,7 +38,7 @@ def test_time_to_onroad(): # once we're enageable, must be for the next few seconds for _ in range(500): sm.update(100) - assert sm['controlsState'].engageable + assert sm['controlsState'].engageable, f"events: {sm['carEvents']}" finally: proc.terminate() if proc.wait(60) is None: From 0e02d8b8d154024c569954bbad430bb5e287d003 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 26 Sep 2023 16:18:41 -0700 Subject: [PATCH 4/8] CI: reduce master-ci build frequency --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c57a17bbd2..2bb22fecb2 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,7 +1,7 @@ name: release on: schedule: - - cron: '0 * * * *' + - cron: '0 10 * * *' workflow_dispatch: jobs: From 7073882f87b1ecc4934fd42ff2ade4c8e9fed6e9 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 26 Sep 2023 16:41:24 -0700 Subject: [PATCH 5/8] HKG tests: assert standard FW requests in database (#30061) * test to help catch random FW in PRs * just fwdCamera * better? --- selfdrive/car/hyundai/tests/test_hyundai.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/selfdrive/car/hyundai/tests/test_hyundai.py b/selfdrive/car/hyundai/tests/test_hyundai.py index eba548bfbd..f0c6b05c00 100755 --- a/selfdrive/car/hyundai/tests/test_hyundai.py +++ b/selfdrive/car/hyundai/tests/test_hyundai.py @@ -6,7 +6,8 @@ from cereal import car from openpilot.selfdrive.car.fw_versions import build_fw_dict from openpilot.selfdrive.car.hyundai.values import CAMERA_SCC_CAR, CANFD_CAR, CAN_GEARS, CAR, CHECKSUM, DATE_FW_ECUS, \ HYBRID_CAR, EV_CAR, FW_QUERY_CONFIG, FW_VERSIONS, LEGACY_SAFETY_MODE_CAR, \ - UNSUPPORTED_LONGITUDINAL_CAR, PLATFORM_CODE_ECUS, get_platform_codes + UNSUPPORTED_LONGITUDINAL_CAR, PLATFORM_CODE_ECUS, HYUNDAI_VERSION_REQUEST_LONG, \ + get_platform_codes Ecu = car.CarParams.Ecu ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()} @@ -71,6 +72,20 @@ class TestHyundaiFingerprint(unittest.TestCase): part = code.split(b"-")[1] self.assertFalse(part.startswith(b'CW'), "Car has bad part number") + def test_correct_ecu_response_database(self): + """ + Assert standard responses for certain ECUs, since they can + respond to multiple queries with different data + """ + expected_fw_prefix = HYUNDAI_VERSION_REQUEST_LONG[1:] + for car_model, ecus in FW_VERSIONS.items(): + with self.subTest(car_model=car_model): + for ecu, fws in ecus.items(): + # TODO: enable for Ecu.fwdRadar, Ecu.abs, Ecu.eps, Ecu.transmission + if ecu[0] in (Ecu.fwdCamera,): + self.assertTrue(all(fw.startswith(expected_fw_prefix) for fw in fws), + f"FW from unexpected request in database: {(ecu, fws)}") + @settings(max_examples=100) @given(data=st.data()) def test_platform_codes_fuzzy_fw(self, data): From a4aa801fbe924ace2ebd493eeac7eb0c596f81b5 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 26 Sep 2023 16:47:21 -0700 Subject: [PATCH 6/8] simplify sensor alert (#29824) * simplify * update proc replay --- selfdrive/controls/controlsd.py | 21 ++++++++++--------- selfdrive/controls/lib/events.py | 2 +- .../test/process_replay/process_replay.py | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/selfdrive/controls/controlsd.py b/selfdrive/controls/controlsd.py index 770c39d3f2..4f3b6a9128 100755 --- a/selfdrive/controls/controlsd.py +++ b/selfdrive/controls/controlsd.py @@ -36,7 +36,6 @@ LANE_DEPARTURE_THRESHOLD = 0.1 REPLAY = "REPLAY" in os.environ SIMULATION = "SIMULATION" in os.environ TESTING_CLOSET = "TESTING_CLOSET" in os.environ -NOSENSOR = "NOSENSOR" in os.environ IGNORE_PROCESSES = {"loggerd", "encoderd", "statsd"} ThermalStatus = log.DeviceState.ThermalStatus @@ -69,6 +68,7 @@ class Controls: self.pm = messaging.PubMaster(['sendcan', 'controlsState', 'carState', 'carControl', 'carEvents', 'carParams']) + self.sensor_packets = ["accelerometer", "gyroscope"] self.camera_packets = ["roadCameraState", "driverCameraState", "wideRoadCameraState"] self.can_sock = can_sock @@ -81,12 +81,13 @@ class Controls: self.params = Params() self.sm = sm if self.sm is None: - ignore = ['testJoystick'] + ignore = self.sensor_packets + ['testJoystick'] if SIMULATION: ignore += ['driverCameraState', 'managerState'] self.sm = messaging.SubMaster(['deviceState', 'pandaStates', 'peripheralState', 'modelV2', 'liveCalibration', 'driverMonitoringState', 'longitudinalPlan', 'lateralPlan', 'liveLocationKalman', - 'managerState', 'liveParameters', 'radarState', 'liveTorqueParameters', 'testJoystick'] + self.camera_packets, + 'managerState', 'liveParameters', 'radarState', 'liveTorqueParameters', + 'testJoystick'] + self.camera_packets + self.sensor_packets, ignore_alive=ignore, ignore_avg_freq=['radarState', 'testJoystick']) if CI is None: @@ -380,14 +381,15 @@ class Controls: self.events.add(EventName.posenetInvalid) if not self.sm['liveLocationKalman'].deviceStable: self.events.add(EventName.deviceFalling) - if not (self.sm['liveParameters'].sensorValid or self.sm['liveLocationKalman'].sensorsOK): - if self.sm.frame > 5 / DT_CTRL: # Give locationd some time to receive sensor inputs - self.events.add(EventName.sensorDataInvalid) if not self.sm['liveLocationKalman'].inputsOK: self.events.add(EventName.locationdTemporaryError) if not self.sm['liveParameters'].valid and not TESTING_CLOSET and (not SIMULATION or REPLAY): self.events.add(EventName.paramsdTemporaryError) + # conservative HW alert. if the data or frequency are off, locationd will throw an error + if any((self.sm.frame - self.sm.rcv_frame[s])*DT_CTRL > 10. for s in self.sensor_packets): + self.events.add(EventName.sensorDataInvalid) + if not REPLAY: # Check for mismatch between openpilot and car's PCM cruise_mismatch = CS.cruiseState.enabled and (not self.enabled or not self.CP.pcmCruise) @@ -415,10 +417,9 @@ class Controls: # TODO: fix simulator if not SIMULATION or REPLAY: - if not NOSENSOR: - if not self.sm['liveLocationKalman'].gpsOK and self.sm['liveLocationKalman'].inputsOK and (self.distance_traveled > 1000): - # Not show in first 1 km to allow for driving out of garage. This event shows after 5 minutes - self.events.add(EventName.noGps) + if not self.sm['liveLocationKalman'].gpsOK and self.sm['liveLocationKalman'].inputsOK and (self.distance_traveled > 1000): + # Not show in first 1 km to allow for driving out of garage. This event shows after 5 minutes + self.events.add(EventName.noGps) if self.sm['modelV2'].frameDropPerc > 20: self.events.add(EventName.modeldLagging) diff --git a/selfdrive/controls/lib/events.py b/selfdrive/controls/lib/events.py index a00b8277de..aeeed47e6b 100755 --- a/selfdrive/controls/lib/events.py +++ b/selfdrive/controls/lib/events.py @@ -688,7 +688,7 @@ EVENTS: Dict[int, Dict[str, Union[Alert, AlertCallbackType]]] = { EventName.sensorDataInvalid: { ET.PERMANENT: Alert( "Sensor Data Invalid", - "Ensure device is mounted securely", + "Possible Hardware Issue", AlertStatus.normal, AlertSize.mid, Priority.LOWER, VisualAlert.none, AudibleAlert.none, .2, creation_delay=1.), ET.NO_ENTRY: NoEntryAlert("Sensor Data Invalid"), diff --git a/selfdrive/test/process_replay/process_replay.py b/selfdrive/test/process_replay/process_replay.py index 753262af12..913f506fa2 100755 --- a/selfdrive/test/process_replay/process_replay.py +++ b/selfdrive/test/process_replay/process_replay.py @@ -472,7 +472,7 @@ CONFIGS = [ "can", "deviceState", "pandaStates", "peripheralState", "liveCalibration", "driverMonitoringState", "longitudinalPlan", "lateralPlan", "liveLocationKalman", "liveParameters", "radarState", "modelV2", "driverCameraState", "roadCameraState", "wideRoadCameraState", "managerState", - "testJoystick", "liveTorqueParameters" + "testJoystick", "liveTorqueParameters", "accelerometer", "gyroscope" ], subs=["controlsState", "carState", "carControl", "sendcan", "carEvents", "carParams"], ignore=["logMonoTime", "valid", "controlsState.startMonoTime", "controlsState.cumLagMs"], From f18d66fa3ef8c979e79395758672a5bf1d38575c Mon Sep 17 00:00:00 2001 From: Justin Newberry Date: Tue, 26 Sep 2023 17:05:46 -0700 Subject: [PATCH 7/8] Simulator: cleanup old common stuff (#30069) cleanup --- tools/sim/bridge/common.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/sim/bridge/common.py b/tools/sim/bridge/common.py index 8850fe0d22..53e7d07bf6 100644 --- a/tools/sim/bridge/common.py +++ b/tools/sim/bridge/common.py @@ -6,8 +6,6 @@ from multiprocessing import Process, Queue from abc import ABC, abstractmethod from typing import Optional -import cereal.messaging as messaging - from openpilot.common.params import Params from openpilot.common.numpy_fast import clip from openpilot.common.realtime import Ratekeeper @@ -34,11 +32,6 @@ class SimulatorBridge(ABC): self.rk = Ratekeeper(100, None) - msg = messaging.new_message('liveCalibration') - msg.liveCalibration.validBlocks = 20 - msg.liveCalibration.rpyCalib = [0.0, 0.0, 0.0] - self.params.put("CalibrationParams", msg.to_bytes()) - self.dual_camera = arguments.dual_camera self.high_quality = arguments.high_quality @@ -94,8 +87,6 @@ class SimulatorBridge(ABC): for _ in range(20): self.world.tick() - throttle_manual = steer_manual = brake_manual = 0. - while self._keep_alive: throttle_out = steer_out = brake_out = 0.0 throttle_op = steer_op = brake_op = 0.0 From 61c5618f15a477d6aaa8dfaaf9803961db5081e6 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 26 Sep 2023 17:23:10 -0700 Subject: [PATCH 8/8] torqued: useParams field reflects if using torque control (#30059) fix useParams --- selfdrive/locationd/torqued.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfdrive/locationd/torqued.py b/selfdrive/locationd/torqued.py index ddb95e944c..0b624d5790 100755 --- a/selfdrive/locationd/torqued.py +++ b/selfdrive/locationd/torqued.py @@ -114,7 +114,7 @@ class TorqueEstimator: self.offline_friction = 0.0 self.offline_latAccelFactor = 0.0 self.resets = 0.0 - self.use_params = CP.carName in ALLOWED_CARS + self.use_params = CP.carName in ALLOWED_CARS and CP.lateralTuning.which() == 'torque' if CP.lateralTuning.which() == 'torque': self.offline_friction = CP.lateralTuning.torque.friction