From 2136e2e45bf083d36b6cfc451fce6bff40dc48da Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 21 Mar 2023 12:24:33 -0700 Subject: [PATCH 01/59] safety tests: add common longitudinal accel class (#1298) * add longitudinal safety test class * rename to accel_cmd_msg * line * do hyundai * set default min and max accel * revert * thanks pylint --- tests/safety/common.py | 34 ++++++++++++++++++++++++++++++ tests/safety/hyundai_common.py | 21 ++++++++---------- tests/safety/test_toyota.py | 27 ++++-------------------- tests/safety/test_volkswagen_pq.py | 14 +----------- 4 files changed, 48 insertions(+), 48 deletions(-) diff --git a/tests/safety/common.py b/tests/safety/common.py index 71bdff0e89..91da3da4d0 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -131,6 +131,40 @@ def test_gas_interceptor_safety_check(self): self.assertEqual(send, self._tx(self._interceptor_gas_cmd(gas))) +class LongitudinalAccelSafetyTest(PandaSafetyTestBase, abc.ABC): + + MIN_ACCEL: float = 2.0 + MAX_ACCEL: float = -3.5 + INACTIVE_ACCEL: float = 0.0 + + @classmethod + def setUpClass(cls): + if cls.__name__ == "LongitudinalAccelSafetyTest": + cls.safety = None + raise unittest.SkipTest + + @abc.abstractmethod + def _accel_msg(self, accel: float): + pass + + def test_accel_actuation_limits(self, stock_longitudinal=False): + limits = ((self.MIN_ACCEL, self.MAX_ACCEL, ALTERNATIVE_EXPERIENCE.DEFAULT), + (self.MIN_ACCEL, self.MAX_ACCEL, ALTERNATIVE_EXPERIENCE.RAISE_LONGITUDINAL_LIMITS_TO_ISO_MAX)) + + for min_accel, max_accel, alternative_experience in limits: + for accel in np.arange(min_accel - 1, max_accel + 1, 0.05): + for controls_allowed in [True, False]: + self.safety.set_controls_allowed(controls_allowed) + self.safety.set_alternative_experience(alternative_experience) + if stock_longitudinal: + should_tx = False + elif controls_allowed: + should_tx = int(min_accel * 1000) <= int(accel * 1000) <= int(max_accel * 1000) + else: + should_tx = np.isclose(accel, self.INACTIVE_ACCEL, atol=0.0001) + self.assertEqual(should_tx, self._tx(self._accel_msg(accel))) + + class TorqueSteeringSafetyTestBase(PandaSafetyTestBase, abc.ABC): MAX_RATE_UP = 0 diff --git a/tests/safety/hyundai_common.py b/tests/safety/hyundai_common.py index dac8e4a617..fde8f77ce2 100644 --- a/tests/safety/hyundai_common.py +++ b/tests/safety/hyundai_common.py @@ -1,6 +1,7 @@ -import numpy as np from typing import Tuple +import unittest +import panda.tests.safety.common as common from panda.tests.libpanda import libpanda_py from panda.tests.safety.common import make_msg @@ -12,8 +13,6 @@ class Buttons: CANCEL = 4 -MAX_ACCEL = 2.0 -MIN_ACCEL = -3.5 PREV_BUTTON_SAMPLES = 8 ENABLE_BUTTONS = (Buttons.RESUME, Buttons.SET, Buttons.CANCEL) @@ -75,12 +74,18 @@ def test_sampling_cruise_buttons(self): self._rx(self._button_msg(Buttons.NONE)) -class HyundaiLongitudinalBase: +class HyundaiLongitudinalBase(common.LongitudinalAccelSafetyTest): # pylint: disable=no-member,abstract-method DISABLED_ECU_UDS_MSG: Tuple[int, int] DISABLED_ECU_ACTUATION_MSG: Tuple[int, int] + @classmethod + def setUpClass(cls): + if cls.__name__ == "HyundaiLongitudinalBase": + cls.safety = None + raise unittest.SkipTest + # override these tests from PandaSafetyTest, hyundai longitudinal uses button enable def test_disable_control_allowed_from_cruise(self): pass @@ -128,14 +133,6 @@ def test_cancel_button(self): self._rx(self._button_msg(Buttons.CANCEL)) self.assertFalse(self.safety.get_controls_allowed()) - def test_accel_safety_check(self): - for controls_allowed in [True, False]: - for accel in np.arange(MIN_ACCEL - 1, MAX_ACCEL + 1, 0.01): - accel = round(accel, 2) # floats might not hit exact boundary conditions without rounding - self.safety.set_controls_allowed(controls_allowed) - send = MIN_ACCEL <= accel <= MAX_ACCEL if controls_allowed else accel == 0 - self.assertEqual(send, self._tx(self._accel_msg(accel)), (controls_allowed, accel)) - def test_tester_present_allowed(self): """ Ensure tester present diagnostic message is allowed to keep ECU knocked out diff --git a/tests/safety/test_toyota.py b/tests/safety/test_toyota.py index 0a6cb8738b..a0ba9c1130 100755 --- a/tests/safety/test_toyota.py +++ b/tests/safety/test_toyota.py @@ -7,10 +7,7 @@ from panda import Panda from panda.tests.libpanda import libpanda_py import panda.tests.safety.common as common -from panda.tests.safety.common import CANPackerPanda, make_msg, ALTERNATIVE_EXPERIENCE - -MAX_ACCEL = 2.0 -MIN_ACCEL = -3.5 +from panda.tests.safety.common import CANPackerPanda, make_msg def interceptor_msg(gas, addr): @@ -22,7 +19,8 @@ def interceptor_msg(gas, addr): return to_send -class TestToyotaSafetyBase(common.PandaSafetyTest, common.InterceptorSafetyTest): +class TestToyotaSafetyBase(common.PandaSafetyTest, common.InterceptorSafetyTest, + common.LongitudinalAccelSafetyTest): TX_MSGS = [[0x283, 0], [0x2E6, 0], [0x2E7, 0], [0x33E, 0], [0x344, 0], [0x365, 0], [0x366, 0], [0x4CB, 0], # DSU bus 0 [0x128, 1], [0x141, 1], [0x160, 1], [0x161, 1], [0x470, 1], # DSU bus 1 @@ -94,23 +92,6 @@ def test_block_aeb(self): msg = libpanda_py.make_CANPacket(0x283, 0, bytes(dat)) self.assertEqual(not bad, self._tx(msg)) - def test_accel_actuation_limits(self, stock_longitudinal=False): - limits = ((MIN_ACCEL, MAX_ACCEL, ALTERNATIVE_EXPERIENCE.DEFAULT), - (MIN_ACCEL, MAX_ACCEL, ALTERNATIVE_EXPERIENCE.RAISE_LONGITUDINAL_LIMITS_TO_ISO_MAX)) - - for min_accel, max_accel, alternative_experience in limits: - for accel in np.arange(min_accel - 1, max_accel + 1, 0.1): - for controls_allowed in [True, False]: - self.safety.set_controls_allowed(controls_allowed) - self.safety.set_alternative_experience(alternative_experience) - if stock_longitudinal: - should_tx = False - elif controls_allowed: - should_tx = int(min_accel * 1000) <= int(accel * 1000) <= int(max_accel * 1000) - else: - should_tx = np.isclose(accel, 0, atol=0.0001) - self.assertEqual(should_tx, self._tx(self._accel_msg(accel))) - # Only allow LTA msgs with no actuation def test_lta_steer_cmd(self): for engaged, req, req2, setme_x64, angle in itertools.product([True, False], @@ -211,7 +192,7 @@ def test_acc_cancel(self): """ for controls_allowed in [True, False]: self.safety.set_controls_allowed(controls_allowed) - for accel in np.arange(MIN_ACCEL - 1, MAX_ACCEL + 1, 0.1): + for accel in np.arange(self.MIN_ACCEL - 1, self.MAX_ACCEL + 1, 0.1): self.assertFalse(self._tx(self._accel_msg(accel))) should_tx = np.isclose(accel, 0, atol=0.0001) self.assertEqual(should_tx, self._tx(self._accel_msg(accel, cancel_req=1))) diff --git a/tests/safety/test_volkswagen_pq.py b/tests/safety/test_volkswagen_pq.py index 563d5de011..be71f1d7e7 100755 --- a/tests/safety/test_volkswagen_pq.py +++ b/tests/safety/test_volkswagen_pq.py @@ -1,5 +1,4 @@ #!/usr/bin/env python3 -import numpy as np import unittest from panda import Panda from panda.tests.libpanda import libpanda_py @@ -17,8 +16,6 @@ MSG_ACC_GRA_ANZEIGE = 0x56A # TX by OP, ACC HUD MSG_LDW_1 = 0x5BE # TX by OP, Lane line recognition and text alerts -MAX_ACCEL = 2.0 -MIN_ACCEL = -3.5 class TestVolkswagenPqSafety(common.PandaSafetyTest, common.DriverTorqueSteeringSafetyTest): cruise_engaged = False @@ -142,7 +139,7 @@ def test_spam_cancel_safety_check(self): self.assertTrue(self._tx(self._button_msg(resume=True))) -class TestVolkswagenPqLongSafety(TestVolkswagenPqSafety): +class TestVolkswagenPqLongSafety(TestVolkswagenPqSafety, common.LongitudinalAccelSafetyTest): TX_MSGS = [[MSG_HCA_1, 0], [MSG_LDW_1, 0], [MSG_ACC_SYSTEM, 0], [MSG_ACC_GRA_ANZEIGE, 0]] FWD_BLACKLISTED_ADDRS = {2: [MSG_HCA_1, MSG_LDW_1, MSG_ACC_SYSTEM, MSG_ACC_GRA_ANZEIGE]} FWD_BUS_LOOKUP = {0: 2, 2: 0} @@ -192,15 +189,6 @@ def test_main_switch(self): self._rx(self._motor_5_msg(main_switch=False)) self.assertFalse(self.safety.get_controls_allowed(), "controls allowed after ACC main switch off") - def test_accel_safety_check(self): - for controls_allowed in [True, False]: - for accel in np.arange(MIN_ACCEL - 2, MAX_ACCEL + 2, 0.005): - accel = round(accel, 2) # floats might not hit exact boundary conditions without rounding - send = MIN_ACCEL <= accel <= MAX_ACCEL if controls_allowed else accel == self.INACTIVE_ACCEL - self.safety.set_controls_allowed(controls_allowed) - # primary accel request used by ECU - self.assertEqual(send, self._tx(self._accel_msg(accel)), (controls_allowed, accel)) - if __name__ == "__main__": unittest.main() From ba50c2ffee990ca3cbfdcebd1cf750c9046a11a7 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 21 Mar 2023 12:42:02 -0700 Subject: [PATCH 02/59] safety tests: fix for common longitudinal accel class (#1299) * fix * fix * something that would catch this --- tests/safety/common.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/safety/common.py b/tests/safety/common.py index 91da3da4d0..c2e1777ada 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -133,8 +133,8 @@ def test_gas_interceptor_safety_check(self): class LongitudinalAccelSafetyTest(PandaSafetyTestBase, abc.ABC): - MIN_ACCEL: float = 2.0 - MAX_ACCEL: float = -3.5 + MAX_ACCEL: float = 2.0 + MIN_ACCEL: float = -3.5 INACTIVE_ACCEL: float = 0.0 @classmethod @@ -147,6 +147,10 @@ def setUpClass(cls): def _accel_msg(self, accel: float): pass + def test_accel_limits_correct(self): + self.assertGreater(self.MAX_ACCEL, 0) + self.assertLess(self.MIN_ACCEL, 0) + def test_accel_actuation_limits(self, stock_longitudinal=False): limits = ((self.MIN_ACCEL, self.MAX_ACCEL, ALTERNATIVE_EXPERIENCE.DEFAULT), (self.MIN_ACCEL, self.MAX_ACCEL, ALTERNATIVE_EXPERIENCE.RAISE_LONGITUDINAL_LIMITS_TO_ISO_MAX)) From ba7d21805a0a0d6770a522ef6c39bca537361e61 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 21 Mar 2023 18:49:47 -0700 Subject: [PATCH 03/59] ci: lower safety test timeout --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 79640431ad..68df63b136 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -67,7 +67,7 @@ jobs: - name: Build Docker image run: eval "$BUILD" - name: Run safety tests - timeout-minutes: 4 + timeout-minutes: 3 run: | ${{ env.RUN }} "cd .. && \ scons -c && \ From c5cd0a0232039807ab95b68babb91c91f3a5319d Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 21 Mar 2023 18:49:58 -0700 Subject: [PATCH 04/59] Enable siren for multi-panda setups (#1300) --- board/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/main.c b/board/main.c index 4620a61573..09954b2c31 100644 --- a/board/main.c +++ b/board/main.c @@ -194,7 +194,7 @@ void tick_handler(void) { siren_countdown -= 1U; } - if (controls_allowed) { + if (controls_allowed || heartbeat_engaged) { controls_allowed_countdown = 30U; } else if (controls_allowed_countdown > 0U) { controls_allowed_countdown -= 1U; From 7aaca348c22089abcbd695afc0430da7f28a609c Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 22 Mar 2023 21:38:37 -0700 Subject: [PATCH 05/59] HITL tests: nose -> pytest (#1301) * HITL tests: nose -> pytest * run all * add back partial tests * big speed up * fix skipping * enable all * that's expected * clean up after ourselves * jungle is fixture * fix --------- Co-authored-by: Bruce Wayne --- python/__init__.py | 4 + requirements.txt | 1 - tests/hitl/0_dfu.py | 3 - tests/hitl/1_program.py | 11 +-- tests/hitl/2_health.py | 30 ++---- tests/hitl/3_usb_to_can.py | 49 ++++------ tests/hitl/4_can_loopback.py | 36 +++---- tests/hitl/5_gps.py | 8 +- tests/hitl/6_safety.py | 12 +-- tests/hitl/conftest.py | 180 +++++++++++++++++++++++++++++++++++ tests/hitl/helpers.py | 164 +------------------------------ tests/hitl/test.sh | 4 +- 12 files changed, 246 insertions(+), 256 deletions(-) create mode 100644 tests/hitl/conftest.py diff --git a/python/__init__.py b/python/__init__.py index 1b8495f40d..5d6f3e9fc4 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -401,6 +401,10 @@ def reset(self, enter_bootstub=False, enter_bootloader=False, reconnect=True): if not enter_bootloader and reconnect: self.reconnect() + @property + def connected(self) -> bool: + return self._handle_open + def reconnect(self): if self._handle_open: self.close() diff --git a/requirements.txt b/requirements.txt index 600be4b2bf..6b063e91b4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,6 @@ numpy hexdump>=3.3 pycryptodome==3.9.8 tqdm>=4.14.0 -nose pytest parameterized requests diff --git a/tests/hitl/0_dfu.py b/tests/hitl/0_dfu.py index ff697ba545..cff4c02c79 100644 --- a/tests/hitl/0_dfu.py +++ b/tests/hitl/0_dfu.py @@ -1,8 +1,5 @@ from panda import Panda, PandaDFU -from .helpers import test_all_pandas, panda_connect_and_init -@test_all_pandas -@panda_connect_and_init def test_dfu(p): app_mcu_type = p.get_mcu_type() dfu_serial = PandaDFU.st_serial_to_dfu_serial(p.get_usb_serial(), p.get_mcu_type()) diff --git a/tests/hitl/1_program.py b/tests/hitl/1_program.py index c129c99b5c..6db3355096 100644 --- a/tests/hitl/1_program.py +++ b/tests/hitl/1_program.py @@ -2,13 +2,14 @@ import time from panda import Panda, PandaDFU, McuType, BASEDIR -from .helpers import test_all_pandas, panda_connect_and_init, check_signature +def check_signature(p): + assert not p.bootstub, "Flashed firmware not booting. Stuck in bootstub." + assert p.up_to_date() + # TODO: make more comprehensive bootstub tests and run on a few production ones + current # TODO: also test release-signed app -@test_all_pandas -@panda_connect_and_init def test_a_known_bootstub(p): """ Test that compiled app can work with known production bootstub @@ -53,14 +54,10 @@ def test_a_known_bootstub(p): check_signature(p) assert not p.bootstub -@test_all_pandas -@panda_connect_and_init def test_b_recover(p): assert p.recover(timeout=30) check_signature(p) -@test_all_pandas -@panda_connect_and_init def test_c_flash(p): # test flash from bootstub serial = p._serial diff --git a/tests/hitl/2_health.py b/tests/hitl/2_health.py index 3a7f1e74a2..f63f777cc0 100644 --- a/tests/hitl/2_health.py +++ b/tests/hitl/2_health.py @@ -1,32 +1,28 @@ import time +import pytest from panda import Panda from panda_jungle import PandaJungle # pylint: disable=import-error -from .helpers import panda_jungle, test_all_pandas, test_all_gen2_pandas, panda_connect_and_init +from panda.tests.hitl.conftest import PandaGroup -@test_all_pandas -@panda_connect_and_init -def test_ignition(p): +def test_ignition(p, panda_jungle): # Set harness orientation to #2, since the ignition line is on the wrong SBU bus :/ panda_jungle.set_harness_orientation(PandaJungle.HARNESS_ORIENTATION_2) p.reset() for ign in (True, False): panda_jungle.set_ignition(ign) - time.sleep(2) + time.sleep(0.1) assert p.health()['ignition_line'] == ign -@test_all_gen2_pandas -@panda_connect_and_init -def test_orientation_detection(p): +@pytest.mark.test_panda_types(PandaGroup.GEN2) +def test_orientation_detection(p, panda_jungle): seen_orientations = [] for i in range(3): panda_jungle.set_harness_orientation(i) - time.sleep(0.25) p.reset() - time.sleep(0.25) detected_harness_orientation = p.health()['car_harness_status'] print(f"Detected orientation: {detected_harness_orientation}") @@ -34,16 +30,12 @@ def test_orientation_detection(p): assert False seen_orientations.append(detected_harness_orientation) -@test_all_pandas -@panda_connect_and_init def test_voltage(p): for _ in range(10): voltage = p.health()['voltage'] assert ((voltage > 11000) and (voltage < 13000)) time.sleep(0.1) -@test_all_pandas -@panda_connect_and_init def test_hw_type(p): """ hw type should be same in bootstub as application @@ -66,10 +58,8 @@ def test_hw_type(p): assert pp.get_mcu_type() == mcu_type, "Bootstub and app MCU type mismatch" assert pp.get_uid() == app_uid - -@test_all_pandas -@panda_connect_and_init -def test_heartbeat(p): +def test_heartbeat(p, panda_jungle): + panda_jungle.set_ignition(True) # TODO: add more cases here once the tests aren't super slow p.set_safety_mode(mode=Panda.SAFETY_HYUNDAI, param=Panda.FLAG_HYUNDAI_LONG) p.send_heartbeat() @@ -79,7 +69,7 @@ def test_heartbeat(p): # shouldn't do anything once we're in a car safety mode p.set_heartbeat_disabled() - time.sleep(6) + time.sleep(6.) h = p.health() assert h['heartbeat_lost'] @@ -87,8 +77,6 @@ def test_heartbeat(p): assert h['safety_param'] == 0 assert h['controls_allowed'] == 0 -@test_all_pandas -@panda_connect_and_init def test_microsecond_timer(p): start_time = p.get_microsecond_timer() time.sleep(1) diff --git a/tests/hitl/3_usb_to_can.py b/tests/hitl/3_usb_to_can.py index 4aca06affc..22cfde438e 100644 --- a/tests/hitl/3_usb_to_can.py +++ b/tests/hitl/3_usb_to_can.py @@ -1,13 +1,12 @@ import sys import time +import pytest from flaky import flaky -from nose.tools import assert_equal, assert_less, assert_greater from panda import Panda -from .helpers import SPEED_NORMAL, SPEED_GMLAN, time_many_sends, test_all_gmlan_pandas, test_all_pandas, panda_connect_and_init +from panda.tests.hitl.conftest import SPEED_NORMAL, SPEED_GMLAN, PandaGroup +from panda.tests.hitl.helpers import time_many_sends -@test_all_pandas -@panda_connect_and_init def test_can_loopback(p): p.set_safety_mode(Panda.SAFETY_ALLOUTPUT) p.set_can_loopback(True) @@ -31,8 +30,6 @@ def test_can_loopback(p): assert 0x1aa == sr[0][0] == lb[0][0] assert b"message" == sr[0][2] == lb[0][2] -@test_all_pandas -@panda_connect_and_init def test_reliability(p): MSG_COUNT = 100 @@ -55,20 +52,18 @@ def test_reliability(p): sent_echo = [x for x in r if x[3] == 0x80] loopback_resp = [x for x in r if x[3] == 0] - assert_equal(sorted([x[0] for x in loopback_resp]), addrs) - assert_equal(sorted([x[0] for x in sent_echo]), addrs) - assert_equal(len(r), 200) + assert sorted([x[0] for x in loopback_resp]) == addrs + assert sorted([x[0] for x in sent_echo]) == addrs + assert len(r) == 200 # take sub 20ms et = (time.monotonic() - st) * 1000.0 - assert_less(et, 20) + assert et < 20 sys.stdout.write("P") sys.stdout.flush() -@test_all_pandas @flaky(max_runs=6, min_passes=1) -@panda_connect_and_init def test_throughput(p): # enable output mode p.set_safety_mode(Panda.SAFETY_ALLOUTPUT) @@ -85,13 +80,12 @@ def test_throughput(p): # bit count from https://en.wikipedia.org/wiki/CAN_bus saturation_pct = (comp_kbps / speed) * 100.0 - assert_greater(saturation_pct, 80) - assert_less(saturation_pct, 100) + assert saturation_pct > 80 + assert saturation_pct < 100 print("loopback 100 messages at speed %d, comp speed is %.2f, percent %.2f" % (speed, comp_kbps, saturation_pct)) -@test_all_gmlan_pandas -@panda_connect_and_init +@pytest.mark.test_panda_types(PandaGroup.GMLAN) def test_gmlan(p): p.set_safety_mode(Panda.SAFETY_ALLOUTPUT) p.set_can_loopback(True) @@ -104,18 +98,17 @@ def test_gmlan(p): for bus in [Panda.GMLAN_CAN2, Panda.GMLAN_CAN3, Panda.GMLAN_CAN2, Panda.GMLAN_CAN3]: p.set_gmlan(bus) comp_kbps_gmlan = time_many_sends(p, 3) - assert_greater(comp_kbps_gmlan, 0.8 * SPEED_GMLAN) - assert_less(comp_kbps_gmlan, 1.0 * SPEED_GMLAN) + assert comp_kbps_gmlan > (0.8 * SPEED_GMLAN) + assert comp_kbps_gmlan < (1.0 * SPEED_GMLAN) p.set_gmlan(None) comp_kbps_normal = time_many_sends(p, bus) - assert_greater(comp_kbps_normal, 0.8 * SPEED_NORMAL) - assert_less(comp_kbps_normal, 1.0 * SPEED_NORMAL) + assert comp_kbps_normal > (0.8 * SPEED_NORMAL) + assert comp_kbps_normal < (1.0 * SPEED_NORMAL) print("%d: %.2f kbps vs %.2f kbps" % (bus, comp_kbps_gmlan, comp_kbps_normal)) -@test_all_gmlan_pandas -@panda_connect_and_init +@pytest.mark.test_panda_types(PandaGroup.GMLAN) def test_gmlan_bad_toggle(p): p.set_safety_mode(Panda.SAFETY_ALLOUTPUT) p.set_can_loopback(True) @@ -124,21 +117,19 @@ def test_gmlan_bad_toggle(p): for bus in [Panda.GMLAN_CAN2, Panda.GMLAN_CAN3]: p.set_gmlan(bus) comp_kbps_gmlan = time_many_sends(p, 3) - assert_greater(comp_kbps_gmlan, 0.6 * SPEED_GMLAN) - assert_less(comp_kbps_gmlan, 1.0 * SPEED_GMLAN) + assert comp_kbps_gmlan > (0.6 * SPEED_GMLAN) + assert comp_kbps_gmlan < (1.0 * SPEED_GMLAN) # normal for bus in [Panda.GMLAN_CAN2, Panda.GMLAN_CAN3]: p.set_gmlan(None) comp_kbps_normal = time_many_sends(p, bus) - assert_greater(comp_kbps_normal, 0.6 * SPEED_NORMAL) - assert_less(comp_kbps_normal, 1.0 * SPEED_NORMAL) + assert comp_kbps_normal > (0.6 * SPEED_NORMAL) + assert comp_kbps_normal < (1.0 * SPEED_NORMAL) # this will fail if you have hardware serial connected -@test_all_pandas -@panda_connect_and_init def test_serial_debug(p): _ = p.serial_read(Panda.SERIAL_DEBUG) # junk p.call_control_api(0x01) - assert(p.serial_read(Panda.SERIAL_DEBUG).startswith(b"NO HANDLER")) + assert p.serial_read(Panda.SERIAL_DEBUG).startswith(b"NO HANDLER") diff --git a/tests/hitl/4_can_loopback.py b/tests/hitl/4_can_loopback.py index 103da33ce1..f0241d2b78 100644 --- a/tests/hitl/4_can_loopback.py +++ b/tests/hitl/4_can_loopback.py @@ -1,18 +1,17 @@ import os import time +import pytest import random import threading from flaky import flaky from collections import defaultdict -from nose.tools import assert_equal, assert_less, assert_greater from panda import Panda -from .helpers import panda_jungle, time_many_sends, test_all_pandas, test_all_gen2_pandas, clear_can_buffers, panda_connect_and_init, PARTIAL_TESTS +from panda.tests.hitl.conftest import PandaGroup, PARTIAL_TESTS +from panda.tests.hitl.helpers import time_many_sends, clear_can_buffers -@test_all_pandas @flaky(max_runs=3, min_passes=1) -@panda_connect_and_init -def test_send_recv(p): +def test_send_recv(p, panda_jungle): def test(p_send, p_recv): p_send.set_can_loopback(False) p_recv.set_can_loopback(False) @@ -35,8 +34,7 @@ def test(p_send, p_recv): comp_kbps = time_many_sends(p_send, bus, p_recv, two_pandas=True) saturation_pct = (comp_kbps / speed) * 100.0 - assert_greater(saturation_pct, 80) - assert_less(saturation_pct, 100) + assert 80 < saturation_pct < 100 print("two pandas bus {}, 100 messages at speed {:4d}, comp speed is {:7.2f}, {:6.2f}%".format(bus, speed, comp_kbps, saturation_pct)) @@ -46,10 +44,8 @@ def test(p_send, p_recv): test(panda_jungle, p) -@test_all_pandas @flaky(max_runs=6, min_passes=1) -@panda_connect_and_init -def test_latency(p): +def test_latency(p, panda_jungle): def test(p_send, p_recv): p_send.set_can_loopback(False) p_recv.set_can_loopback(False) @@ -93,14 +89,14 @@ def test(p_send, p_recv): if len(r) == 0 or len(r_echo) == 0: print("r: {}, r_echo: {}".format(r, r_echo)) - assert_equal(len(r), 1) - assert_equal(len(r_echo), 1) + assert len(r) == 1 + assert len(r_echo) == 1 et = (et - st) * 1000.0 comp_kbps = (1 + 11 + 1 + 1 + 1 + 4 + 8 * 8 + 15 + 1 + 1 + 1 + 7) / et latency = et - ((1 + 11 + 1 + 1 + 1 + 4 + 8 * 8 + 15 + 1 + 1 + 1 + 7) / speed) - assert_less(latency, 5.0) + assert latency < 5.0 saturation_pct = (comp_kbps / speed) * 100.0 latencies.append(latency) @@ -108,7 +104,7 @@ def test(p_send, p_recv): saturation_pcts.append(saturation_pct) average_latency = sum(latencies) / num_messages - assert_less(average_latency, 1.0) + assert average_latency < 1.0 average_comp_kbps = sum(comp_kbps_list) / num_messages average_saturation_pct = sum(saturation_pcts) / num_messages @@ -121,9 +117,9 @@ def test(p_send, p_recv): test(panda_jungle, p) -@test_all_gen2_pandas -@panda_connect_and_init -def test_gen2_loopback(p): +@pytest.mark.panda_expect_can_error +@pytest.mark.test_panda_types(PandaGroup.GEN2) +def test_gen2_loopback(p, panda_jungle): def test(p_send, p_recv, address=None): for bus in range(4): obd = False @@ -167,9 +163,7 @@ def test(p_send, p_recv, address=None): test(p, panda_jungle, 0x18DB33F1) test(panda_jungle, p, 0x18DB33F1) -@test_all_pandas -@panda_connect_and_init -def test_bulk_write(p): +def test_bulk_write(p, panda_jungle): # TODO: doesn't work in partial test mode if PARTIAL_TESTS: return @@ -211,8 +205,6 @@ def flood_tx(panda): # Set back to silent mode p.set_safety_mode(Panda.SAFETY_SILENT) -@test_all_pandas -@panda_connect_and_init def test_message_integrity(p): clear_can_buffers(p) diff --git a/tests/hitl/5_gps.py b/tests/hitl/5_gps.py index 7fb6329c6b..3cead333ed 100644 --- a/tests/hitl/5_gps.py +++ b/tests/hitl/5_gps.py @@ -1,9 +1,11 @@ import time +import pytest + from panda import PandaSerial -from .helpers import test_all_gps_pandas, panda_connect_and_init +from panda.tests.hitl.conftest import PandaGroup + -@test_all_gps_pandas -@panda_connect_and_init +@pytest.mark.test_panda_types(PandaGroup.GPS) def test_gps_version(p): serial = PandaSerial(p, 1, 9600) # Reset and check twice to make sure the enabling works diff --git a/tests/hitl/6_safety.py b/tests/hitl/6_safety.py index 375283823d..2237f53036 100644 --- a/tests/hitl/6_safety.py +++ b/tests/hitl/6_safety.py @@ -1,11 +1,8 @@ import time -from nose.tools import assert_equal from panda import Panda -from .helpers import test_all_pandas, panda_connect_and_init -@test_all_pandas -@panda_connect_and_init + def test_safety_nooutput(p): p.set_safety_mode(Panda.SAFETY_SILENT) p.set_can_loopback(True) @@ -20,14 +17,13 @@ def test_safety_nooutput(p): assert len([x for x in r if x[3] != 192]) == 0 assert len([x for x in r if x[3] == 192]) == 1 -@test_all_pandas -@panda_connect_and_init + def test_canfd_safety_modes(p): # works on all pandas p.set_safety_mode(Panda.SAFETY_TOYOTA) - assert_equal(p.health()['safety_mode'], Panda.SAFETY_TOYOTA) + assert p.health()['safety_mode'] == Panda.SAFETY_TOYOTA # shouldn't be able to set a CAN-FD safety mode on non CAN-FD panda p.set_safety_mode(Panda.SAFETY_HYUNDAI_CANFD) expected_mode = Panda.SAFETY_HYUNDAI_CANFD if p.get_type() in Panda.H7_DEVICES else Panda.SAFETY_SILENT - assert_equal(p.health()['safety_mode'], expected_mode) + assert p.health()['safety_mode'] == expected_mode diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py new file mode 100644 index 0000000000..a70f2ca769 --- /dev/null +++ b/tests/hitl/conftest.py @@ -0,0 +1,180 @@ +import concurrent.futures +import os +import time +import pytest + +from panda import Panda +from panda_jungle import PandaJungle # pylint: disable=import-error +from panda.tests.hitl.helpers import clear_can_buffers + + +SPEED_NORMAL = 500 +SPEED_GMLAN = 33.3 +BUS_SPEEDS = [(0, SPEED_NORMAL), (1, SPEED_NORMAL), (2, SPEED_NORMAL), (3, SPEED_GMLAN)] + + +PEDAL_SERIAL = 'none' +JUNGLE_SERIAL = os.getenv("PANDAS_JUNGLE") +PANDAS_EXCLUDE = os.getenv("PANDAS_EXCLUDE", "").strip().split(" ") +PARTIAL_TESTS = os.environ.get("PARTIAL_TESTS", "0") == "1" + +class PandaGroup: + H7 = (Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2) + GEN2 = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO) + H7 + GPS = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO) + GMLAN = (Panda.HW_TYPE_WHITE_PANDA, Panda.HW_TYPE_GREY_PANDA) + + TESTED = (Panda.HW_TYPE_WHITE_PANDA, Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2, Panda.HW_TYPE_UNO) + +if PARTIAL_TESTS: + # minimal set of pandas to get most of our coverage + # * red panda covers GEN2, STM32H7 + # * black panda covers STM32F4, GEN2, and GPS + PandaGroup.TESTED = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_RED_PANDA) # type: ignore + +# Find all pandas connected +_all_pandas = {} +_panda_jungle = None +def init_all_pandas(): + global _panda_jungle + _panda_jungle = PandaJungle(JUNGLE_SERIAL) + _panda_jungle.set_panda_power(True) + + for serial in Panda.list(): + if serial not in PANDAS_EXCLUDE and serial != PEDAL_SERIAL: + with Panda(serial=serial) as p: + ptype = bytes(p.get_type()) + if ptype in PandaGroup.TESTED: + _all_pandas[serial] = ptype + + # ensure we have all tested panda types + missing_types = set(PandaGroup.TESTED) - set(_all_pandas.values()) + assert len(missing_types) == 0, f"Missing panda types: {missing_types}" + + print(f"{len(_all_pandas)} total pandas") +init_all_pandas() +_all_panda_serials = list(_all_pandas.keys()) + + +def init_jungle(): + clear_can_buffers(_panda_jungle) + _panda_jungle.set_panda_power(True) + _panda_jungle.set_can_loopback(False) + _panda_jungle.set_obd(False) + _panda_jungle.set_harness_orientation(PandaJungle.HARNESS_ORIENTATION_1) + for bus, speed in BUS_SPEEDS: + _panda_jungle.set_can_speed_kbps(bus, speed) + + +def pytest_configure(config): + config.addinivalue_line( + "markers", "test_panda_types(name): mark test to run only on specified panda types" + ) + config.addinivalue_line( + "markers", "panda_expect_can_error: mark test to ignore CAN health errors" + ) + + +def pytest_make_parametrize_id(config, val, argname): + if val in _all_pandas: + # TODO: get nice string instead of int + hw_type = _all_pandas[val][0] + return f"serial={val}, hw_type={hw_type}" + return None + + +@pytest.fixture(name='panda_jungle') +def fixture__panda_jungle(request): + init_jungle() + return _panda_jungle + +@pytest.fixture(name='p') +def func_fixture_panda(request, module_panda): + p = module_panda + + # Check if test is applicable to this panda + mark = request.node.get_closest_marker('test_panda_types') + if mark: + assert len(mark.args) > 0, "Missing allowed panda types in mark" + test_types = mark.args[0] + if _all_pandas[p.get_usb_serial()] not in test_types: + pytest.skip(f"Not applicable, {test_types} pandas only") + + # TODO: reset is slow (2+ seconds) + p.reset() + + # Run test + yield p + + # Teardown + if not p.connected: + p.reconnect() + if p.bootstub: + p.reset() + + # TODO: would be nice to make these common checks in the teardown + # show up as failed tests instead of "errors" + + # Check for faults + assert p.health()['fault_status'] == 0 + + # Check health of each CAN core after test, normal to fail for test_gen2_loopback on OBD bus, so skipping + mark = request.node.get_closest_marker('panda_expect_can_error') + expect_can_error = mark is not None + if not expect_can_error: + for i in range(3): + can_health = p.can_health(i) + assert can_health['bus_off_cnt'] == 0 + assert can_health['receive_error_cnt'] == 0 + assert can_health['transmit_error_cnt'] == 0 + assert can_health['total_rx_lost_cnt'] == 0 + assert can_health['total_tx_lost_cnt'] == 0 + assert can_health['total_error_cnt'] == 0 + assert can_health['total_tx_checksum_error_cnt'] == 0 + +@pytest.fixture(name='module_panda', params=_all_panda_serials, scope='module') +def fixture_panda_setup(request): + """ + Clean up all pandas + jungle and return the panda under test. + """ + panda_serial = request.param + + # Initialize jungle + init_jungle() + + # wait for all pandas to come up + for _ in range(50): + if set(_all_panda_serials).issubset(set(Panda.list())): + break + time.sleep(0.1) + + # Connect to pandas + def cnnct(s): + if s == panda_serial: + p = Panda(serial=s) + p.reset(reconnect=True) + + p.set_can_loopback(False) + p.set_gmlan(None) + p.set_esp_power(False) + p.set_power_save(False) + for bus, speed in BUS_SPEEDS: + p.set_can_speed_kbps(bus, speed) + clear_can_buffers(p) + p.set_power_save(False) + return p + else: + with Panda(serial=s) as p: + p.reset(reconnect=False) + return None + + with concurrent.futures.ThreadPoolExecutor() as exc: + ps = list(exc.map(cnnct, _all_panda_serials, timeout=20)) + pandas = [p for p in ps if p is not None] + + # run test + yield pandas[0] + + # Teardown + for p in pandas: + p.close() diff --git a/tests/hitl/helpers.py b/tests/hitl/helpers.py index 99070f0f96..2c36b79896 100644 --- a/tests/hitl/helpers.py +++ b/tests/hitl/helpers.py @@ -1,81 +1,5 @@ -import concurrent.futures -import os import time import random -import faulthandler -from functools import wraps, partial -from nose.tools import assert_equal -from parameterized import parameterized - -from panda import Panda -from panda_jungle import PandaJungle # pylint: disable=import-error - -SPEED_NORMAL = 500 -SPEED_GMLAN = 33.3 -BUS_SPEEDS = [(0, SPEED_NORMAL), (1, SPEED_NORMAL), (2, SPEED_NORMAL), (3, SPEED_GMLAN)] -H7_HW_TYPES = [Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2] -GEN2_HW_TYPES = [Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO] + H7_HW_TYPES -GPS_HW_TYPES = [Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO] -PEDAL_SERIAL = 'none' -JUNGLE_SERIAL = os.getenv("PANDAS_JUNGLE") -PANDAS_EXCLUDE = os.getenv("PANDAS_EXCLUDE", "").strip().split(" ") - -PARTIAL_TESTS = os.environ.get("PARTIAL_TESTS", "0") == "1" - -# Enable fault debug -faulthandler.enable(all_threads=False) - -# Connect to Panda Jungle -panda_jungle = PandaJungle(JUNGLE_SERIAL) - -# Find all pandas connected -_all_pandas = [] -def init_all_pandas(): - global _all_pandas - _all_pandas = [] - - # power cycle pandas - panda_jungle.set_panda_power(False) - time.sleep(3) - panda_jungle.set_panda_power(True) - time.sleep(5) - - for serial in Panda.list(): - if serial not in PANDAS_EXCLUDE and serial != PEDAL_SERIAL: - with Panda(serial=serial) as p: - _all_pandas.append((serial, p.get_type())) - print(f"{len(_all_pandas)} total pandas") -init_all_pandas() -_all_panda_serials = [x[0] for x in _all_pandas] - -def parameterized_panda_types(types): - serials = [] - for typ in types: - for s, t in _all_pandas: - if t == typ and s not in serials: - serials.append(s) - break - else: - raise IOError("No unused panda found for type: {}".format(typ)) - return parameterized(serials) - -# Panda providers -TESTED_HW_TYPES = (Panda.HW_TYPE_WHITE_PANDA, Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2, Panda.HW_TYPE_UNO) -test_all_pandas = parameterized_panda_types(TESTED_HW_TYPES) -test_all_gen2_pandas = parameterized_panda_types(GEN2_HW_TYPES) -test_all_gps_pandas = parameterized_panda_types(GPS_HW_TYPES) - -# no grey for speedup, should be sufficiently covered by white for these tests -test_all_gmlan_pandas = parameterized_panda_types([Panda.HW_TYPE_WHITE_PANDA, ]) - -if PARTIAL_TESTS: - # minimal set of pandas to get most of our coverage - # * red panda covers STM32H7 - # * black panda covers STM32F4, GEN2, and GPS - partial_pandas = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_RED_PANDA) - test_all_pandas = parameterized_panda_types(partial_pandas) - test_all_gen2_pandas = parameterized_panda_types(partial_pandas) - test_all_gps_pandas = parameterized_panda_types([Panda.HW_TYPE_BLACK_PANDA, ]) def time_many_sends(p, bus, p_recv=None, msg_count=100, msg_id=None, two_pandas=False): @@ -105,92 +29,16 @@ def time_many_sends(p, bus, p_recv=None, msg_count=100, msg_id=None, two_pandas= resp = [x for x in r if x[3] == bus and x[0] == msg_id] leftovers = [x for x in r if (x[3] != 0x80 | bus and x[3] != bus) or x[0] != msg_id] - assert_equal(len(leftovers), 0) + assert len(leftovers) == 0 - assert_equal(len(resp), msg_count) - assert_equal(len(sent_echo), msg_count) + assert len(resp) == msg_count + assert len(sent_echo) == msg_count end_time = (end_time - start_time) * 1000.0 comp_kbps = (1 + 11 + 1 + 1 + 1 + 4 + 8 * 8 + 15 + 1 + 1 + 1 + 7) * msg_count / end_time return comp_kbps -def panda_connect_and_init(fn=None): - if not fn: - return partial(panda_connect_and_init) - - @wraps(fn) - def wrapper(panda_serials, **kwargs): - # Change panda_serials to a list - if panda_serials is not None: - if not isinstance(panda_serials, list): - panda_serials = [panda_serials, ] - - # Initialize jungle - clear_can_buffers(panda_jungle) - panda_jungle.set_panda_power(True) - panda_jungle.set_can_loopback(False) - panda_jungle.set_obd(False) - panda_jungle.set_harness_orientation(PandaJungle.HARNESS_ORIENTATION_1) - for bus, speed in BUS_SPEEDS: - panda_jungle.set_can_speed_kbps(bus, speed) - - # wait for all pandas to come up - for _ in range(50): - if set(_all_panda_serials).issubset(set(Panda.list())): - break - time.sleep(0.1) - - # Connect to pandas - def cnnct(s): - if s in panda_serials: - p = Panda(serial=s) - p.reset(reconnect=True) - - p.set_can_loopback(False) - p.set_gmlan(None) - p.set_esp_power(False) - p.set_power_save(False) - for bus, speed in BUS_SPEEDS: - p.set_can_speed_kbps(bus, speed) - clear_can_buffers(p) - p.set_power_save(False) - return p - else: - with Panda(serial=s) as p: - p.reset(reconnect=False) - return None - - with concurrent.futures.ThreadPoolExecutor() as exc: - ps = list(exc.map(cnnct, _all_panda_serials, timeout=20)) - pandas = [p for p in ps if p is not None] - - try: - fn(*pandas, *kwargs) - - # Check if the pandas did not throw any faults while running test - for panda in pandas: - if not panda.bootstub: - #panda.reconnect() - assert panda.health()['fault_status'] == 0 - # Check health of each CAN core after test, normal to fail for test_gen2_loopback on OBD bus, so skipping - if fn.__name__ != "test_gen2_loopback": - for i in range(3): - can_health = panda.can_health(i) - assert can_health['bus_off_cnt'] == 0 - assert can_health['receive_error_cnt'] == 0 - assert can_health['transmit_error_cnt'] == 0 - assert can_health['total_rx_lost_cnt'] == 0 - assert can_health['total_tx_lost_cnt'] == 0 - assert can_health['total_error_cnt'] == 0 - assert can_health['total_tx_checksum_error_cnt'] == 0 - finally: - for p in pandas: - try: - p.close() - except Exception: - pass - return wrapper def clear_can_buffers(panda): # clear tx buffers @@ -207,9 +55,3 @@ def clear_can_buffers(panda): if (time.monotonic() - st) > 10: print("Unable to clear can buffers for panda ", panda.get_serial()) assert False - -def check_signature(p): - assert not p.bootstub, "Flashed firmware not booting. Stuck in bootstub." - firmware_sig = Panda.get_signature_from_firmware(p.get_mcu_type().config.app_path) - panda_sig = p.get_signature() - assert_equal(panda_sig, firmware_sig) diff --git a/tests/hitl/test.sh b/tests/hitl/test.sh index d2de5f3b7d..7382ac5988 100755 --- a/tests/hitl/test.sh +++ b/tests/hitl/test.sh @@ -2,4 +2,6 @@ set -e DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" -nosetests -x -v --with-flaky -s $(ls $DIR/$1*.py) +cd $DIR + +pytest --durations=0 --maxfail=1 *.py From f29125f6ade6f0f5a5ecb5b23c1f3169fe1d80a0 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Wed, 22 Mar 2023 21:57:18 -0700 Subject: [PATCH 06/59] DriverTorqueSteeringSafetyTest: remove useless function (#1275) Update common.py --- tests/safety/common.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/safety/common.py b/tests/safety/common.py index c2e1777ada..fc63d26313 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -337,12 +337,8 @@ def test_non_realtime_limit_up(self): self.safety.set_torque_driver(0, 0) super().test_non_realtime_limit_up() - # TODO: make this test something - def test_non_realtime_limit_down(self): - self.safety.set_torque_driver(0, 0) - self.safety.set_controls_allowed(True) - def test_against_torque_driver(self): + # Tests down limits and driver torque blending self.safety.set_controls_allowed(True) for sign in [-1, 1]: From 6cb3561f1687efdc4bb04d812ab15e269b7626e8 Mon Sep 17 00:00:00 2001 From: AlexandreSato <66435071+AlexandreSato@users.noreply.github.com> Date: Sat, 25 Mar 2023 23:02:34 -0300 Subject: [PATCH 07/59] safety replay: update Honda steer value getter (#1292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * safety replay: update Honda steer value getter I think now it´s right! ``` BO_ 228 STEERING_CONTROL: 5 EON SG_ STEER_TORQUE : 7|16@0- (1,0) [-4096|4096] "" EPS ``` * ops --- tests/safety_replay/helpers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/safety_replay/helpers.py b/tests/safety_replay/helpers.py index 9eb26ee82e..4877b2e5e4 100644 --- a/tests/safety_replay/helpers.py +++ b/tests/safety_replay/helpers.py @@ -29,7 +29,8 @@ def is_steering_msg(mode, addr): def get_steer_value(mode, to_send): torque, angle = 0, 0 if mode in (Panda.SAFETY_HONDA_NIDEC, Panda.SAFETY_HONDA_BOSCH): - torque = to_send.RDLR & 0xFFFF0000 + torque = (to_send.data[0] << 8) | to_send.data[1] + torque = to_signed(torque, 16) elif mode == Panda.SAFETY_TOYOTA: torque = (to_send.data[1] << 8) | (to_send.data[2]) torque = to_signed(torque, 16) From 01ea26e342a2b84059ee6361ee0124d7e39b036b Mon Sep 17 00:00:00 2001 From: Igor Biletksyy Date: Mon, 27 Mar 2023 10:38:04 -0700 Subject: [PATCH 08/59] prevent host from setting wrong returned flag --- board/drivers/can_common.h | 1 + 1 file changed, 1 insertion(+) diff --git a/board/drivers/can_common.h b/board/drivers/can_common.h index 9fd2743d6c..01abc6a073 100644 --- a/board/drivers/can_common.h +++ b/board/drivers/can_common.h @@ -257,6 +257,7 @@ void can_send(CANPacket_t *to_push, uint8_t bus_number, bool skip_tx_hook) { } } else { safety_tx_blocked += 1U; + to_push->returned = 0U; to_push->rejected = 1U; // data changed From db6c50cd09f773c231f962fc0e31b4612c572b08 Mon Sep 17 00:00:00 2001 From: royjr Date: Tue, 28 Mar 2023 02:10:25 -0400 Subject: [PATCH 09/59] Honda Bosch Radarless longitudinal safety (#1008) * civic22_long use long flag * maybe? * Update safety_honda.h * revert the 0x1c8 addition * init radarless safety check * correct accel parsing * add test class (WIP) * remove current_controls_allowed * Fix ACC_HUD * patch * Add missing safety to test name * Pass irrelevant test * move comment * remove unused comments * add comment to TestHondaBoschRadarlessSafety * Fix message overlap The "exceptions for common msgs across different hondas" fix does not seem to be working. Created another exception for TestHondaBoschRadarless based on the HondaNidec exception above * Fix base * remove comment * remove buttons from LONG TX * make a base Radarless test class * can't remove them, since safety doesn't check bosch * test accel limits * test accel limits * we can do the clean up in another pr --------- Co-authored-by: Adeeb Shihadeh Co-authored-by: sshane --- board/safety/safety_honda.h | 25 ++++++++++++++++--- tests/safety/common.py | 4 +++ tests/safety/test_honda.py | 49 ++++++++++++++++++++++++++++++++++--- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h index 51a908b853..e0a6e2828c 100644 --- a/board/safety/safety_honda.h +++ b/board/safety/safety_honda.h @@ -8,8 +8,9 @@ // brake > 0mph const CanMsg HONDA_N_TX_MSGS[] = {{0xE4, 0, 5}, {0x194, 0, 4}, {0x1FA, 0, 8}, {0x200, 0, 6}, {0x30C, 0, 8}, {0x33D, 0, 5}}; const CanMsg HONDA_BOSCH_TX_MSGS[] = {{0xE4, 0, 5}, {0xE5, 0, 8}, {0x296, 1, 4}, {0x33D, 0, 5}, {0x33DA, 0, 5}, {0x33DB, 0, 8}}; // Bosch -const CanMsg HONDA_RADARLESS_TX_MSGS[] = {{0xE4, 0, 5}, {0x296, 2, 4}, {0x33D, 0, 8}}; // Bosch radarless const CanMsg HONDA_BOSCH_LONG_TX_MSGS[] = {{0xE4, 1, 5}, {0x1DF, 1, 8}, {0x1EF, 1, 8}, {0x1FA, 1, 8}, {0x30C, 1, 8}, {0x33D, 1, 5}, {0x33DA, 1, 5}, {0x33DB, 1, 8}, {0x39F, 1, 8}, {0x18DAB0F1, 1, 8}}; // Bosch w/ gas and brakes +const CanMsg HONDA_RADARLESS_TX_MSGS[] = {{0xE4, 0, 5}, {0x296, 2, 4}, {0x33D, 0, 8}}; // Bosch radarless +const CanMsg HONDA_RADARLESS_LONG_TX_MSGS[] = {{0xE4, 0, 5}, {0x33D, 0, 8}, {0x1C8, 0, 8}, {0x30C, 0, 8}}; // Bosch radarless w/ gas and brakes // panda interceptor threshold needs to be equivalent to openpilot threshold to avoid controls mismatches // If thresholds are mismatched then it is possible for panda to see the gas fall and rise while openpilot is in the pre-enabled state @@ -266,8 +267,10 @@ static int honda_tx_hook(CANPacket_t *to_send) { int addr = GET_ADDR(to_send); int bus = GET_BUS(to_send); - if ((honda_hw == HONDA_BOSCH) && honda_bosch_radarless) { + if ((honda_hw == HONDA_BOSCH) && honda_bosch_radarless && !honda_bosch_long) { tx = msg_allowed(to_send, HONDA_RADARLESS_TX_MSGS, sizeof(HONDA_RADARLESS_TX_MSGS)/sizeof(HONDA_RADARLESS_TX_MSGS[0])); + } else if ((honda_hw == HONDA_BOSCH) && honda_bosch_radarless && honda_bosch_long) { + tx = msg_allowed(to_send, HONDA_RADARLESS_LONG_TX_MSGS, sizeof(HONDA_RADARLESS_LONG_TX_MSGS)/sizeof(HONDA_RADARLESS_LONG_TX_MSGS[0])); } else if ((honda_hw == HONDA_BOSCH) && !honda_bosch_long) { tx = msg_allowed(to_send, HONDA_BOSCH_TX_MSGS, sizeof(HONDA_BOSCH_TX_MSGS)/sizeof(HONDA_BOSCH_TX_MSGS[0])); } else if ((honda_hw == HONDA_BOSCH) && honda_bosch_long) { @@ -319,6 +322,18 @@ static int honda_tx_hook(CANPacket_t *to_send) { } } + // ACCEL: safety check (radarless) + if ((addr == 0x1C8) && (bus == bus_pt)) { + int accel = (GET_BYTE(to_send, 0) << 4) | (GET_BYTE(to_send, 1) >> 4); + accel = to_signed(accel, 12); + + bool violation = false; + violation |= longitudinal_accel_checks(accel, HONDA_BOSCH_LONG_LIMITS); + if (violation) { + tx = 0; + } + } + // STEER: safety check if ((addr == 0xE4) || (addr == 0x194)) { if (!controls_allowed) { @@ -386,7 +401,7 @@ static const addr_checks* honda_bosch_init(uint16_t param) { // radar disabled so allow gas/brakes #ifdef ALLOW_DEBUG - honda_bosch_long = GET_FLAG(param, HONDA_PARAM_BOSCH_LONG) && !honda_bosch_radarless; + honda_bosch_long = GET_FLAG(param, HONDA_PARAM_BOSCH_LONG); #endif if (honda_bosch_radarless) { @@ -431,7 +446,9 @@ static int honda_bosch_fwd_hook(int bus_num, CANPacket_t *to_fwd) { if (bus_num == 2) { int addr = GET_ADDR(to_fwd); int is_lkas_msg = (addr == 0xE4) || (addr == 0xE5) || (addr == 0x33D) || (addr == 0x33DA) || (addr == 0x33DB); - if (!is_lkas_msg) { + int is_acc_msg = ((addr == 0x1C8) || (addr == 0x30C)) && honda_bosch_radarless && honda_bosch_long; + bool block_msg = is_lkas_msg || is_acc_msg; + if (!block_msg) { bus_fwd = 0; } } diff --git a/tests/safety/common.py b/tests/safety/common.py index fc63d26313..ab478b7ff6 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -837,6 +837,10 @@ def test_tx_hook_on_wrong_safety_mode(self): if attr == 'TestVolkswagenMqbLongSafety' and current_test.startswith('TestHondaNidec'): tx = list(filter(lambda m: m[0] not in [0x30c, ], tx)) + # Volkswagen MQB and Honda Bosch Radarless ACC HUD messages overlap + if attr == 'TestVolkswagenMqbLongSafety' and current_test.startswith('TestHondaBoschRadarless'): + tx = list(filter(lambda m: m[0] not in [0x30c, ], tx)) + # TODO: Temporary, should be fixed in panda firmware, safety_honda.h if attr.startswith('TestHonda'): # exceptions for common msgs across different hondas diff --git a/tests/safety/test_honda.py b/tests/safety/test_honda.py index 9a429771a0..60c322bf0e 100755 --- a/tests/safety/test_honda.py +++ b/tests/safety/test_honda.py @@ -36,6 +36,8 @@ def interceptor_msg(gas, addr): # * interceptor with alt SCM messages # * Bosch # * Bosch with Longitudinal Support +# * Bosch Radarless +# * Bosch Radarless with Longitudinal Support class HondaButtonEnableBase(common.PandaSafetyTest): @@ -522,7 +524,8 @@ def test_brake_safety_check(self): self.assertEqual(send, self._tx(self._send_gas_brake_msg(self.NO_GAS, accel)), (controls_allowed, accel)) -class TestHondaBoschRadarless(HondaPcmEnableBase, TestHondaBoschSafetyBase): +class TestHondaBoschRadarlessSafetyBase(TestHondaBoschSafetyBase): + """Base class for radarless Honda Bosch""" PT_BUS = 0 STEER_BUS = 0 BUTTONS_BUS = 2 # camera controls ACC, need to send buttons on bus 2 @@ -530,14 +533,54 @@ class TestHondaBoschRadarless(HondaPcmEnableBase, TestHondaBoschSafetyBase): TX_MSGS = [[0xE4, 0], [0x296, 2], [0x33D, 0]] FWD_BLACKLISTED_ADDRS = {2: [0xE4, 0xE5, 0x33D, 0x33DA, 0x33DB]} + @classmethod + def setUpClass(cls): + if cls.__name__ == "TestHondaBoschRadarlessSafetyBase": + cls.packer = None + cls.safety = None + raise unittest.SkipTest + def setUp(self): self.packer = CANPackerPanda("honda_civic_ex_2022_can_generated") self.safety = libpanda_py.libpanda + + # These cars do not have 0x1BE (BRAKE_MODULE) + def test_alt_disengage_on_brake(self): + pass + + +class TestHondaBoschRadarlessSafety(HondaPcmEnableBase, TestHondaBoschRadarlessSafetyBase): + """ + Covers the Honda Bosch Radarless safety mode with stock longitudinal + """ + + def setUp(self): + super().setUp() self.safety.set_safety_hooks(Panda.SAFETY_HONDA_BOSCH, Panda.FLAG_HONDA_RADARLESS) self.safety.init_tests() - def test_alt_disengage_on_brake(self): - # These cars do not have 0x1BE (BRAKE_MODULE) + +class TestHondaBoschRadarlessLongSafety(common.LongitudinalAccelSafetyTest, HondaButtonEnableBase, + TestHondaBoschRadarlessSafetyBase): + """ + Covers the Honda Bosch Radarless safety mode with longitudinal control + """ + TX_MSGS = [[0xE4, 0], [0x33D, 0], [0x1C8, 0], [0x30C, 0]] + FWD_BLACKLISTED_ADDRS = {2: [0xE4, 0xE5, 0x33D, 0x33DA, 0x33DB, 0x1C8, 0x30C]} + + def setUp(self): + super().setUp() + self.safety.set_safety_hooks(Panda.SAFETY_HONDA_BOSCH, Panda.FLAG_HONDA_RADARLESS | Panda.FLAG_HONDA_BOSCH_LONG) + self.safety.init_tests() + + def _accel_msg(self, accel): + values = { + "ACCEL_COMMAND": accel, + } + return self.packer.make_can_msg_panda("ACC_CONTROL", self.PT_BUS, values) + + # Longitudinal doesn't need to send buttons + def test_spam_cancel_safety_check(self): pass From 7392f24868ec018fb83b27cf7953761b0904d2bf Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 28 Mar 2023 00:05:35 -0700 Subject: [PATCH 10/59] Honda tests: check endswith Base to skip test (#1305) check endswith base --- tests/safety/test_honda.py | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/tests/safety/test_honda.py b/tests/safety/test_honda.py index 60c322bf0e..f98186703c 100755 --- a/tests/safety/test_honda.py +++ b/tests/safety/test_honda.py @@ -188,7 +188,7 @@ class HondaBase(common.PandaSafetyTest): @classmethod def setUpClass(cls): - if cls.__name__ == "HondaBase": + if cls.__name__.endswith("Base"): cls.packer = None cls.safety = None raise unittest.SkipTest @@ -270,12 +270,6 @@ class TestHondaNidecSafetyBase(HondaBase): INTERCEPTOR_THRESHOLD = 492 MAX_GAS = 198 - @classmethod - def setUpClass(cls): - if cls.__name__ == "TestHondaNidecSafetyBase": - cls.safety = None - raise unittest.SkipTest - def setUp(self): self.packer = CANPackerPanda("honda_civic_touring_2016_can_generated") self.safety = libpanda_py.libpanda @@ -405,13 +399,6 @@ class TestHondaBoschSafetyBase(HondaBase): TX_MSGS = [[0xE4, 0], [0xE5, 0], [0x296, 1], [0x33D, 0], [0x33DA, 0], [0x33DB, 0]] FWD_BLACKLISTED_ADDRS = {2: [0xE4, 0xE5, 0x33D, 0x33DA, 0x33DB]} - @classmethod - def setUpClass(cls): - if cls.__name__ == "TestHondaBoschSafetyBase": - cls.packer = None - cls.safety = None - raise unittest.SkipTest - def setUp(self): self.packer = CANPackerPanda("honda_accord_2018_can_generated") self.safety = libpanda_py.libpanda @@ -533,13 +520,6 @@ class TestHondaBoschRadarlessSafetyBase(TestHondaBoschSafetyBase): TX_MSGS = [[0xE4, 0], [0x296, 2], [0x33D, 0]] FWD_BLACKLISTED_ADDRS = {2: [0xE4, 0xE5, 0x33D, 0x33DA, 0x33DB]} - @classmethod - def setUpClass(cls): - if cls.__name__ == "TestHondaBoschRadarlessSafetyBase": - cls.packer = None - cls.safety = None - raise unittest.SkipTest - def setUp(self): self.packer = CANPackerPanda("honda_civic_ex_2022_can_generated") self.safety = libpanda_py.libpanda From a1e1451d1c2f23b953d82eea7691a657c9a8a75a Mon Sep 17 00:00:00 2001 From: Vivek Aithal Date: Wed, 29 Mar 2023 10:39:25 -0700 Subject: [PATCH 11/59] GM: Reduce steer down limit and increase driver allowance (#1281) reduce steer down limit and increase driver allowance --- board/safety/safety_gm.h | 4 ++-- tests/safety/test_gm.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h index 176a4e1c63..660f050cda 100644 --- a/board/safety/safety_gm.h +++ b/board/safety/safety_gm.h @@ -1,8 +1,8 @@ const SteeringLimits GM_STEERING_LIMITS = { .max_steer = 300, .max_rate_up = 10, - .max_rate_down = 25, - .driver_torque_allowance = 50, + .max_rate_down = 15, + .driver_torque_allowance = 65, .driver_torque_factor = 4, .max_rt_delta = 128, .max_rt_interval = 250000, diff --git a/tests/safety/test_gm.py b/tests/safety/test_gm.py index ff52471897..02a7b6d9d0 100755 --- a/tests/safety/test_gm.py +++ b/tests/safety/test_gm.py @@ -63,11 +63,11 @@ class TestGmSafetyBase(common.PandaSafetyTest, common.DriverTorqueSteeringSafety BRAKE_BUS = 0 # tx only MAX_RATE_UP = 10 - MAX_RATE_DOWN = 25 + MAX_RATE_DOWN = 15 MAX_TORQUE = 300 MAX_RT_DELTA = 128 RT_INTERVAL = 250000 - DRIVER_TORQUE_ALLOWANCE = 50 + DRIVER_TORQUE_ALLOWANCE = 65 DRIVER_TORQUE_FACTOR = 4 MAX_GAS = 0 From 12b9b65985edf7207a1dfd48de0b714192e6e55d Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 29 Mar 2023 15:07:12 -0700 Subject: [PATCH 12/59] fake_siren: add fault on codec enable failure (#1308) * fake_siren: add fault on codec enable failure * little shorter --------- Co-authored-by: Comma Device --- board/drivers/fake_siren.h | 8 +++++--- board/faults.h | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/board/drivers/fake_siren.h b/board/drivers/fake_siren.h index c139cb2abc..bef37ab124 100644 --- a/board/drivers/fake_siren.h +++ b/board/drivers/fake_siren.h @@ -1,13 +1,14 @@ #include "stm32h7/lli2c.h" #define CODEC_I2C_ADDR 0x10 + // 1Vpp sine wave with 1V offset const uint8_t fake_siren_lut[360] = { 134U, 135U, 137U, 138U, 139U, 140U, 141U, 143U, 144U, 145U, 146U, 148U, 149U, 150U, 151U, 152U, 154U, 155U, 156U, 157U, 158U, 159U, 160U, 162U, 163U, 164U, 165U, 166U, 167U, 168U, 169U, 170U, 171U, 172U, 174U, 175U, 176U, 177U, 177U, 178U, 179U, 180U, 181U, 182U, 183U, 184U, 185U, 186U, 186U, 187U, 188U, 189U, 190U, 190U, 191U, 192U, 193U, 193U, 194U, 195U, 195U, 196U, 196U, 197U, 197U, 198U, 199U, 199U, 199U, 200U, 200U, 201U, 201U, 202U, 202U, 202U, 203U, 203U, 203U, 203U, 204U, 204U, 204U, 204U, 204U, 204U, 204U, 205U, 205U, 205U, 205U, 205U, 205U, 205U, 204U, 204U, 204U, 204U, 204U, 204U, 204U, 203U, 203U, 203U, 203U, 202U, 202U, 202U, 201U, 201U, 200U, 200U, 199U, 199U, 199U, 198U, 197U, 197U, 196U, 196U, 195U, 195U, 194U, 193U, 193U, 192U, 191U, 190U, 190U, 189U, 188U, 187U, 186U, 186U, 185U, 184U, 183U, 182U, 181U, 180U, 179U, 178U, 177U, 177U, 176U, 175U, 174U, 172U, 171U, 170U, 169U, 168U, 167U, 166U, 165U, 164U, 163U, 162U, 160U, 159U, 158U, 157U, 156U, 155U, 154U, 152U, 151U, 150U, 149U, 148U, 146U, 145U, 144U, 143U, 141U, 140U, 139U, 138U, 137U, 135U, 134U, 133U, 132U, 130U, 129U, 128U, 127U, 125U, 124U, 123U, 122U, 121U, 119U, 118U, 117U, 116U, 115U, 113U, 112U, 111U, 110U, 109U, 108U, 106U, 105U, 104U, 103U, 102U, 101U, 100U, 99U, 98U, 97U, 96U, 95U, 94U, 93U, 92U, 91U, 90U, 89U, 88U, 87U, 86U, 85U, 84U, 83U, 82U, 82U, 81U, 80U, 79U, 78U, 78U, 77U, 76U, 76U, 75U, 74U, 74U, 73U, 72U, 72U, 71U, 71U, 70U, 70U, 69U, 69U, 68U, 68U, 67U, 67U, 67U, 66U, 66U, 66U, 65U, 65U, 65U, 65U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 63U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 64U, 65U, 65U, 65U, 65U, 66U, 66U, 66U, 67U, 67U, 67U, 68U, 68U, 69U, 69U, 70U, 70U, 71U, 71U, 72U, 72U, 73U, 74U, 74U, 75U, 76U, 76U, 77U, 78U, 78U, 79U, 80U, 81U, 82U, 82U, 83U, 84U, 85U, 86U, 87U, 88U, 89U, 90U, 91U, 92U, 93U, 94U, 95U, 96U, 97U, 98U, 99U, 100U, 101U, 102U, 103U, 104U, 105U, 106U, 108U, 109U, 110U, 111U, 112U, 113U, 115U, 116U, 117U, 118U, 119U, 121U, 122U, 123U, 124U, 125U, 127U, 128U, 129U, 130U, 132U, 133U }; bool fake_siren_enabled = false; void fake_siren_codec_enable(bool enabled) { - if(enabled) { + if (enabled) { bool success = false; success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x2B, (1U << 1)); // Left speaker mix from INA1 success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x2C, (1U << 1)); // Right speaker mix from INA1 @@ -17,11 +18,12 @@ void fake_siren_codec_enable(bool enabled) { success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x4C, (1U << 7)); // Enable INA success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x51, (1U << 7)); // Disable global shutdown if (!success) { - print("Failed to setup the codec for fake siren\n"); + print("Siren codec enable failed\n"); + fault_occurred(FAULT_SIREN_MALFUNCTION); } } else { // Disable INA input. Make sure to retry a few times if the I2C bus is busy. - for(uint8_t i=0U; i<10U; i++) { + for (uint8_t i=0U; i<10U; i++) { if (i2c_clear_reg_bits(I2C5, CODEC_I2C_ADDR, 0x4C, (1U << 7))) { break; } diff --git a/board/faults.h b/board/faults.h index c5df1131b1..e4a102a613 100644 --- a/board/faults.h +++ b/board/faults.h @@ -28,6 +28,7 @@ #define FAULT_INTERRUPT_RATE_EXTI (1U << 22) #define FAULT_INTERRUPT_RATE_SPI (1U << 23) #define FAULT_INTERRUPT_RATE_UART_7 (1U << 24) +#define FAULT_SIREN_MALFUNCTION (1U << 25) // Permanent faults #define PERMANENT_FAULTS 0U From 9349337ebb7cad601ef881265af1d27a2115d1b9 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 30 Mar 2023 22:14:35 -0700 Subject: [PATCH 13/59] IsoTpMessage: check CAN frame length (#1312) * add some more sanity checks for _isotp_rx_next * another PR * revert --- python/uds.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/uds.py b/python/uds.py index 5bf35088ea..c95074a45e 100644 --- a/python/uds.py +++ b/python/uds.py @@ -457,6 +457,9 @@ def recv(self, timeout=None) -> Tuple[Optional[bytes], bool]: print(f"ISO-TP: RESPONSE - {hex(self._can_client.rx_addr)} 0x{bytes.hex(self.rx_dat)}") def _isotp_rx_next(self, rx_data: bytes) -> None: + # ISO 15765-2 specifies an eight byte CAN frame for ISO-TP communication + assert len(rx_data) == 8, f"isotp - rx: invalid CAN frame length: {len(rx_data)}" + # single rx_frame if rx_data[0] >> 4 == 0x0: self.rx_len = rx_data[0] & 0xFF From 23563cec7c03a5a3718b29ec206fed1c8cee8edd Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 30 Mar 2023 22:35:02 -0700 Subject: [PATCH 14/59] IsoTpMessage: check not reserved frame (#1313) * add some more sanity checks for _isotp_rx_next * another PR * rever * can remove return * add back the space here tho :( * raise --- python/uds.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/python/uds.py b/python/uds.py index c95074a45e..b58ebffc53 100644 --- a/python/uds.py +++ b/python/uds.py @@ -468,10 +468,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: self.rx_done = True if self.debug: print(f"ISO-TP: RX - single frame - {hex(self._can_client.rx_addr)} idx={self.rx_idx} done={self.rx_done}") - return # first rx_frame - if rx_data[0] >> 4 == 0x1: + elif rx_data[0] >> 4 == 0x1: self.rx_len = ((rx_data[0] & 0x0F) << 8) + rx_data[1] self.rx_dat = rx_data[2:] self.rx_idx = 0 @@ -482,10 +481,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: print(f"ISO-TP: TX - flow control continue - {hex(self._can_client.tx_addr)}") # send flow control message self._can_client.send([self.flow_control_msg]) - return # consecutive rx frame - if rx_data[0] >> 4 == 0x2: + elif rx_data[0] >> 4 == 0x2: assert not self.rx_done, "isotp - rx: consecutive frame with no active frame" self.rx_idx += 1 assert self.rx_idx & 0xF == rx_data[0] & 0xF, "isotp - rx: invalid consecutive frame index" @@ -498,10 +496,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: self._can_client.send([self.flow_control_msg]) if self.debug: print(f"ISO-TP: RX - consecutive frame - {hex(self._can_client.rx_addr)} idx={self.rx_idx} done={self.rx_done}") - return # flow control - if rx_data[0] >> 4 == 0x3: + elif rx_data[0] >> 4 == 0x3: assert not self.tx_done, "isotp - rx: flow control with no active frame" assert rx_data[0] != 0x32, "isotp - rx: flow-control overflow/abort" assert rx_data[0] == 0x30 or rx_data[0] == 0x31, "isotp - rx: flow-control transfer state indicator invalid" @@ -535,8 +532,14 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: if self.debug: print(f"ISO-TP: TX - flow control wait - {hex(self._can_client.tx_addr)}") + # 4-15 - reserved + else: + raise Exception(f"isotp - rx: invalid frame type: {rx_data[0] >> 4}") + + FUNCTIONAL_ADDRS = [0x7DF, 0x18DB33F1] + def get_rx_addr_for_tx_addr(tx_addr, rx_offset=0x8): if tx_addr in FUNCTIONAL_ADDRS: return None From 7933635b54b68eda3f8a906d5e2c3ca30dbed279 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 30 Mar 2023 23:23:42 -0700 Subject: [PATCH 15/59] IsoTpMessage: rx_len sanity checks (#1311) * add some more sanity checks for _isotp_rx_next * another PR * Update python/uds.py * cannot be bigger than 0xfff --- python/uds.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/uds.py b/python/uds.py index b58ebffc53..83d7a54af0 100644 --- a/python/uds.py +++ b/python/uds.py @@ -463,6 +463,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # single rx_frame if rx_data[0] >> 4 == 0x0: self.rx_len = rx_data[0] & 0xFF + assert self.rx_len <= 0x7, f"isotp - rx: invalid single frame length: {self.rx_len}" self.rx_dat = rx_data[1:1 + self.rx_len] self.rx_idx = 0 self.rx_done = True @@ -472,6 +473,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # first rx_frame elif rx_data[0] >> 4 == 0x1: self.rx_len = ((rx_data[0] & 0x0F) << 8) + rx_data[1] + assert 0x8 <= self.rx_len, f"isotp - rx: invalid first frame length: {self.rx_len}" self.rx_dat = rx_data[2:] self.rx_idx = 0 self.rx_done = False From 09fee3e7ead96a6a9613430c81bb2d3e095aa0fe Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 12:11:13 -0700 Subject: [PATCH 16/59] Revert "IsoTpMessage: check CAN frame length (#1312)" This reverts commit 9349337ebb7cad601ef881265af1d27a2115d1b9. --- python/uds.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/python/uds.py b/python/uds.py index 83d7a54af0..5e2f50d9f8 100644 --- a/python/uds.py +++ b/python/uds.py @@ -457,9 +457,6 @@ def recv(self, timeout=None) -> Tuple[Optional[bytes], bool]: print(f"ISO-TP: RESPONSE - {hex(self._can_client.rx_addr)} 0x{bytes.hex(self.rx_dat)}") def _isotp_rx_next(self, rx_data: bytes) -> None: - # ISO 15765-2 specifies an eight byte CAN frame for ISO-TP communication - assert len(rx_data) == 8, f"isotp - rx: invalid CAN frame length: {len(rx_data)}" - # single rx_frame if rx_data[0] >> 4 == 0x0: self.rx_len = rx_data[0] & 0xFF From 02eb84936b62cc37d7e3f1753cb501ebe547bf27 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 13:59:02 -0700 Subject: [PATCH 17/59] UdsClient: support sub addresses (#1317) * UdsClient: support sub addresses * Update python/uds.py * Update python/uds.py --- python/uds.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/python/uds.py b/python/uds.py index 5e2f50d9f8..9fd179cc3b 100644 --- a/python/uds.py +++ b/python/uds.py @@ -556,15 +556,16 @@ def get_rx_addr_for_tx_addr(tx_addr, rx_offset=0x8): class UdsClient(): - def __init__(self, panda, tx_addr: int, rx_addr: int = None, bus: int = 0, timeout: float = 1, debug: bool = False, - tx_timeout: float = 1, response_pending_timeout: float = 10): + def __init__(self, panda, tx_addr: int, rx_addr: int = None, bus: int = 0, sub_addr: int = None, timeout: float = 1, + debug: bool = False, tx_timeout: float = 1, response_pending_timeout: float = 10): self.bus = bus self.tx_addr = tx_addr self.rx_addr = rx_addr if rx_addr is not None else get_rx_addr_for_tx_addr(tx_addr) + self.sub_addr = sub_addr self.timeout = timeout self.debug = debug can_send_with_timeout = partial(panda.can_send, timeout=int(tx_timeout*1000)) - self._can_client = CanClient(can_send_with_timeout, panda.can_recv, self.tx_addr, self.rx_addr, self.bus, debug=self.debug) + self._can_client = CanClient(can_send_with_timeout, panda.can_recv, self.tx_addr, self.rx_addr, self.bus, self.sub_addr, debug=self.debug) self.response_pending_timeout = response_pending_timeout # generic uds request @@ -576,7 +577,8 @@ def _uds_request(self, service_type: SERVICE_TYPE, subfunction: int = None, data req += data # send request, wait for response - isotp_msg = IsoTpMessage(self._can_client, timeout=self.timeout, debug=self.debug) + max_len = 8 if self.sub_addr is None else 7 + isotp_msg = IsoTpMessage(self._can_client, timeout=self.timeout, debug=self.debug, max_len=max_len) isotp_msg.send(req) response_pending = False while True: From cb3530916064b147cb7b8539035c7c0689d6ee47 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Fri, 31 Mar 2023 14:39:30 -0700 Subject: [PATCH 18/59] spi tx timeout print --- board/drivers/spi.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/board/drivers/spi.h b/board/drivers/spi.h index f7382e469b..8723b5a29d 100644 --- a/board/drivers/spi.h +++ b/board/drivers/spi.h @@ -145,6 +145,10 @@ void spi_handle_rx(void) { } void spi_handle_tx(bool timed_out) { + if (timed_out) { + print("SPI: TX timeout\n"); + } + if ((spi_state == SPI_STATE_HEADER_NACK) || timed_out) { // Reset state spi_state = SPI_STATE_HEADER; From c92f8ecfde847edea7778f75c25f05047b770c77 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 19:17:16 -0700 Subject: [PATCH 19/59] IsoTpMessage: fix rx_len sanity checks (#1319) fix a bug --- python/uds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/uds.py b/python/uds.py index 9fd179cc3b..4fbff043b1 100644 --- a/python/uds.py +++ b/python/uds.py @@ -460,7 +460,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # single rx_frame if rx_data[0] >> 4 == 0x0: self.rx_len = rx_data[0] & 0xFF - assert self.rx_len <= 0x7, f"isotp - rx: invalid single frame length: {self.rx_len}" + assert self.rx_len < self.max_len, f"isotp - rx: invalid single frame length: {self.rx_len}" self.rx_dat = rx_data[1:1 + self.rx_len] self.rx_idx = 0 self.rx_done = True @@ -470,7 +470,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # first rx_frame elif rx_data[0] >> 4 == 0x1: self.rx_len = ((rx_data[0] & 0x0F) << 8) + rx_data[1] - assert 0x8 <= self.rx_len, f"isotp - rx: invalid first frame length: {self.rx_len}" + assert self.max_len <= self.rx_len, f"isotp - rx: invalid first frame length: {self.rx_len}" self.rx_dat = rx_data[2:] self.rx_idx = 0 self.rx_done = False From 189f0436ba3625b938015fc59e68a53a5bdba143 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 19:22:16 -0700 Subject: [PATCH 20/59] IsoTpMessage: check CAN frame length (#1315) * Revert "Revert "IsoTpMessage: check CAN frame length (#1312)"" This reverts commit 09fee3e7ead96a6a9613430c81bb2d3e095aa0fe. * need to pad for the check to work * pass rx_offset up from can client * detect from internal can_client and just use it for the checks * move * revert this for another PR * fix --- python/uds.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/uds.py b/python/uds.py index 4fbff043b1..64ac397967 100644 --- a/python/uds.py +++ b/python/uds.py @@ -457,6 +457,9 @@ def recv(self, timeout=None) -> Tuple[Optional[bytes], bool]: print(f"ISO-TP: RESPONSE - {hex(self._can_client.rx_addr)} 0x{bytes.hex(self.rx_dat)}") def _isotp_rx_next(self, rx_data: bytes) -> None: + # ISO 15765-2 specifies an eight byte CAN frame for ISO-TP communication + assert len(rx_data) == self.max_len, f"isotp - rx: invalid CAN frame length: {len(rx_data)}" + # single rx_frame if rx_data[0] >> 4 == 0x0: self.rx_len = rx_data[0] & 0xFF From a12c0a795678373d946b644b43d9402b72e502a9 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 19:44:46 -0700 Subject: [PATCH 21/59] IsoTpMessage: don't skip a byte when sending consecutive frames to subaddress (#1320) handle --- python/uds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/uds.py b/python/uds.py index 64ac397967..6f6922a0bc 100644 --- a/python/uds.py +++ b/python/uds.py @@ -514,7 +514,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # first frame = 6 bytes, each consecutive frame = 7 bytes num_bytes = self.max_len - 1 - start = 6 + self.tx_idx * num_bytes + start = self.max_len - 2 + self.tx_idx * num_bytes count = rx_data[1] end = start + count * num_bytes if count > 0 else self.tx_len tx_msgs = [] From 8efbcf041c1b6da1764bb56f822609fd4bb0758f Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 31 Mar 2023 20:11:53 -0700 Subject: [PATCH 22/59] IsoTpMessage: return if consecutive frame last received (#1314) * what about * actually this is what we want * more explicit about this condition * frame type * add enum * use in func --- python/uds.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/python/uds.py b/python/uds.py index 6f6922a0bc..f727b7507e 100644 --- a/python/uds.py +++ b/python/uds.py @@ -141,6 +141,12 @@ class DYNAMIC_DEFINITION_TYPE(IntEnum): DEFINE_BY_MEMORY_ADDRESS = 2 CLEAR_DYNAMICALLY_DEFINED_DATA_IDENTIFIER = 3 +class ISOTP_FRAME_TYPE(IntEnum): + SINGLE = 0 + FIRST = 1 + CONSECUTIVE = 2 + FLOW = 3 + class DynamicSourceDefinition(NamedTuple): data_identifier: int position: int @@ -438,30 +444,29 @@ def recv(self, timeout=None) -> Tuple[Optional[bytes], bool]: timeout = self.timeout start_time = time.monotonic() - updated = False + rx_in_progress = False try: while True: for msg in self._can_client.recv(): - self._isotp_rx_next(msg) + frame_type = self._isotp_rx_next(msg) start_time = time.monotonic() - updated = True + rx_in_progress = frame_type == ISOTP_FRAME_TYPE.CONSECUTIVE if self.tx_done and self.rx_done: - return self.rx_dat, updated + return self.rx_dat, False # no timeout indicates non-blocking if timeout == 0: - return None, updated + return None, rx_in_progress if time.monotonic() - start_time > timeout: raise MessageTimeoutError("timeout waiting for response") finally: if self.debug and self.rx_dat: print(f"ISO-TP: RESPONSE - {hex(self._can_client.rx_addr)} 0x{bytes.hex(self.rx_dat)}") - def _isotp_rx_next(self, rx_data: bytes) -> None: + def _isotp_rx_next(self, rx_data: bytes) -> ISOTP_FRAME_TYPE: # ISO 15765-2 specifies an eight byte CAN frame for ISO-TP communication assert len(rx_data) == self.max_len, f"isotp - rx: invalid CAN frame length: {len(rx_data)}" - # single rx_frame - if rx_data[0] >> 4 == 0x0: + if rx_data[0] >> 4 == ISOTP_FRAME_TYPE.SINGLE: self.rx_len = rx_data[0] & 0xFF assert self.rx_len < self.max_len, f"isotp - rx: invalid single frame length: {self.rx_len}" self.rx_dat = rx_data[1:1 + self.rx_len] @@ -469,9 +474,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: self.rx_done = True if self.debug: print(f"ISO-TP: RX - single frame - {hex(self._can_client.rx_addr)} idx={self.rx_idx} done={self.rx_done}") + return ISOTP_FRAME_TYPE.SINGLE - # first rx_frame - elif rx_data[0] >> 4 == 0x1: + elif rx_data[0] >> 4 == ISOTP_FRAME_TYPE.FIRST: self.rx_len = ((rx_data[0] & 0x0F) << 8) + rx_data[1] assert self.max_len <= self.rx_len, f"isotp - rx: invalid first frame length: {self.rx_len}" self.rx_dat = rx_data[2:] @@ -483,9 +488,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: print(f"ISO-TP: TX - flow control continue - {hex(self._can_client.tx_addr)}") # send flow control message self._can_client.send([self.flow_control_msg]) + return ISOTP_FRAME_TYPE.FIRST - # consecutive rx frame - elif rx_data[0] >> 4 == 0x2: + elif rx_data[0] >> 4 == ISOTP_FRAME_TYPE.CONSECUTIVE: assert not self.rx_done, "isotp - rx: consecutive frame with no active frame" self.rx_idx += 1 assert self.rx_idx & 0xF == rx_data[0] & 0xF, "isotp - rx: invalid consecutive frame index" @@ -498,9 +503,9 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: self._can_client.send([self.flow_control_msg]) if self.debug: print(f"ISO-TP: RX - consecutive frame - {hex(self._can_client.rx_addr)} idx={self.rx_idx} done={self.rx_done}") + return ISOTP_FRAME_TYPE.CONSECUTIVE - # flow control - elif rx_data[0] >> 4 == 0x3: + elif rx_data[0] >> 4 == ISOTP_FRAME_TYPE.FLOW: assert not self.tx_done, "isotp - rx: flow control with no active frame" assert rx_data[0] != 0x32, "isotp - rx: flow-control overflow/abort" assert rx_data[0] == 0x30 or rx_data[0] == 0x31, "isotp - rx: flow-control transfer state indicator invalid" @@ -533,6 +538,7 @@ def _isotp_rx_next(self, rx_data: bytes) -> None: # wait (do nothing until next flow control message) if self.debug: print(f"ISO-TP: TX - flow control wait - {hex(self._can_client.tx_addr)}") + return ISOTP_FRAME_TYPE.FLOW # 4-15 - reserved else: From 6a8ed7f1477057144f6a9cd6e1003e76afcf2b9f Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 1 Apr 2023 18:52:32 -0700 Subject: [PATCH 23/59] CI: setup dos (#1321) * CI: setup dos * just build and flash for now * fix new device setup --- Jenkinsfile | 130 +++++++++++++++++++++++++--------- tests/ci_reset_internal_hw.py | 47 ++++++++++++ tests/setup_device_ci.sh | 79 +++++++++++++++++++++ 3 files changed, 222 insertions(+), 34 deletions(-) create mode 100755 tests/ci_reset_internal_hw.py create mode 100755 tests/setup_device_ci.sh diff --git a/Jenkinsfile b/Jenkinsfile index 4d850e0b08..e6c4bb7ca5 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -12,11 +12,59 @@ def docker_run(String step_label, int timeout_mins, String cmd) { } } + +def phone(String ip, String step_label, String cmd) { + withCredentials([file(credentialsId: 'id_rsa', variable: 'key_file')]) { + def ssh_cmd = """ +ssh -tt -o StrictHostKeyChecking=no -i ${key_file} 'comma@${ip}' /usr/bin/bash <<'END' + +set -e + + +source ~/.bash_profile +if [ -f /etc/profile ]; then + source /etc/profile +fi + +export CI=1 +export TEST_DIR=${env.TEST_DIR} +export SOURCE_DIR=${env.SOURCE_DIR} +export GIT_BRANCH=${env.GIT_BRANCH} +export GIT_COMMIT=${env.GIT_COMMIT} +export PYTHONPATH=${env.TEST_DIR}/../ + +cd ${env.TEST_DIR} || true +${cmd} +exit 0 + +END""" + + sh script: ssh_cmd, label: step_label + } +} + +def phone_steps(String device_type, steps) { + lock(resource: "", label: device_type, inversePrecedence: true, variable: 'device_ip', quantity: 1) { + timeout(time: 20, unit: 'MINUTES') { + phone(device_ip, "git checkout", readFile("tests/setup_device_ci.sh"),) + steps.each { item -> + phone(device_ip, item[0], item[1]) + } + } + } +} + + + pipeline { agent any environment { + CI = "1" PARTIAL_TESTS = "${env.BRANCH_NAME == 'master' ? ' ' : '1'}" DOCKER_IMAGE_TAG = "panda:build-${env.GIT_COMMIT}" + + TEST_DIR = "/data/panda" + SOURCE_DIR = "/data/panda_source/" } options { timeout(time: 3, unit: 'HOURS') @@ -24,46 +72,60 @@ pipeline { } stages { - stage ('Acquire resource locks') { - options { - lock(resource: "pandas") - } - stages { - stage('Build Docker Image') { + stage('panda tests') { + parallel { + stage('test dos') { + agent { docker { image 'ghcr.io/commaai/alpine-ssh'; args '--user=root' } } steps { - timeout(time: 20, unit: 'MINUTES') { - script { - sh 'git archive -v -o panda.tar.gz --format=tar.gz HEAD' - dockerImage = docker.build("${env.DOCKER_IMAGE_TAG}") - } - } + phone_steps("panda-dos", [ + ["build", "scons -j4"], + ["flash", "cd tests/ && ./ci_reset_internal_hw.py"], + ]) } } - stage('prep') { - steps { - script { - docker_run("reset hardware", 3, "python ./tests/ci_reset_hw.py") - } + + stage ('Acquire resource locks') { + options { + lock(resource: "pandas") } - } - stage('pedal tests') { - steps { - script { - docker_run("test pedal", 1, "PEDAL_JUNGLE=058010800f51363038363036 python ./tests/pedal/test_pedal.py") + stages { + stage('Build Docker Image') { + steps { + timeout(time: 20, unit: 'MINUTES') { + script { + sh 'git archive -v -o panda.tar.gz --format=tar.gz HEAD' + dockerImage = docker.build("${env.DOCKER_IMAGE_TAG}") + } + } + } } - } - } - stage('HITL tests') { - steps { - script { - docker_run("HITL tests", 35, 'PANDAS_JUNGLE=23002d000851393038373731 PANDAS_EXCLUDE="1d0002000c51303136383232 2f002e000c51303136383232" ./tests/hitl/test.sh') + stage('prep') { + steps { + script { + docker_run("reset hardware", 3, "python ./tests/ci_reset_hw.py") + } + } } - } - } - stage('CANFD tests') { - steps { - script { - docker_run("CANFD tets", 6, 'JUNGLE=058010800f51363038363036 H7_PANDAS_EXCLUDE="080021000c51303136383232 33000e001051393133353939" ./tests/canfd/test_canfd.py') + stage('pedal tests') { + steps { + script { + docker_run("test pedal", 1, "PEDAL_JUNGLE=058010800f51363038363036 python ./tests/pedal/test_pedal.py") + } + } + } + stage('HITL tests') { + steps { + script { + docker_run("HITL tests", 35, 'PANDAS_JUNGLE=23002d000851393038373731 PANDAS_EXCLUDE="1d0002000c51303136383232 2f002e000c51303136383232" ./tests/hitl/test.sh') + } + } + } + stage('CANFD tests') { + steps { + script { + docker_run("CANFD tets", 6, 'JUNGLE=058010800f51363038363036 H7_PANDAS_EXCLUDE="080021000c51303136383232 33000e001051393133353939" ./tests/canfd/test_canfd.py') + } + } } } } diff --git a/tests/ci_reset_internal_hw.py b/tests/ci_reset_internal_hw.py new file mode 100755 index 0000000000..e18b966b6f --- /dev/null +++ b/tests/ci_reset_internal_hw.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python3 +import time +from panda import Panda, PandaDFU + +class GPIO: + STM_RST_N = 124 + STM_BOOT0 = 134 + HUB_RST_N = 30 + + +def gpio_init(pin, output): + with open(f"/sys/class/gpio/gpio{pin}/direction", 'wb') as f: + f.write(b"out" if output else b"in") + +def gpio_set(pin, high): + with open(f"/sys/class/gpio/gpio{pin}/value", 'wb') as f: + f.write(b"1" if high else b"0") + + +if __name__ == "__main__": + for pin in (GPIO.STM_RST_N, GPIO.STM_BOOT0, GPIO.HUB_RST_N): + gpio_init(pin, True) + + # reset USB hub + gpio_set(GPIO.HUB_RST_N, 0) + time.sleep(0.5) + gpio_set(GPIO.HUB_RST_N, 1) + + # flash bootstub + gpio_set(GPIO.STM_RST_N, 1) + gpio_set(GPIO.STM_BOOT0, 1) + time.sleep(1) + gpio_set(GPIO.STM_RST_N, 0) + gpio_set(GPIO.STM_BOOT0, 0) + time.sleep(1) + + PandaDFU(None).recover() + + gpio_set(GPIO.STM_RST_N, 1) + time.sleep(0.5) + gpio_set(GPIO.STM_RST_N, 0) + time.sleep(1) + + # flash app + p = Panda() + assert p.bootstub + p.flash() diff --git a/tests/setup_device_ci.sh b/tests/setup_device_ci.sh new file mode 100755 index 0000000000..6bf0e509fb --- /dev/null +++ b/tests/setup_device_ci.sh @@ -0,0 +1,79 @@ +#!/usr/bin/bash + +set -e + +if [ -z "$SOURCE_DIR" ]; then + echo "SOURCE_DIR must be set" + exit 1 +fi + +if [ -z "$GIT_COMMIT" ]; then + echo "GIT_COMMIT must be set" + exit 1 +fi + +if [ -z "$TEST_DIR" ]; then + echo "TEST_DIR must be set" + exit 1 +fi + +CONTINUE_PATH="/data/continue.sh" +tee $CONTINUE_PATH << EOF +#!/usr/bin/bash + +sudo abctl --set_success + +# patch sshd config +sudo mount -o rw,remount / +echo tici-$(cat /proc/cmdline | sed -e 's/^.*androidboot.serialno=//' -e 's/ .*$//') | sudo tee /etc/hostname +sudo sed -i "s,/data/params/d/GithubSshKeys,/usr/comma/setup_keys," /etc/ssh/sshd_config +sudo systemctl daemon-reload +sudo systemctl restart ssh +sudo systemctl disable ssh-param-watcher.path +sudo systemctl disable ssh-param-watcher.service +sudo mount -o ro,remount / + +while true; do + if ! sudo systemctl is-active -q ssh; then + sudo systemctl start ssh + fi + sleep 5s +done + +sleep infinity +EOF +chmod +x $CONTINUE_PATH + + +# set up environment +if [ ! -d "$SOURCE_DIR" ]; then + git clone https://github.com/commaai/panda.git $SOURCE_DIR +fi + +# setup panda_jungle +cd $SOURCE_DIR/../ +if [ ! -d panda_jungle/ ]; then + git clone https://github.com/commaai/panda_jungle.git +fi +cd panda_jungle +git fetch --all +git checkout -f master +git reset --hard origin/master + +# checkout panda commit +cd $SOURCE_DIR + +rm -f .git/index.lock +git reset --hard +git fetch --no-tags --no-recurse-submodules -j4 --verbose --depth 1 origin $GIT_COMMIT +find . -maxdepth 1 -not -path './.git' -not -name '.' -not -name '..' -exec rm -rf '{}' \; +git reset --hard $GIT_COMMIT +git checkout $GIT_COMMIT +git clean -xdff + +echo "git checkout done, t=$SECONDS" +du -hs $SOURCE_DIR $SOURCE_DIR/.git + +rsync -a --delete $SOURCE_DIR $TEST_DIR + +echo "$TEST_DIR synced with $GIT_COMMIT, t=$SECONDS" From 416981d1f73aee8ffc4e936ecd79f47b35294a7b Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 1 Apr 2023 19:28:54 -0700 Subject: [PATCH 24/59] CI: setup tres (#1322) --- Jenkinsfile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index e6c4bb7ca5..988b43c2f8 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -84,6 +84,16 @@ pipeline { } } + stage('test tres') { + agent { docker { image 'ghcr.io/commaai/alpine-ssh'; args '--user=root' } } + steps { + phone_steps("panda-tres", [ + ["build", "scons -j4"], + ["flash", "cd tests/ && ./ci_reset_internal_hw.py"], + ]) + } + } + stage ('Acquire resource locks') { options { lock(resource: "pandas") From 3e89b7127ad6162ccbed29ad4af10a54528ed058 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 1 Apr 2023 23:09:12 -0700 Subject: [PATCH 25/59] python: fix libusb deprecation warning (#1302) * update list * this one too * update dfu --- python/__init__.py | 29 +++++++++++++++-------------- python/dfu.py | 15 ++++++++------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/python/__init__.py b/python/__init__.py index 5d6f3e9fc4..e396fb36f2 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -315,8 +315,9 @@ def spi_connect(serial): @staticmethod def usb_connect(serial, claim=True, wait=False): handle, usb_serial, bootstub, bcd = None, None, None, None - context = usb1.USBContext() while 1: + context = usb1.USBContext() + context.open() try: for device in context.getDeviceList(skip_on_error=True): if device.getVendorID() == 0xbbaa and device.getProductID() in (0xddcc, 0xddee): @@ -347,7 +348,7 @@ def usb_connect(serial, claim=True, wait=False): logging.exception("USB connect error") if not wait or handle is not None: break - context = usb1.USBContext() # New context needed so new devices show up + context.close() usb_handle = None if handle is not None: @@ -363,21 +364,21 @@ def list(): @staticmethod def usb_list(): - context = usb1.USBContext() ret = [] try: - for device in context.getDeviceList(skip_on_error=True): - if device.getVendorID() == 0xbbaa and device.getProductID() in (0xddcc, 0xddee): - try: - serial = device.getSerialNumber() - if len(serial) == 24: - ret.append(serial) - else: - warnings.warn(f"found device with panda descriptors but invalid serial: {serial}", RuntimeWarning) - except Exception: - continue + with usb1.USBContext() as context: + for device in context.getDeviceList(skip_on_error=True): + if device.getVendorID() == 0xbbaa and device.getProductID() in (0xddcc, 0xddee): + try: + serial = device.getSerialNumber() + if len(serial) == 24: + ret.append(serial) + else: + warnings.warn(f"found device with panda descriptors but invalid serial: {serial}", RuntimeWarning) + except Exception: + continue except Exception: - pass + logging.exception("exception while listing pandas") return ret @staticmethod diff --git a/python/dfu.py b/python/dfu.py index 15e018c5ba..f724bf15d9 100644 --- a/python/dfu.py +++ b/python/dfu.py @@ -27,6 +27,7 @@ def __init__(self, dfu_serial: Optional[str]): def usb_connect(dfu_serial: Optional[str]) -> Optional[STBootloaderUSBHandle]: handle = None context = usb1.USBContext() + context.open() for device in context.getDeviceList(skip_on_error=True): if device.getVendorID() == 0x0483 and device.getProductID() == 0xdf11: try: @@ -64,15 +65,15 @@ def list() -> List[str]: @staticmethod def usb_list() -> List[str]: - context = usb1.USBContext() dfu_serials = [] try: - for device in context.getDeviceList(skip_on_error=True): - if device.getVendorID() == 0x0483 and device.getProductID() == 0xdf11: - try: - dfu_serials.append(device.open().getASCIIStringDescriptor(3)) - except Exception: - pass + with usb1.USBContext() as context: + for device in context.getDeviceList(skip_on_error=True): + if device.getVendorID() == 0x0483 and device.getProductID() == 0xdf11: + try: + dfu_serials.append(device.open().getASCIIStringDescriptor(3)) + except Exception: + pass except Exception: pass return dfu_serials From b6c378ad02cd7902ba8a13572b08e9c9d4ed3070 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sun, 2 Apr 2023 09:33:35 -0700 Subject: [PATCH 26/59] CI: set PYTHONWARNINGS=error (#1323) * CI: set PYTHONWARNINGS=error * update resetter * fix build warnings * bump jungle * fix one more * fix linter --------- Co-authored-by: Bruce Wayne --- .github/workflows/test.yaml | 2 ++ Dockerfile | 2 +- Jenkinsfile | 3 +++ board/SConscript | 3 ++- crypto/sign.py | 44 +++++++++++++++++++------------------ python/__init__.py | 6 ++--- tests/libs/resetter.py | 28 ++++++++++------------- tests/pedal/test_pedal.py | 1 + 8 files changed, 46 insertions(+), 43 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 68df63b136..7e3574f05e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -12,6 +12,8 @@ env: export DOCKER_BUILDKIT=1 docker build --pull --build-arg BUILDKIT_INLINE_CACHE=1 --cache-from ghcr.io/commaai/panda:latest -t panda -f Dockerfile . + PYTHONWARNINGS: "error" + jobs: docker_push: name: docker push diff --git a/Dockerfile b/Dockerfile index 8757c4fac8..235476b5c0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -83,7 +83,7 @@ RUN cd /tmp/openpilot && \ git clone https://github.com/commaai/panda_jungle.git && \ cd panda_jungle && \ git fetch && \ - git checkout 7b7197c605915ac34f3d62f314edd84e2e78a759 && \ + git checkout 3a791be1f1877a69cf45de16a670992380622297 && \ rm -rf .git/ RUN cd /tmp/openpilot && \ diff --git a/Jenkinsfile b/Jenkinsfile index 988b43c2f8..3337ffd0d5 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -2,6 +2,7 @@ def docker_run(String step_label, int timeout_mins, String cmd) { timeout(time: timeout_mins, unit: 'MINUTES') { sh script: "docker run --rm --privileged \ --env PARTIAL_TESTS=${env.PARTIAL_TESTS} \ + --env PYTHONWARNINGS=error \ --volume /dev/bus/usb:/dev/bus/usb \ --volume /var/run/dbus:/var/run/dbus \ --workdir /tmp/openpilot/panda \ @@ -32,6 +33,7 @@ export SOURCE_DIR=${env.SOURCE_DIR} export GIT_BRANCH=${env.GIT_BRANCH} export GIT_COMMIT=${env.GIT_COMMIT} export PYTHONPATH=${env.TEST_DIR}/../ +export PYTHONWARNINGS=error cd ${env.TEST_DIR} || true ${cmd} @@ -61,6 +63,7 @@ pipeline { environment { CI = "1" PARTIAL_TESTS = "${env.BRANCH_NAME == 'master' ? ' ' : '1'}" + PYTHONWARNINGS= "error" DOCKER_IMAGE_TAG = "panda:build-${env.GIT_COMMIT}" TEST_DIR = "/data/panda" diff --git a/board/SConscript b/board/SConscript index 7c91279047..dc095e712a 100644 --- a/board/SConscript +++ b/board/SConscript @@ -102,7 +102,8 @@ def get_key_header(name): from Crypto.PublicKey import RSA public_fn = File(f'../certs/{name}.pub').srcnode().abspath - rsa = RSA.importKey(open(public_fn).read()) + with open(public_fn) as f: + rsa = RSA.importKey(f.read()) assert(rsa.size_in_bits() == 1024) rr = pow(2**1024, 2, rsa.n) diff --git a/crypto/sign.py b/crypto/sign.py index e199a6b148..daea38630c 100755 --- a/crypto/sign.py +++ b/crypto/sign.py @@ -9,28 +9,30 @@ # increment this to make new hardware not run old versions VERSION = 2 -rsa = RSA.importKey(open(sys.argv[3]).read()) +if __name__ == "__main__": + with open(sys.argv[3]) as k: + rsa = RSA.importKey(k.read()) -with open(sys.argv[1], "rb") as f: - dat = f.read() + with open(sys.argv[1], "rb") as f: + dat = f.read() -print("signing", len(dat), "bytes") + print("signing", len(dat), "bytes") -with open(sys.argv[2], "wb") as f: - if os.getenv("SETLEN") is not None: - # add the version at the end - dat += b"VERS" + struct.pack("I", VERSION) - # add the length at the beginning - x = struct.pack("I", len(dat)) + dat[4:] - # mock signature of dat[4:] - dd = hashlib.sha1(dat[4:]).digest() - else: - x = dat - dd = hashlib.sha1(dat).digest() + with open(sys.argv[2], "wb") as f: + if os.getenv("SETLEN") is not None: + # add the version at the end + dat += b"VERS" + struct.pack("I", VERSION) + # add the length at the beginning + x = struct.pack("I", len(dat)) + dat[4:] + # mock signature of dat[4:] + dd = hashlib.sha1(dat[4:]).digest() + else: + x = dat + dd = hashlib.sha1(dat).digest() - print("hash:", str(binascii.hexlify(dd), "utf-8")) - dd = b"\x00\x01" + b"\xff" * 0x69 + b"\x00" + dd - rsa_out = pow(int.from_bytes(dd, byteorder='big', signed=False), rsa.d, rsa.n) - sig = (hex(rsa_out)[2:].rjust(0x100, '0')) - x += binascii.unhexlify(sig) - f.write(x) + print("hash:", str(binascii.hexlify(dd), "utf-8")) + dd = b"\x00\x01" + b"\xff" * 0x69 + b"\x00" + dd + rsa_out = pow(int.from_bytes(dd, byteorder='big', signed=False), rsa.d, rsa.n) + sig = (hex(rsa_out)[2:].rjust(0x100, '0')) + x += binascii.unhexlify(sig) + f.write(x) diff --git a/python/__init__.py b/python/__init__.py index e396fb36f2..44e9d80d5a 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -612,9 +612,9 @@ def get_version(self): @staticmethod def get_signature_from_firmware(fn) -> bytes: - f = open(fn, 'rb') - f.seek(-128, 2) # Seek from end of file - return f.read(128) + with open(fn, 'rb') as f: + f.seek(-128, 2) # Seek from end of file + return f.read(128) def get_signature(self) -> bytes: part_1 = self._handle.controlRead(Panda.REQUEST_IN, 0xd3, 0, 0, 0x40) diff --git a/tests/libs/resetter.py b/tests/libs/resetter.py index c9dead2e7e..720ae9c21f 100644 --- a/tests/libs/resetter.py +++ b/tests/libs/resetter.py @@ -9,31 +9,25 @@ def __init__(self): def close(self): self._handle.close() + self._context.close() self._handle = None def connect(self): if self._handle: self.close() - context = usb1.USBContext() self._handle = None - while True: - try: - for device in context.getDeviceList(skip_on_error=True): - if device.getVendorID() == 0xbbaa and device.getProductID() == 0xddc0: - try: - self._handle = device.open() - self._handle.claimInterface(0) - break - except Exception as e: - print(e) - continue - except Exception as e: - print(e) - if self._handle: - break - context = usb1.USBContext() + self._context = usb1.USBContext() + self._context.open() + for device in self._context.getDeviceList(skip_on_error=True): + if device.getVendorID() == 0xbbaa and device.getProductID() == 0xddc0: + try: + self._handle = device.open() + self._handle.claimInterface(0) + break + except Exception as e: + print(e) assert self._handle def enable_power(self, port, enabled): diff --git a/tests/pedal/test_pedal.py b/tests/pedal/test_pedal.py index 652a90c033..05a0512267 100755 --- a/tests/pedal/test_pedal.py +++ b/tests/pedal/test_pedal.py @@ -1,3 +1,4 @@ +#!/usr/bin/env python3 import os import time import subprocess From 79708f912dc1681c99569b59624de74fa9341a31 Mon Sep 17 00:00:00 2001 From: Robbe Derks Date: Mon, 3 Apr 2023 17:13:02 +0200 Subject: [PATCH 27/59] USB enumeration based on frame numbers (#1324) * use frame numbers to detect host connection * fix weird formatting --------- Co-authored-by: Comma Device --- board/drivers/usb.h | 20 +++++++------------- board/main.c | 1 + board/stm32fx/inc/stm32f205xx.h | 4 +++- board/stm32h7/llusb.h | 4 ++-- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/board/drivers/usb.h b/board/drivers/usb.h index 1ac5b21b3d..469299d74e 100644 --- a/board/drivers/usb.h +++ b/board/drivers/usb.h @@ -24,6 +24,7 @@ typedef union _USB_Setup { USB_Setup_TypeDef; bool usb_enumerated = false; +uint16_t usb_last_frame_num = 0U; void usb_init(void); void usb_cb_ep3_out_complete(void); @@ -471,6 +472,12 @@ char to_hex_char(int a) { return ret; } +void usb_tick(void) { + uint16_t current_frame_num = (USBx_DEVICE->DSTS & USB_OTG_DSTS_FNSOF_Msk) >> USB_OTG_DSTS_FNSOF_Pos; + usb_enumerated = (current_frame_num != usb_last_frame_num); + usb_last_frame_num = current_frame_num; +} + void usb_setup(void) { int resp_len; ControlPacket_t control_req; @@ -673,24 +680,11 @@ void usb_irqhandler(void) { print("connector ID status change\n"); } - if ((gintsts & USB_OTG_GINTSTS_ESUSP) != 0) { - print("ESUSP detected\n"); - } - - if ((gintsts & USB_OTG_GINTSTS_EOPF) != 0) { - usb_enumerated = true; - } - if ((gintsts & USB_OTG_GINTSTS_USBRST) != 0) { print("USB reset\n"); - usb_enumerated = false; usb_reset(); } - if ((gintsts & USB_OTG_GINTSTS_USBSUSP) != 0) { - usb_enumerated = false; - } - if ((gintsts & USB_OTG_GINTSTS_ENUMDNE) != 0) { print("enumeration done"); // Full speed, ENUMSPD diff --git a/board/main.c b/board/main.c index 09954b2c31..ef35d7edcb 100644 --- a/board/main.c +++ b/board/main.c @@ -150,6 +150,7 @@ void tick_handler(void) { // tick drivers at 8Hz fan_tick(); + usb_tick(); // decimated to 1Hz if (loop_counter == 0U) { diff --git a/board/stm32fx/inc/stm32f205xx.h b/board/stm32fx/inc/stm32f205xx.h index 622faead62..368bcf39e8 100644 --- a/board/stm32fx/inc/stm32f205xx.h +++ b/board/stm32fx/inc/stm32f205xx.h @@ -8,7 +8,7 @@ * This file contains : * - Data structures and the address mapping for all peripherals * - Peripherals registers declarations and bits definition - * - Macros to access peripheral’s registers hardware + * - Macros to access peripheral's registers hardware * ****************************************************************************** * @attention @@ -6728,6 +6728,8 @@ USB_OTG_HostChannelTypeDef; #define USB_OTG_DSTS_ENUMSPD_0 0x00000002U /*!GINTMSK = USB_OTG_GINTMSK_USBRST | USB_OTG_GINTMSK_ENUMDNEM | USB_OTG_GINTMSK_OTGINT | USB_OTG_GINTMSK_RXFLVLM | USB_OTG_GINTMSK_GONAKEFFM | USB_OTG_GINTMSK_GINAKEFFM | - USB_OTG_GINTMSK_OEPINT | USB_OTG_GINTMSK_IEPINT | USB_OTG_GINTMSK_USBSUSPM | - USB_OTG_GINTMSK_CIDSCHGM | USB_OTG_GINTMSK_SRQIM | USB_OTG_GINTMSK_MMISM | USB_OTG_GINTMSK_EOPFM; + USB_OTG_GINTMSK_OEPINT | USB_OTG_GINTMSK_IEPINT | + USB_OTG_GINTMSK_CIDSCHGM | USB_OTG_GINTMSK_SRQIM | USB_OTG_GINTMSK_MMISM; // Set USB Turnaround time USBx->GUSBCFG |= ((USBD_FS_TRDT_VALUE << 10) & USB_OTG_GUSBCFG_TRDT); From eef3d026e37e64a1ba42f70b6a9e83b2f58dad55 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Mon, 3 Apr 2023 20:21:10 -0700 Subject: [PATCH 28/59] CI: run HITL tests on dos (#1304) * run on dos * skip voltage on dos * run these * revert that --------- Co-authored-by: Comma Device --- Jenkinsfile | 1 + tests/hitl/2_health.py | 1 + tests/hitl/conftest.py | 24 +++++++++++++++++++----- tests/hitl/helpers.py | 9 +++++---- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 3337ffd0d5..00b327291b 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -83,6 +83,7 @@ pipeline { phone_steps("panda-dos", [ ["build", "scons -j4"], ["flash", "cd tests/ && ./ci_reset_internal_hw.py"], + ["test", "cd tests/hitl && HW_TYPES=6 pytest --durations=0 [2-6]*.py -k 'not test_send_recv'"], ]) } } diff --git a/tests/hitl/2_health.py b/tests/hitl/2_health.py index f63f777cc0..15321b6427 100644 --- a/tests/hitl/2_health.py +++ b/tests/hitl/2_health.py @@ -30,6 +30,7 @@ def test_orientation_detection(p, panda_jungle): assert False seen_orientations.append(detected_harness_orientation) +@pytest.mark.skip_panda_types((Panda.HW_TYPE_DOS, )) def test_voltage(p): for _ in range(10): voltage = p.health()['voltage'] diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py index a70f2ca769..152bb04253 100644 --- a/tests/hitl/conftest.py +++ b/tests/hitl/conftest.py @@ -17,10 +17,11 @@ JUNGLE_SERIAL = os.getenv("PANDAS_JUNGLE") PANDAS_EXCLUDE = os.getenv("PANDAS_EXCLUDE", "").strip().split(" ") PARTIAL_TESTS = os.environ.get("PARTIAL_TESTS", "0") == "1" +HW_TYPES = os.environ.get("HW_TYPES", None) class PandaGroup: - H7 = (Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2) - GEN2 = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO) + H7 + H7 = (Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2, Panda.HW_TYPE_TRES) + GEN2 = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO, Panda.HW_TYPE_DOS) + H7 GPS = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_UNO) GMLAN = (Panda.HW_TYPE_WHITE_PANDA, Panda.HW_TYPE_GREY_PANDA) @@ -31,6 +32,9 @@ class PandaGroup: # * red panda covers GEN2, STM32H7 # * black panda covers STM32F4, GEN2, and GPS PandaGroup.TESTED = (Panda.HW_TYPE_BLACK_PANDA, Panda.HW_TYPE_RED_PANDA) # type: ignore +elif HW_TYPES is not None: + PandaGroup.TESTED = [bytes([int(x), ]) for x in HW_TYPES.strip().split(",")] # type: ignore + # Find all pandas connected _all_pandas = {} @@ -68,7 +72,10 @@ def init_jungle(): def pytest_configure(config): config.addinivalue_line( - "markers", "test_panda_types(name): mark test to run only on specified panda types" + "markers", "test_panda_types(name): whitelist a test for specific panda types" + ) + config.addinivalue_line( + "markers", "skip_panda_types(name): blacklist panda types from a test" ) config.addinivalue_line( "markers", "panda_expect_can_error: mark test to ignore CAN health errors" @@ -84,7 +91,7 @@ def pytest_make_parametrize_id(config, val, argname): @pytest.fixture(name='panda_jungle') -def fixture__panda_jungle(request): +def fixture_panda_jungle(request): init_jungle() return _panda_jungle @@ -95,11 +102,18 @@ def func_fixture_panda(request, module_panda): # Check if test is applicable to this panda mark = request.node.get_closest_marker('test_panda_types') if mark: - assert len(mark.args) > 0, "Missing allowed panda types in mark" + assert len(mark.args) > 0, "Missing panda types argument in mark" test_types = mark.args[0] if _all_pandas[p.get_usb_serial()] not in test_types: pytest.skip(f"Not applicable, {test_types} pandas only") + mark = request.node.get_closest_marker('skip_panda_types') + if mark: + assert len(mark.args) > 0, "Missing panda types argument in mark" + skip_types = mark.args[0] + if _all_pandas[p.get_usb_serial()] in skip_types: + pytest.skip(f"Not applicable to {skip_types}") + # TODO: reset is slow (2+ seconds) p.reset() diff --git a/tests/hitl/helpers.py b/tests/hitl/helpers.py index 2c36b79896..97b0b9607d 100644 --- a/tests/hitl/helpers.py +++ b/tests/hitl/helpers.py @@ -2,16 +2,17 @@ import random -def time_many_sends(p, bus, p_recv=None, msg_count=100, msg_id=None, two_pandas=False): +def time_many_sends(p, bus, p_recv=None, msg_count=100, two_pandas=False): if p_recv is None: p_recv = p - if msg_id is None: - msg_id = random.randint(0x100, 0x200) if p == p_recv and two_pandas: raise ValueError("Cannot have two pandas that are the same panda") + msg_id = random.randint(0x100, 0x200) + to_send = [(msg_id, 0, b"\xaa" * 8, bus)] * msg_count + start_time = time.monotonic() - p.can_send_many([(msg_id, 0, b"\xaa" * 8, bus)] * msg_count) + p.can_send_many(to_send) r = [] r_echo = [] r_len_expected = msg_count if two_pandas else msg_count * 2 From 0400ac28af22e63721c940e160c622b97d0c8b6b Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Mon, 3 Apr 2023 20:48:26 -0700 Subject: [PATCH 29/59] fake siren: fix false positive fault (#1327) --- board/drivers/fake_siren.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/drivers/fake_siren.h b/board/drivers/fake_siren.h index bef37ab124..38c87deb0c 100644 --- a/board/drivers/fake_siren.h +++ b/board/drivers/fake_siren.h @@ -9,7 +9,7 @@ bool fake_siren_enabled = false; void fake_siren_codec_enable(bool enabled) { if (enabled) { - bool success = false; + bool success = true; success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x2B, (1U << 1)); // Left speaker mix from INA1 success &= i2c_set_reg_bits(I2C5, CODEC_I2C_ADDR, 0x2C, (1U << 1)); // Right speaker mix from INA1 success &= i2c_set_reg_mask(I2C5, CODEC_I2C_ADDR, 0x3D, 0x17, 0b11111); // Left speaker volume From e516a5752b56d9b95229042c5cca8c74b6a9feb0 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 4 Apr 2023 18:51:35 -0700 Subject: [PATCH 30/59] watchdog to log fault on heartbeat loop (#1328) Co-authored-by: Comma Device --- board/drivers/simple_watchdog.h | 26 ++++++++++++++++++++++++++ board/faults.h | 3 ++- board/main.c | 5 +++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 board/drivers/simple_watchdog.h diff --git a/board/drivers/simple_watchdog.h b/board/drivers/simple_watchdog.h new file mode 100644 index 0000000000..fe0c856afc --- /dev/null +++ b/board/drivers/simple_watchdog.h @@ -0,0 +1,26 @@ +typedef struct simple_watchdog_state_t { + uint32_t fault; + uint32_t last_ts; + uint32_t threshold; +} simple_watchdog_state_t; + +simple_watchdog_state_t wd_state; + + +void simple_watchdog_kick(void) { + uint32_t ts = microsecond_timer_get(); + + uint32_t et = get_ts_elapsed(ts, wd_state.last_ts); + if (et > wd_state.threshold) { + print("WD timeout 0x"); puth(et); print("\n"); + fault_occurred(wd_state.fault); + } + + wd_state.last_ts = ts; +} + +void simple_watchdog_init(uint32_t fault, uint32_t threshold) { + wd_state.fault = fault; + wd_state.threshold = threshold; + wd_state.last_ts = microsecond_timer_get(); +} diff --git a/board/faults.h b/board/faults.h index e4a102a613..294ac1176f 100644 --- a/board/faults.h +++ b/board/faults.h @@ -2,7 +2,7 @@ #define FAULT_STATUS_TEMPORARY 1U #define FAULT_STATUS_PERMANENT 2U -// Fault types +// Fault types, matches cereal.log.PandaState.FaultType #define FAULT_RELAY_MALFUNCTION (1U << 0) #define FAULT_UNUSED_INTERRUPT_HANDLED (1U << 1) #define FAULT_INTERRUPT_RATE_CAN_1 (1U << 2) @@ -29,6 +29,7 @@ #define FAULT_INTERRUPT_RATE_SPI (1U << 23) #define FAULT_INTERRUPT_RATE_UART_7 (1U << 24) #define FAULT_SIREN_MALFUNCTION (1U << 25) +#define FAULT_HEARTBEAT_LOOP_WATCHDOG (1U << 26) // Permanent faults #define PERMANENT_FAULTS 0U diff --git a/board/main.c b/board/main.c index ef35d7edcb..a7bae4e8aa 100644 --- a/board/main.c +++ b/board/main.c @@ -5,6 +5,7 @@ #include "drivers/usb.h" #include "drivers/gmlan_alt.h" #include "drivers/kline_init.h" +#include "drivers/simple_watchdog.h" #include "early_init.h" #include "provision.h" @@ -151,6 +152,7 @@ void tick_handler(void) { // tick drivers at 8Hz fan_tick(); usb_tick(); + simple_watchdog_kick(); // decimated to 1Hz if (loop_counter == 0U) { @@ -365,6 +367,9 @@ int main(void) { // enable CAN TXs current_board->enable_can_transceivers(true); + // init watchdog for heartbeat loop, trigger after 4 8Hz cycles + simple_watchdog_init(FAULT_HEARTBEAT_LOOP_WATCHDOG, (4U * 1000000U / 8U)); + // 8Hz timer REGISTER_INTERRUPT(TICK_TIMER_IRQ, tick_handler, 10U, FAULT_INTERRUPT_RATE_TICK) tick_timer_init(); From 2e8f27486f1522db6d442fb9e9c261a0a2eee1d2 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 5 Apr 2023 09:15:31 -0700 Subject: [PATCH 31/59] spi: bump up to 50MHz --- python/spi.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/spi.py b/python/spi.py index 0ff15c3bc3..b742f28f8b 100644 --- a/python/spi.py +++ b/python/spi.py @@ -58,7 +58,7 @@ class SpiDevice: """ Provides locked, thread-safe access to a panda's SPI interface. """ - def __init__(self, speed=30000000): + def __init__(self, speed): if not os.path.exists(DEV_PATH): raise PandaSpiUnavailable(f"SPI device not found: {DEV_PATH}") if spidev is None: @@ -87,7 +87,9 @@ class PandaSpiHandle(BaseHandle): A class that mimics a libusb1 handle for panda SPI communications. """ def __init__(self): - self.dev = SpiDevice() + # 50MHz is the max of the 845. older rev comma three + # may not support the full 50MHz + self.dev = SpiDevice(50000000) # helpers def _calc_checksum(self, data: List[int]) -> int: From 6f852b44a9ef1ee119fabf6aa2a9c477ed594a46 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 5 Apr 2023 22:05:14 -0700 Subject: [PATCH 32/59] SPI: log checksum errors in health (#1334) * SPI: log checksum errors in health * actually send it * check in hitl teardown * check that * fix misra --- board/drivers/spi.h | 12 ++++++++++-- board/health.h | 4 ++-- board/main_comms.h | 2 ++ python/__init__.py | 5 +++-- tests/hitl/conftest.py | 3 +++ 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/board/drivers/spi.h b/board/drivers/spi.h index 8723b5a29d..0320212241 100644 --- a/board/drivers/spi.h +++ b/board/drivers/spi.h @@ -31,6 +31,8 @@ uint8_t spi_state = SPI_STATE_HEADER; uint8_t spi_endpoint; uint16_t spi_data_len_mosi; uint16_t spi_data_len_miso; +uint16_t spi_checksum_error_count = 0; + #define SPI_HEADER_SIZE 7U @@ -63,6 +65,7 @@ bool check_checksum(uint8_t *data, uint16_t len) { void spi_handle_rx(void) { uint8_t next_rx_state = SPI_STATE_HEADER; + bool checksum_valid = false; // parse header spi_endpoint = spi_buf_rx[1]; @@ -70,7 +73,8 @@ void spi_handle_rx(void) { spi_data_len_miso = (spi_buf_rx[5] << 8) | spi_buf_rx[4]; if (spi_state == SPI_STATE_HEADER) { - if ((spi_buf_rx[0] == SPI_SYNC_BYTE) && check_checksum(spi_buf_rx, SPI_HEADER_SIZE)) { + checksum_valid = check_checksum(spi_buf_rx, SPI_HEADER_SIZE); + if ((spi_buf_rx[0] == SPI_SYNC_BYTE) && checksum_valid) { // response: ACK and start receiving data portion spi_buf_tx[0] = SPI_HACK; next_rx_state = SPI_STATE_HEADER_ACK; @@ -85,7 +89,8 @@ void spi_handle_rx(void) { // We got everything! Based on the endpoint specified, call the appropriate handler uint16_t response_len = 0U; bool reponse_ack = false; - if (check_checksum(&(spi_buf_rx[SPI_HEADER_SIZE]), spi_data_len_mosi + 1U)) { + checksum_valid = check_checksum(&(spi_buf_rx[SPI_HEADER_SIZE]), spi_data_len_mosi + 1U); + if (checksum_valid) { if (spi_endpoint == 0U) { if (spi_data_len_mosi >= sizeof(ControlPacket_t)) { ControlPacket_t ctrl; @@ -142,6 +147,9 @@ void spi_handle_rx(void) { } spi_state = next_rx_state; + if (!checksum_valid && (spi_checksum_error_count < __UINT16_MAX__)) { + spi_checksum_error_count += 1U; + } } void spi_handle_tx(bool timed_out) { diff --git a/board/health.h b/board/health.h index afe4f7254e..70582cd3fc 100644 --- a/board/health.h +++ b/board/health.h @@ -1,7 +1,6 @@ // When changing these structs, python/__init__.py needs to be kept up to date! -#define HEALTH_PACKET_VERSION 11 - +#define HEALTH_PACKET_VERSION 12 struct __attribute__((packed)) health_t { uint32_t uptime_pkt; uint32_t voltage_pkt; @@ -26,6 +25,7 @@ struct __attribute__((packed)) health_t { float interrupt_load; uint8_t fan_power; uint8_t safety_rx_checks_invalid; + uint16_t spi_checksum_error_count; }; #define CAN_HEALTH_PACKET_VERSION 4 diff --git a/board/main_comms.h b/board/main_comms.h index 4b7b3c5c94..4a011cf75e 100644 --- a/board/main_comms.h +++ b/board/main_comms.h @@ -31,6 +31,8 @@ int get_health_pkt(void *dat) { health->heartbeat_lost_pkt = (uint8_t)(heartbeat_lost); health->safety_rx_checks_invalid = safety_rx_checks_invalid; + health->spi_checksum_error_count = spi_checksum_error_count; + health->fault_status_pkt = fault_status; health->faults_pkt = faults; diff --git a/python/__init__.py b/python/__init__.py index 44e9d80d5a..ded8a202d2 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -182,9 +182,9 @@ class Panda: HW_TYPE_TRES = b'\x09' CAN_PACKET_VERSION = 4 - HEALTH_PACKET_VERSION = 11 + HEALTH_PACKET_VERSION = 12 CAN_HEALTH_PACKET_VERSION = 4 - HEALTH_STRUCT = struct.Struct(" Date: Thu, 6 Apr 2023 20:52:09 -0700 Subject: [PATCH 33/59] add tres to internal devices --- python/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/__init__.py b/python/__init__.py index ded8a202d2..6537893aa4 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -191,7 +191,7 @@ class Panda: F4_DEVICES = (HW_TYPE_WHITE_PANDA, HW_TYPE_GREY_PANDA, HW_TYPE_BLACK_PANDA, HW_TYPE_UNO, HW_TYPE_DOS) H7_DEVICES = (HW_TYPE_RED_PANDA, HW_TYPE_RED_PANDA_V2, HW_TYPE_TRES) - INTERNAL_DEVICES = (HW_TYPE_UNO, HW_TYPE_DOS) + INTERNAL_DEVICES = (HW_TYPE_UNO, HW_TYPE_DOS, HW_TYPE_TRES) HAS_OBD = (HW_TYPE_BLACK_PANDA, HW_TYPE_UNO, HW_TYPE_DOS, HW_TYPE_RED_PANDA, HW_TYPE_RED_PANDA_V2, HW_TYPE_TRES) # first byte is for EPS scaling factor From a1390647fc650d7a52347579d15e9cf44c54bf67 Mon Sep 17 00:00:00 2001 From: royjr Date: Fri, 7 Apr 2023 21:05:00 -0400 Subject: [PATCH 34/59] README: fix error while installing requirements (#1336) fixes error while installing requirements --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 522b010ca5..43ff67ea0e 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Setup your dependencies: ```bash # Ubuntu -sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip +sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev pip install -r requirements.txt # macOS From 2a567674dc2a8925ab270b6de720f2ae0e16241a Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 8 Apr 2023 16:20:38 -0700 Subject: [PATCH 35/59] Tesla: move reading AEB state to RX hook (#1340) --- board/safety/safety_tesla.h | 19 ++++++++++++------- tests/libpanda/safety_helpers.h | 1 + tests/safety/test_tesla.py | 16 ++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/board/safety/safety_tesla.h b/board/safety/safety_tesla.h index d862d8d314..9d797bb517 100644 --- a/board/safety/safety_tesla.h +++ b/board/safety/safety_tesla.h @@ -34,6 +34,7 @@ const CanMsg TESLA_PT_TX_MSGS[] = { #define TESLA_PT_TX_LEN (sizeof(TESLA_PT_TX_MSGS) / sizeof(TESLA_PT_TX_MSGS[0])) AddrCheckStruct tesla_addr_checks[] = { + {.msg = {{0x2b9, 0, 8, .expected_timestep = 40000U}, { 0 }, { 0 }}}, // DAS_control (25Hz) {.msg = {{0x370, 0, 8, .expected_timestep = 40000U}, { 0 }, { 0 }}}, // EPAS_sysStatus (25Hz) {.msg = {{0x108, 0, 8, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // DI_torque1 (100Hz) {.msg = {{0x118, 0, 6, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // DI_torque2 (100Hz) @@ -48,6 +49,7 @@ AddrCheckStruct tesla_pt_addr_checks[] = { {.msg = {{0x106, 0, 8, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // DI_torque1 (100Hz) {.msg = {{0x116, 0, 6, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // DI_torque2 (100Hz) {.msg = {{0x1f8, 0, 8, .expected_timestep = 20000U}, { 0 }, { 0 }}}, // BrakeMessage (50Hz) + {.msg = {{0x2bf, 0, 8, .expected_timestep = 40000U}, { 0 }, { 0 }}}, // DAS_control (25Hz) {.msg = {{0x256, 0, 8, .expected_timestep = 100000U}, { 0 }, { 0 }}}, // DI_state (10Hz) }; #define TESLA_PT_ADDR_CHECK_LEN (sizeof(tesla_pt_addr_checks) / sizeof(tesla_pt_addr_checks[0])) @@ -104,6 +106,14 @@ static int tesla_rx_hook(CANPacket_t *to_push) { } } + if (bus == 2) { + int das_control_addr = (tesla_powertrain ? 0x2bf : 0x2b9); + if (tesla_longitudinal && (addr == das_control_addr)) { + // "AEB_ACTIVE" + tesla_stock_aeb = ((GET_BYTE(to_push, 2) & 0x03U) == 1U); + } + } + if (tesla_powertrain) { // 0x2bf: DAS_control should not be received on bus 0 generic_rx_checks((addr == 0x2bf) && (bus == 0)); @@ -200,13 +210,8 @@ static int tesla_fwd_hook(int bus_num, CANPacket_t *to_fwd) { block_msg = true; } - if (tesla_longitudinal && (addr == das_control_addr)) { - // "AEB_ACTIVE" - tesla_stock_aeb = ((GET_BYTE(to_fwd, 2) & 0x03U) == 1U); - - if (!tesla_stock_aeb) { - block_msg = true; - } + if (tesla_longitudinal && (addr == das_control_addr) && !tesla_stock_aeb) { + block_msg = true; } if(!block_msg) { diff --git a/tests/libpanda/safety_helpers.h b/tests/libpanda/safety_helpers.h index 7933431d2b..d74d8a2621 100644 --- a/tests/libpanda/safety_helpers.h +++ b/tests/libpanda/safety_helpers.h @@ -158,6 +158,7 @@ void init_tests(void){ // car-specific stuff honda_fwd_brake = false; + tesla_stock_aeb = false; } void set_gmlan_digital_output(int to_set){ diff --git a/tests/safety/test_tesla.py b/tests/safety/test_tesla.py index 90290c4e48..8ba5a6b72e 100755 --- a/tests/safety/test_tesla.py +++ b/tests/safety/test_tesla.py @@ -50,7 +50,7 @@ def _pcm_status_msg(self, enable): values = {"DI_cruiseState": 2 if enable else 0} return self.packer.make_can_msg_panda("DI_state", 0, values) - def _long_control_msg(self, set_speed, acc_val=0, jerk_limits=(0, 0), accel_limits=(0, 0), aeb_event=0): + def _long_control_msg(self, set_speed, acc_val=0, jerk_limits=(0, 0), accel_limits=(0, 0), aeb_event=0, bus=0): values = { "DAS_setSpeed": set_speed, "DAS_accState": acc_val, @@ -60,7 +60,7 @@ def _long_control_msg(self, set_speed, acc_val=0, jerk_limits=(0, 0), accel_limi "DAS_accelMin": accel_limits[0], "DAS_accelMax": accel_limits[1], } - return self.packer.make_can_msg_panda("DAS_control", 0, values) + return self.packer.make_can_msg_panda("DAS_control", bus, values) class TestTeslaSteeringSafety(TestTeslaSafety, common.AngleSteeringSafetyTest): @@ -119,19 +119,19 @@ def test_no_aeb(self): def test_stock_aeb_passthrough(self): no_aeb_msg = self._long_control_msg(10, aeb_event=0) - aeb_msg = self._long_control_msg(10, aeb_event=1) + no_aeb_msg_cam = self._long_control_msg(10, aeb_event=0, bus=2) + aeb_msg_cam = self._long_control_msg(10, aeb_event=1, bus=2) # stock system sends no AEB -> no forwarding, and OP is allowed to TX - self.assertEqual(-1, self.safety.safety_fwd_hook(2, no_aeb_msg)) + self.assertEqual(1, self._rx(no_aeb_msg_cam)) + self.assertEqual(-1, self.safety.safety_fwd_hook(2, no_aeb_msg_cam)) self.assertEqual(True, self._tx(no_aeb_msg)) # stock system sends AEB -> forwarding, and OP is not allowed to TX - self.assertEqual(0, self.safety.safety_fwd_hook(2, aeb_msg)) + self.assertEqual(1, self._rx(aeb_msg_cam)) + self.assertEqual(0, self.safety.safety_fwd_hook(2, aeb_msg_cam)) self.assertEqual(False, self._tx(no_aeb_msg)) - # reset global state for next tests - self.assertEqual(-1, self.safety.safety_fwd_hook(2, no_aeb_msg)) - def test_acc_accel_limits(self): for controls_allowed in [True, False]: self.safety.set_controls_allowed(controls_allowed) From 85cc70d4aa3797d0b30e22064b755b7dd472767f Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 8 Apr 2023 16:45:59 -0700 Subject: [PATCH 36/59] safety: only pass addr to fwd hook (#1339) --- board/drivers/bxcan.h | 2 +- board/drivers/fdcan.h | 2 +- board/safety.h | 4 ++-- board/safety/safety_chrysler.h | 3 +-- board/safety/safety_defaults.h | 8 ++++---- board/safety/safety_ford.h | 3 +-- board/safety/safety_gm.h | 3 +-- board/safety/safety_honda.h | 6 ++---- board/safety/safety_hyundai.h | 3 +-- board/safety/safety_hyundai_canfd.h | 3 +-- board/safety/safety_mazda.h | 3 +-- board/safety/safety_nissan.h | 3 +-- board/safety/safety_subaru.h | 3 +-- board/safety/safety_subaru_legacy.h | 3 +-- board/safety/safety_tesla.h | 3 +-- board/safety/safety_toyota.h | 3 +-- board/safety/safety_volkswagen_mqb.h | 3 +-- board/safety/safety_volkswagen_pq.h | 3 +-- board/safety_declarations.h | 2 +- tests/libpanda/libpanda_py.py | 4 ++-- tests/safety/common.py | 5 ++--- tests/safety/test_tesla.py | 4 ++-- 22 files changed, 30 insertions(+), 46 deletions(-) diff --git a/board/drivers/bxcan.h b/board/drivers/bxcan.h index 8c4a3fc46d..25105dc536 100644 --- a/board/drivers/bxcan.h +++ b/board/drivers/bxcan.h @@ -191,7 +191,7 @@ void can_rx(uint8_t can_number) { can_set_checksum(&to_push); // forwarding (panda only) - int bus_fwd_num = safety_fwd_hook(bus_number, &to_push); + int bus_fwd_num = safety_fwd_hook(bus_number, to_push.addr); if (bus_fwd_num != -1) { CANPacket_t to_send; diff --git a/board/drivers/fdcan.h b/board/drivers/fdcan.h index d8793fd66a..7529c57fba 100644 --- a/board/drivers/fdcan.h +++ b/board/drivers/fdcan.h @@ -173,7 +173,7 @@ void can_rx(uint8_t can_number) { can_set_checksum(&to_push); // forwarding (panda only) - int bus_fwd_num = safety_fwd_hook(bus_number, &to_push); + int bus_fwd_num = safety_fwd_hook(bus_number, to_push.addr); if (bus_fwd_num != -1) { CANPacket_t to_send; diff --git a/board/safety.h b/board/safety.h index 45f76a3332..5c5ba6977b 100644 --- a/board/safety.h +++ b/board/safety.h @@ -77,8 +77,8 @@ int safety_tx_lin_hook(int lin_num, uint8_t *data, int len) { return current_hooks->tx_lin(lin_num, data, len); } -int safety_fwd_hook(int bus_num, CANPacket_t *to_fwd) { - return (relay_malfunction ? -1 : current_hooks->fwd(bus_num, to_fwd)); +int safety_fwd_hook(int bus_num, int addr) { + return (relay_malfunction ? -1 : current_hooks->fwd(bus_num, addr)); } bool get_longitudinal_allowed(void) { diff --git a/board/safety/safety_chrysler.h b/board/safety/safety_chrysler.h index 694d034501..0caaa2c649 100644 --- a/board/safety/safety_chrysler.h +++ b/board/safety/safety_chrysler.h @@ -265,9 +265,8 @@ static int chrysler_tx_hook(CANPacket_t *to_send) { return tx; } -static int chrysler_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int chrysler_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); // forward to camera if (bus_num == 0) { diff --git a/board/safety/safety_defaults.h b/board/safety/safety_defaults.h index 41c8862953..90c8a0655d 100644 --- a/board/safety/safety_defaults.h +++ b/board/safety/safety_defaults.h @@ -27,9 +27,9 @@ static int nooutput_tx_lin_hook(int lin_num, uint8_t *data, int len) { return false; } -static int default_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int default_fwd_hook(int bus_num, int addr) { UNUSED(bus_num); - UNUSED(to_fwd); + UNUSED(addr); return -1; } @@ -65,9 +65,9 @@ static int alloutput_tx_lin_hook(int lin_num, uint8_t *data, int len) { return true; } -static int alloutput_fwd_hook(int bus_num, CANPacket_t *to_fwd) { - UNUSED(to_fwd); +static int alloutput_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; + UNUSED(addr); if (alloutput_passthrough) { if (bus_num == 0) { diff --git a/board/safety/safety_ford.h b/board/safety/safety_ford.h index 69bada7c90..a0f218e705 100644 --- a/board/safety/safety_ford.h +++ b/board/safety/safety_ford.h @@ -222,8 +222,7 @@ static int ford_tx_hook(CANPacket_t *to_send) { return tx; } -static int ford_fwd_hook(int bus_num, CANPacket_t *to_fwd) { - int addr = GET_ADDR(to_fwd); +static int ford_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; switch (bus_num) { diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h index 660f050cda..76e55f0bc5 100644 --- a/board/safety/safety_gm.h +++ b/board/safety/safety_gm.h @@ -210,12 +210,11 @@ static int gm_tx_hook(CANPacket_t *to_send) { return tx; } -static int gm_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int gm_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; if (gm_hw == GM_CAM) { - int addr = GET_ADDR(to_fwd); if (bus_num == 0) { // block PSCMStatus; forwarded through openpilot to hide an alert from the camera bool is_pscm_msg = (addr == 388); diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h index e0a6e2828c..c0a73459a1 100644 --- a/board/safety/safety_honda.h +++ b/board/safety/safety_honda.h @@ -412,7 +412,7 @@ static const addr_checks* honda_bosch_init(uint16_t param) { return &honda_rx_checks; } -static int honda_nidec_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int honda_nidec_fwd_hook(int bus_num, int addr) { // fwd from car to camera. also fwd certain msgs from camera to car // 0xE4 is steering on all cars except CRV and RDX, 0x194 for CRV and RDX, // 0x1FA is brake control, 0x30C is acc hud, 0x33D is lkas hud @@ -424,7 +424,6 @@ static int honda_nidec_fwd_hook(int bus_num, CANPacket_t *to_fwd) { if (bus_num == 2) { // block stock lkas messages and stock acc messages (if OP is doing ACC) - int addr = GET_ADDR(to_fwd); bool is_lkas_msg = (addr == 0xE4) || (addr == 0x194) || (addr == 0x33D); bool is_acc_hud_msg = addr == 0x30C; bool is_brake_msg = addr == 0x1FA; @@ -437,14 +436,13 @@ static int honda_nidec_fwd_hook(int bus_num, CANPacket_t *to_fwd) { return bus_fwd; } -static int honda_bosch_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int honda_bosch_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; if (bus_num == 0) { bus_fwd = 2; } if (bus_num == 2) { - int addr = GET_ADDR(to_fwd); int is_lkas_msg = (addr == 0xE4) || (addr == 0xE5) || (addr == 0x33D) || (addr == 0x33DA) || (addr == 0x33DB); int is_acc_msg = ((addr == 0x1C8) || (addr == 0x30C)) && honda_bosch_radarless && honda_bosch_long; bool block_msg = is_lkas_msg || is_acc_msg; diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index beb622cab8..8a18554bc0 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -305,10 +305,9 @@ static int hyundai_tx_hook(CANPacket_t *to_send) { return tx; } -static int hyundai_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int hyundai_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); // forward cam to ccan and viceversa, except lkas cmd if (bus_num == 0) { diff --git a/board/safety/safety_hyundai_canfd.h b/board/safety/safety_hyundai_canfd.h index 0481273444..188a4c6916 100644 --- a/board/safety/safety_hyundai_canfd.h +++ b/board/safety/safety_hyundai_canfd.h @@ -312,9 +312,8 @@ static int hyundai_canfd_tx_hook(CANPacket_t *to_send) { return tx; } -static int hyundai_canfd_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int hyundai_canfd_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); if (bus_num == 0) { bus_fwd = 2; diff --git a/board/safety/safety_mazda.h b/board/safety/safety_mazda.h index 70866fe608..e17f1ff4b3 100644 --- a/board/safety/safety_mazda.h +++ b/board/safety/safety_mazda.h @@ -108,9 +108,8 @@ static int mazda_tx_hook(CANPacket_t *to_send) { return tx; } -static int mazda_fwd_hook(int bus, CANPacket_t *to_fwd) { +static int mazda_fwd_hook(int bus, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); if (bus == MAZDA_MAIN) { bus_fwd = MAZDA_CAM; diff --git a/board/safety/safety_nissan.h b/board/safety/safety_nissan.h index cc8048aa40..3f8532d7f3 100644 --- a/board/safety/safety_nissan.h +++ b/board/safety/safety_nissan.h @@ -136,9 +136,8 @@ static int nissan_tx_hook(CANPacket_t *to_send) { } -static int nissan_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int nissan_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); if (bus_num == 0) { int block_msg = (addr == 0x280); // CANCEL_MSG diff --git a/board/safety/safety_subaru.h b/board/safety/safety_subaru.h index 538c5a7ce1..44b399cd3b 100644 --- a/board/safety/safety_subaru.h +++ b/board/safety/safety_subaru.h @@ -145,7 +145,7 @@ static int subaru_tx_hook(CANPacket_t *to_send) { return tx; } -static int subaru_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int subaru_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; if (bus_num == 0) { @@ -157,7 +157,6 @@ static int subaru_fwd_hook(int bus_num, CANPacket_t *to_fwd) { // 0x122 ES_LKAS // 0x321 ES_DashStatus // 0x322 ES_LKAS_State - int addr = GET_ADDR(to_fwd); bool block_lkas = (addr == 0x122) || (addr == 0x321) || (addr == 0x322); if (!block_lkas) { bus_fwd = 0; // Main CAN diff --git a/board/safety/safety_subaru_legacy.h b/board/safety/safety_subaru_legacy.h index d84d4a6491..4b5b79437c 100644 --- a/board/safety/safety_subaru_legacy.h +++ b/board/safety/safety_subaru_legacy.h @@ -83,7 +83,7 @@ static int subaru_legacy_tx_hook(CANPacket_t *to_send) { return tx; } -static int subaru_legacy_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int subaru_legacy_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; if (bus_num == 0) { @@ -94,7 +94,6 @@ static int subaru_legacy_fwd_hook(int bus_num, CANPacket_t *to_fwd) { // Preglobal platform // 0x161 is ES_CruiseThrottle // 0x164 is ES_LKAS - int addr = GET_ADDR(to_fwd); int block_msg = ((addr == 0x161) || (addr == 0x164)); if (!block_msg) { bus_fwd = 0; // Main CAN diff --git a/board/safety/safety_tesla.h b/board/safety/safety_tesla.h index 9d797bb517..96a8e42275 100644 --- a/board/safety/safety_tesla.h +++ b/board/safety/safety_tesla.h @@ -192,9 +192,8 @@ static int tesla_tx_hook(CANPacket_t *to_send) { return tx; } -static int tesla_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int tesla_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; - int addr = GET_ADDR(to_fwd); if(bus_num == 0) { // Chassis/PT to autopilot diff --git a/board/safety/safety_toyota.h b/board/safety/safety_toyota.h index 6206130844..7aec33a0d9 100644 --- a/board/safety/safety_toyota.h +++ b/board/safety/safety_toyota.h @@ -235,7 +235,7 @@ static const addr_checks* toyota_init(uint16_t param) { return &toyota_rx_checks; } -static int toyota_fwd_hook(int bus_num, CANPacket_t *to_fwd) { +static int toyota_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; @@ -244,7 +244,6 @@ static int toyota_fwd_hook(int bus_num, CANPacket_t *to_fwd) { } if (bus_num == 2) { - int addr = GET_ADDR(to_fwd); // block stock lkas messages and stock acc messages (if OP is doing ACC) // in TSS2, 0x191 is LTA which we need to block to avoid controls collision int is_lkas_msg = ((addr == 0x2E4) || (addr == 0x412) || (addr == 0x191)); diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h index afedc20a18..8399e64491 100644 --- a/board/safety/safety_volkswagen_mqb.h +++ b/board/safety/safety_volkswagen_mqb.h @@ -265,8 +265,7 @@ static int volkswagen_mqb_tx_hook(CANPacket_t *to_send) { return tx; } -static int volkswagen_mqb_fwd_hook(int bus_num, CANPacket_t *to_fwd) { - int addr = GET_ADDR(to_fwd); +static int volkswagen_mqb_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; switch (bus_num) { diff --git a/board/safety/safety_volkswagen_pq.h b/board/safety/safety_volkswagen_pq.h index 2cacb1227f..7a909d4d5e 100644 --- a/board/safety/safety_volkswagen_pq.h +++ b/board/safety/safety_volkswagen_pq.h @@ -227,8 +227,7 @@ static int volkswagen_pq_tx_hook(CANPacket_t *to_send) { return tx; } -static int volkswagen_pq_fwd_hook(int bus_num, CANPacket_t *to_fwd) { - int addr = GET_ADDR(to_fwd); +static int volkswagen_pq_fwd_hook(int bus_num, int addr) { int bus_fwd = -1; switch (bus_num) { diff --git a/board/safety_declarations.h b/board/safety_declarations.h index 49c8db03d4..6dde0d4a3a 100644 --- a/board/safety_declarations.h +++ b/board/safety_declarations.h @@ -153,7 +153,7 @@ typedef const addr_checks* (*safety_hook_init)(uint16_t param); typedef int (*rx_hook)(CANPacket_t *to_push); typedef int (*tx_hook)(CANPacket_t *to_send); typedef int (*tx_lin_hook)(int lin_num, uint8_t *data, int len); -typedef int (*fwd_hook)(int bus_num, CANPacket_t *to_fwd); +typedef int (*fwd_hook)(int bus_num, int addr); typedef struct { safety_hook_init init; diff --git a/tests/libpanda/libpanda_py.py b/tests/libpanda/libpanda_py.py index 420986550b..2beec5ccad 100644 --- a/tests/libpanda/libpanda_py.py +++ b/tests/libpanda/libpanda_py.py @@ -27,7 +27,7 @@ ffi.cdef(""" int safety_rx_hook(CANPacket_t *to_send); int safety_tx_hook(CANPacket_t *to_push); -int safety_fwd_hook(int bus_num, CANPacket_t *to_fwd); +int safety_fwd_hook(int bus_num, int addr); int set_safety_hooks(uint16_t mode, uint16_t param); """) @@ -70,7 +70,7 @@ class Panda(PandaSafety): # safety def safety_rx_hook(self, to_send: CANPacket) -> int: ... def safety_tx_hook(self, to_push: CANPacket) -> int: ... - def safety_fwd_hook(self, bus_num: int, to_fwd: CANPacket) -> int: ... + def safety_fwd_hook(self, bus_num: int, addr: int) -> int: ... def set_safety_hooks(self, mode: int, param: int) -> int: ... diff --git a/tests/safety/common.py b/tests/safety/common.py index ab478b7ff6..604cc38900 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -654,18 +654,17 @@ def test_relay_malfunction(self): for bus in range(0, 3): for addr in self.SCANNED_ADDRS: self.assertEqual(-1, self._tx(make_msg(bus, addr, 8))) - self.assertEqual(-1, self.safety.safety_fwd_hook(bus, make_msg(bus, addr, 8))) + self.assertEqual(-1, self.safety.safety_fwd_hook(bus, addr)) def test_fwd_hook(self): # some safety modes don't forward anything, while others blacklist msgs for bus in range(0, 3): for addr in self.SCANNED_ADDRS: # assume len 8 - msg = make_msg(bus, addr, 8) fwd_bus = self.FWD_BUS_LOOKUP.get(bus, -1) if bus in self.FWD_BLACKLISTED_ADDRS and addr in self.FWD_BLACKLISTED_ADDRS[bus]: fwd_bus = -1 - self.assertEqual(fwd_bus, self.safety.safety_fwd_hook(bus, msg), f"{addr=:#x} from {bus=} to {fwd_bus=}") + self.assertEqual(fwd_bus, self.safety.safety_fwd_hook(bus, addr), f"{addr=:#x} from {bus=} to {fwd_bus=}") def test_spam_can_buses(self): for bus in range(0, 4): diff --git a/tests/safety/test_tesla.py b/tests/safety/test_tesla.py index 8ba5a6b72e..fffbfcf4d2 100755 --- a/tests/safety/test_tesla.py +++ b/tests/safety/test_tesla.py @@ -124,12 +124,12 @@ def test_stock_aeb_passthrough(self): # stock system sends no AEB -> no forwarding, and OP is allowed to TX self.assertEqual(1, self._rx(no_aeb_msg_cam)) - self.assertEqual(-1, self.safety.safety_fwd_hook(2, no_aeb_msg_cam)) + self.assertEqual(-1, self.safety.safety_fwd_hook(2, no_aeb_msg_cam.addr)) self.assertEqual(True, self._tx(no_aeb_msg)) # stock system sends AEB -> forwarding, and OP is not allowed to TX self.assertEqual(1, self._rx(aeb_msg_cam)) - self.assertEqual(0, self.safety.safety_fwd_hook(2, aeb_msg_cam)) + self.assertEqual(0, self.safety.safety_fwd_hook(2, aeb_msg_cam.addr)) self.assertEqual(False, self._tx(no_aeb_msg)) def test_acc_accel_limits(self): From 6d86f546c0214448d4d339f9f85625cf08c494e9 Mon Sep 17 00:00:00 2001 From: royjr Date: Tue, 11 Apr 2023 22:38:02 -0400 Subject: [PATCH 37/59] README: fix git error while trying to build (#1344) Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 43ff67ea0e..714ffb9aea 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Setup your dependencies: ```bash # Ubuntu -sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev +sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev git pip install -r requirements.txt # macOS From 0832d65347619439137900599d92cdd2b8d8d194 Mon Sep 17 00:00:00 2001 From: Igor Biletskyy Date: Wed, 12 Apr 2023 11:07:58 -0700 Subject: [PATCH 38/59] Fix CAN message corruption on H7 under high load (#1342) * fix RX FIFO corruption * Add checksum to CANFD test data * cleaner and MISRA * nah * oops * fix test --- board/drivers/fdcan.h | 8 +++++++- board/stm32h7/llfdcan.h | 12 +++++++----- tests/canfd/test_canfd.py | 10 +++++++--- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/board/drivers/fdcan.h b/board/drivers/fdcan.h index 7529c57fba..a8dc261bcb 100644 --- a/board/drivers/fdcan.h +++ b/board/drivers/fdcan.h @@ -88,6 +88,7 @@ void process_can(uint8_t can_number) { can_health[can_number].total_tx_cnt += 1U; uint32_t TxFIFOSA = FDCAN_START_ADDRESS + (can_number * FDCAN_OFFSET) + (FDCAN_RX_FIFO_0_EL_CNT * FDCAN_RX_FIFO_0_EL_SIZE); + // get the index of the next TX FIFO element (0 to FDCAN_TX_FIFO_EL_CNT - 1) uint8_t tx_index = (CANx->TXFQS >> FDCAN_TXFQS_TFQPI_Pos) & 0x1F; // only send if we have received a packet canfd_fifo *fifo; @@ -145,9 +146,14 @@ void can_rx(uint8_t can_number) { // can is live pending_can_live = 1; - // getting new message index (0 to 63) + // get the index of the next RX FIFO element (0 to FDCAN_RX_FIFO_0_EL_CNT - 1) uint8_t rx_fifo_idx = (uint8_t)((CANx->RXF0S >> FDCAN_RXF0S_F0GI_Pos) & 0x3F); + // Recommended to offset get index by at least +1 if RX FIFO is in overwrite mode and full (datasheet) + if((CANx->RXF0S & FDCAN_RXF0S_F0F) == FDCAN_RXF0S_F0F) { + rx_fifo_idx = ((rx_fifo_idx + 1U) >= FDCAN_RX_FIFO_0_EL_CNT) ? 0U : (rx_fifo_idx + 1U); + } + uint32_t RxFIFO0SA = FDCAN_START_ADDRESS + (can_number * FDCAN_OFFSET); CANPacket_t to_push; canfd_fifo *fifo; diff --git a/board/stm32h7/llfdcan.h b/board/stm32h7/llfdcan.h index b2bb885b43..e5791fc528 100644 --- a/board/stm32h7/llfdcan.h +++ b/board/stm32h7/llfdcan.h @@ -13,12 +13,14 @@ // FDCAN core settings #define FDCAN_MESSAGE_RAM_SIZE 0x2800UL #define FDCAN_START_ADDRESS 0x4000AC00UL -#define FDCAN_OFFSET 3412UL // bytes for each FDCAN module -#define FDCAN_OFFSET_W 853UL // words for each FDCAN module -#define FDCAN_END_ADDRESS 0x4000D3FCUL // Message RAM has a width of 4 Bytes +#define FDCAN_OFFSET 3384UL // bytes for each FDCAN module, equally +#define FDCAN_OFFSET_W 846UL // words for each FDCAN module, equally +#define FDCAN_END_ADDRESS 0x4000D3FCUL // Message RAM has a width of 4 bytes + +// FDCAN_RX_FIFO_0_EL_CNT + FDCAN_TX_FIFO_EL_CNT can't exceed 47 elements (47 * 72 bytes = 3,384 bytes) per FDCAN module // RX FIFO 0 -#define FDCAN_RX_FIFO_0_EL_CNT 24UL +#define FDCAN_RX_FIFO_0_EL_CNT 30UL #define FDCAN_RX_FIFO_0_HEAD_SIZE 8UL // bytes #define FDCAN_RX_FIFO_0_DATA_SIZE 64UL // bytes #define FDCAN_RX_FIFO_0_EL_SIZE (FDCAN_RX_FIFO_0_HEAD_SIZE + FDCAN_RX_FIFO_0_DATA_SIZE) @@ -26,7 +28,7 @@ #define FDCAN_RX_FIFO_0_OFFSET 0UL // TX FIFO -#define FDCAN_TX_FIFO_EL_CNT 16UL +#define FDCAN_TX_FIFO_EL_CNT 17UL #define FDCAN_TX_FIFO_HEAD_SIZE 8UL // bytes #define FDCAN_TX_FIFO_DATA_SIZE 64UL // bytes #define FDCAN_TX_FIFO_EL_SIZE (FDCAN_TX_FIFO_HEAD_SIZE + FDCAN_TX_FIFO_DATA_SIZE) diff --git a/tests/canfd/test_canfd.py b/tests/canfd/test_canfd.py index 5a1511ef45..508e0e3992 100755 --- a/tests/canfd/test_canfd.py +++ b/tests/canfd/test_canfd.py @@ -3,7 +3,7 @@ import time import random from collections import defaultdict -from panda import Panda +from panda import Panda, calculate_checksum from panda_jungle import PandaJungle # pylint: disable=import-error H7_HW_TYPES = [Panda.HW_TYPE_RED_PANDA, Panda.HW_TYPE_RED_PANDA_V2] @@ -55,9 +55,11 @@ def canfd_test(p_send, p_recv): bus = random.randrange(3) for dlc in range(len(DLC_TO_LEN)): address = random.randrange(1, 1<<29) - data = bytes([random.getrandbits(8) for _ in range(DLC_TO_LEN[dlc])]) + data = bytearray(random.getrandbits(8) for _ in range(DLC_TO_LEN[dlc])) + if len(data) >= 2: + data[0] = calculate_checksum(data[1:] + bytes(str(address), encoding="utf-8")) to_send.append([address, 0, data, bus]) - sent_msgs[bus].add((address, data)) + sent_msgs[bus].add((address, bytes(data))) p_send.can_send_many(to_send, timeout=0) @@ -66,6 +68,8 @@ def canfd_test(p_send, p_recv): incoming = p_recv.can_recv() for msg in incoming: address, _, data, bus = msg + if len(data) >= 2: + assert calculate_checksum(data[1:] + bytes(str(address), encoding="utf-8")) == data[0] k = (address, bytes(data)) assert k in sent_msgs[bus], f"message {k} was never sent on bus {bus}" sent_msgs[bus].discard(k) From 9e03d10857e456a4f0119f09de5a2bb76c9504f6 Mon Sep 17 00:00:00 2001 From: royjr Date: Wed, 12 Apr 2023 19:22:19 -0400 Subject: [PATCH 39/59] README: better setup order (#1337) --- README.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 714ffb9aea..90bc836477 100644 --- a/README.md +++ b/README.md @@ -13,25 +13,30 @@ It uses an [STM32F413](http://www.st.com/en/microcontrollers/stm32f413-423.html? ## Usage -Setup your dependencies: - +Setup dependencies: ```bash # Ubuntu sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev git -pip install -r requirements.txt - +``` +```bash # macOS brew tap ArmMbed/homebrew-formulae brew install python dfu-util arm-none-eabi-gcc gcc@12 -pip install -r requirements.txt ``` -### Python - -To install the library: +Clone panda repository: ``` bash git clone https://github.com/commaai/panda.git cd panda +``` + +Install requirements: +```bash +pip install -r requirements.txt +``` + +Install library: +``` bash python setup.py install ``` From 22b8e5df7f6a5241c417775e9fec545d94889dfd Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 12 Apr 2023 16:28:45 -0700 Subject: [PATCH 40/59] add extra fault assert in HITL tests for context --- tests/hitl/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py index fff4b71fc5..669ae99c76 100644 --- a/tests/hitl/conftest.py +++ b/tests/hitl/conftest.py @@ -130,6 +130,7 @@ def func_fixture_panda(request, module_panda): # show up as failed tests instead of "errors" # Check for faults + assert p.health()['faults'] == 0 assert p.health()['fault_status'] == 0 # Check for SPI errors From fddca54fd6f9830ebbfb8efa287f902331b35f88 Mon Sep 17 00:00:00 2001 From: Jason Young <46612682+jyoung8607@users.noreply.github.com> Date: Thu, 13 Apr 2023 20:28:33 -0400 Subject: [PATCH 41/59] VW: Allow inactive accel values at all times (#1247) * allow inactive accel values at all times * cleaner * unnecessary, done by default * better comments * move test to common class * fix * flip * comment * append 0 and INACTIVE_ACCEL to test accels + check acc_07 sends if inactive only * cleanup * fix that * copy testing convention of VW and Honda --------- Co-authored-by: Shane Smiskol --- board/safety.h | 10 +++------- tests/safety/common.py | 9 +++++---- tests/safety/test_volkswagen_mqb.py | 10 ++++++---- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/board/safety.h b/board/safety.h index 5c5ba6977b..80f7181c42 100644 --- a/board/safety.h +++ b/board/safety.h @@ -493,13 +493,9 @@ float interpolate(struct lookup_t xy, float x) { // Safety checks for longitudinal actuation bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limits) { - bool violation = false; - if (!get_longitudinal_allowed()) { - violation |= desired_accel != limits.inactive_accel; - } else { - violation |= max_limit_check(desired_accel, limits.max_accel, limits.min_accel); - } - return violation; + bool accel_valid = get_longitudinal_allowed() && !max_limit_check(desired_accel, limits.max_accel, limits.min_accel); + bool accel_inactive = desired_accel == limits.inactive_accel; + return !(accel_valid || accel_inactive); } bool longitudinal_speed_checks(int desired_speed, const LongitudinalLimits limits) { diff --git a/tests/safety/common.py b/tests/safety/common.py index 604cc38900..5df16d8bde 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -156,16 +156,17 @@ def test_accel_actuation_limits(self, stock_longitudinal=False): (self.MIN_ACCEL, self.MAX_ACCEL, ALTERNATIVE_EXPERIENCE.RAISE_LONGITUDINAL_LIMITS_TO_ISO_MAX)) for min_accel, max_accel, alternative_experience in limits: - for accel in np.arange(min_accel - 1, max_accel + 1, 0.05): + # enforce we don't skip over 0 or inactive accel + for accel in np.concatenate((np.arange(min_accel - 1, max_accel + 1, 0.05), [0, self.INACTIVE_ACCEL])): + accel = round(accel, 2) # floats might not hit exact boundary conditions without rounding for controls_allowed in [True, False]: self.safety.set_controls_allowed(controls_allowed) self.safety.set_alternative_experience(alternative_experience) if stock_longitudinal: should_tx = False - elif controls_allowed: - should_tx = int(min_accel * 1000) <= int(accel * 1000) <= int(max_accel * 1000) else: - should_tx = np.isclose(accel, self.INACTIVE_ACCEL, atol=0.0001) + should_tx = controls_allowed and min_accel <= accel <= max_accel + should_tx = should_tx or accel == self.INACTIVE_ACCEL self.assertEqual(should_tx, self._tx(self._accel_msg(accel))) diff --git a/tests/safety/test_volkswagen_mqb.py b/tests/safety/test_volkswagen_mqb.py index abee4e6fdb..deec801253 100755 --- a/tests/safety/test_volkswagen_mqb.py +++ b/tests/safety/test_volkswagen_mqb.py @@ -208,16 +208,18 @@ def test_main_switch(self): def test_accel_safety_check(self): for controls_allowed in [True, False]: - for accel in np.arange(MIN_ACCEL - 2, MAX_ACCEL + 2, 0.03): + # enforce we don't skip over 0 or inactive accel + for accel in np.concatenate((np.arange(MIN_ACCEL - 2, MAX_ACCEL + 2, 0.03), [0, self.INACTIVE_ACCEL])): accel = round(accel, 2) # floats might not hit exact boundary conditions without rounding - send = MIN_ACCEL <= accel <= MAX_ACCEL if controls_allowed else accel == self.INACTIVE_ACCEL + is_inactive_accel = accel == self.INACTIVE_ACCEL + send = (controls_allowed and MIN_ACCEL <= accel <= MAX_ACCEL) or is_inactive_accel self.safety.set_controls_allowed(controls_allowed) # primary accel request used by ECU self.assertEqual(send, self._tx(self._acc_06_msg(accel)), (controls_allowed, accel)) # additional accel request used by ABS/ESP self.assertEqual(send, self._tx(self._acc_07_msg(accel)), (controls_allowed, accel)) - # ensure the optional secondary accel field remains disabled for now - self.assertFalse(self._tx(self._acc_07_msg(accel, secondary_accel=accel)), (controls_allowed, accel)) + # ensure the optional secondary accel field remains inactive for now + self.assertEqual(is_inactive_accel, self._tx(self._acc_07_msg(accel, secondary_accel=accel)), (controls_allowed, accel)) if __name__ == "__main__": From 0117ff9d131eb22c0737e0758ab9178acec19744 Mon Sep 17 00:00:00 2001 From: Igor Biletskyy Date: Thu, 13 Apr 2023 17:37:48 -0700 Subject: [PATCH 42/59] H7: Rx FIFO lost counter (#1345) * fifo rx lost counter * add comment --- board/drivers/fdcan.h | 4 +--- board/health.h | 10 +++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/board/drivers/fdcan.h b/board/drivers/fdcan.h index a8dc261bcb..4dcc3fdf46 100644 --- a/board/drivers/fdcan.h +++ b/board/drivers/fdcan.h @@ -61,9 +61,6 @@ void update_can_health_pkt(uint8_t can_number, bool error_irq) { if (error_irq) { can_health[can_number].total_error_cnt += 1U; - if ((CANx->IR & (FDCAN_IR_RF0L)) != 0) { - can_health[can_number].total_rx_lost_cnt += 1U; - } if ((CANx->IR & (FDCAN_IR_TEFL)) != 0) { can_health[can_number].total_tx_lost_cnt += 1U; } @@ -152,6 +149,7 @@ void can_rx(uint8_t can_number) { // Recommended to offset get index by at least +1 if RX FIFO is in overwrite mode and full (datasheet) if((CANx->RXF0S & FDCAN_RXF0S_F0F) == FDCAN_RXF0S_F0F) { rx_fifo_idx = ((rx_fifo_idx + 1U) >= FDCAN_RX_FIFO_0_EL_CNT) ? 0U : (rx_fifo_idx + 1U); + can_health[can_number].total_rx_lost_cnt += 1U; // At least one message was lost } uint32_t RxFIFO0SA = FDCAN_START_ADDRESS + (can_number * FDCAN_OFFSET); diff --git a/board/health.h b/board/health.h index 70582cd3fc..7f822401c1 100644 --- a/board/health.h +++ b/board/health.h @@ -38,11 +38,11 @@ typedef struct __attribute__((packed)) { uint8_t last_stored_error; // last LEC positive error code stored uint8_t last_data_error; // DLEC (for CANFD only) uint8_t last_data_stored_error; // last DLEC positive error code stored (for CANFD only) - uint8_t receive_error_cnt; // REC - uint8_t transmit_error_cnt; // TEC - uint32_t total_error_cnt; // How many times any error interrupt were invoked - uint32_t total_tx_lost_cnt; // Tx event FIFO element Lost - uint32_t total_rx_lost_cnt; // Rx FIFO 0 message Lost + uint8_t receive_error_cnt; // Actual state of the receive error counter, values between 0 and 127. FDCAN_ECR.REC + uint8_t transmit_error_cnt; // Actual state of the transmit error counter, values between 0 and 255. FDCAN_ECR.TEC + uint32_t total_error_cnt; // How many times any error interrupt was invoked + uint32_t total_tx_lost_cnt; // Tx event FIFO element lost + uint32_t total_rx_lost_cnt; // Rx FIFO 0 message lost due to FIFO full condition uint32_t total_tx_cnt; uint32_t total_rx_cnt; uint32_t total_fwd_cnt; // Messages forwarded from one bus to another From ada8b49cb5940d306c529156622022be2d7fdaa2 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 14 Apr 2023 01:51:19 -0700 Subject: [PATCH 43/59] Honda Bosch Radarless: allow alt brake (#1341) * radarless can use this * fix addr check * update opendbc ref --- Dockerfile | 2 +- board/safety/safety_honda.h | 5 +++-- tests/safety/test_honda.py | 4 ---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 235476b5c0..00983344e0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -51,7 +51,7 @@ ENV PATH="/root/.pyenv/bin:/root/.pyenv/shims:${PATH}" ENV PANDA_PATH=/tmp/openpilot/panda ENV OPENPILOT_REF="ee0dd36a3c775dbd82493c84f4e7272c1eb3fcbd" -ENV OPENDBC_REF="e8e97fcf00be9a696be009aa37ca13c55b9f632c" +ENV OPENDBC_REF="3c81860270e918d1a08f14a833522fd798aa39ca" COPY requirements.txt /tmp/ RUN pyenv install 3.8.10 && \ diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h index c0a73459a1..26a20bb6d1 100644 --- a/board/safety/safety_honda.h +++ b/board/safety/safety_honda.h @@ -39,7 +39,8 @@ AddrCheckStruct honda_common_addr_checks[] = { {.msg = {{0x1A6, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 40000U}, // SCM_BUTTONS {0x296, 0, 4, .check_checksum = true, .max_counter = 3U, .expected_timestep = 40000U}, { 0 }}}, {.msg = {{0x158, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // ENGINE_DATA - {.msg = {{0x17C, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, // POWERTRAIN_DATA + {.msg = {{0x17C, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, // POWERTRAIN_DATA + {0x1BE, 0, 3, .check_checksum = true, .max_counter = 3U, .expected_timestep = 20000U}, { 0 }}}, // BRAKE_MODULE (for bosch radarless) {.msg = {{0x326, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 100000U}, { 0 }, { 0 }}}, // SCM_FEEDBACK }; #define HONDA_COMMON_ADDR_CHECKS_LEN (sizeof(honda_common_addr_checks) / sizeof(honda_common_addr_checks[0])) @@ -397,7 +398,7 @@ static const addr_checks* honda_bosch_init(uint16_t param) { honda_hw = HONDA_BOSCH; honda_bosch_radarless = GET_FLAG(param, HONDA_PARAM_RADARLESS); // Checking for alternate brake override from safety parameter - honda_alt_brake_msg = GET_FLAG(param, HONDA_PARAM_ALT_BRAKE) && !honda_bosch_radarless; + honda_alt_brake_msg = GET_FLAG(param, HONDA_PARAM_ALT_BRAKE); // radar disabled so allow gas/brakes #ifdef ALLOW_DEBUG diff --git a/tests/safety/test_honda.py b/tests/safety/test_honda.py index f98186703c..8e6bed0f8c 100755 --- a/tests/safety/test_honda.py +++ b/tests/safety/test_honda.py @@ -524,10 +524,6 @@ def setUp(self): self.packer = CANPackerPanda("honda_civic_ex_2022_can_generated") self.safety = libpanda_py.libpanda - # These cars do not have 0x1BE (BRAKE_MODULE) - def test_alt_disengage_on_brake(self): - pass - class TestHondaBoschRadarlessSafety(HondaPcmEnableBase, TestHondaBoschRadarlessSafetyBase): """ From 6ec078db5a1bc5e34caf0e28df202a3f76617d40 Mon Sep 17 00:00:00 2001 From: Jason Young <46612682+jyoung8607@users.noreply.github.com> Date: Sat, 15 Apr 2023 14:34:42 -0400 Subject: [PATCH 44/59] VW MQB: Cleanup HCA control message (#1310) * VW MQB: Cleanup HCA control message * update opendbc ref * revert this before merging * MISRA * revert Dockerfile to comma master * reverted a little too much --- Dockerfile | 2 +- board/safety/safety_volkswagen_mqb.h | 10 +++++----- tests/safety/common.py | 2 +- tests/safety/test_volkswagen_mqb.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 00983344e0..c885e0bbbf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -51,7 +51,7 @@ ENV PATH="/root/.pyenv/bin:/root/.pyenv/shims:${PATH}" ENV PANDA_PATH=/tmp/openpilot/panda ENV OPENPILOT_REF="ee0dd36a3c775dbd82493c84f4e7272c1eb3fcbd" -ENV OPENDBC_REF="3c81860270e918d1a08f14a833522fd798aa39ca" +ENV OPENDBC_REF="342c0320dd271fb585db3cced397c5122078af85" COPY requirements.txt /tmp/ RUN pyenv install 3.8.10 && \ diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h index 8399e64491..9277e8e018 100644 --- a/board/safety/safety_volkswagen_mqb.h +++ b/board/safety/safety_volkswagen_mqb.h @@ -214,12 +214,12 @@ static int volkswagen_mqb_tx_hook(CANPacket_t *to_send) { } // Safety check for HCA_01 Heading Control Assist torque - // Signal: HCA_01.Assist_Torque (absolute torque) - // Signal: HCA_01.Assist_VZ (direction) + // Signal: HCA_01.HCA_01_LM_Offset (absolute torque) + // Signal: HCA_01.HCA_01_LM_OffSign (direction) if (addr == MSG_HCA_01) { - int desired_torque = GET_BYTE(to_send, 2) | ((GET_BYTE(to_send, 3) & 0x3FU) << 8); - int sign = (GET_BYTE(to_send, 3) & 0x80U) >> 7; - if (sign == 1) { + int desired_torque = GET_BYTE(to_send, 2) | ((GET_BYTE(to_send, 3) & 0x1U) << 8); + bool sign = GET_BIT(to_send, 31U); + if (sign) { desired_torque *= -1; } diff --git a/tests/safety/common.py b/tests/safety/common.py index 5df16d8bde..be5e914c0b 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -199,7 +199,7 @@ def _set_prev_torque(self, t): def test_steer_safety_check(self): for enabled in [0, 1]: - for t in range(-self.MAX_TORQUE * 2, self.MAX_TORQUE * 2): + for t in range(int(-self.MAX_TORQUE * 1.5), int(self.MAX_TORQUE * 1.5)): self.safety.set_controls_allowed(enabled) self._set_prev_torque(t) if abs(t) > self.MAX_TORQUE or (not enabled and abs(t) > 0): diff --git a/tests/safety/test_volkswagen_mqb.py b/tests/safety/test_volkswagen_mqb.py index deec801253..0a49d477b0 100755 --- a/tests/safety/test_volkswagen_mqb.py +++ b/tests/safety/test_volkswagen_mqb.py @@ -85,7 +85,7 @@ def _torque_driver_msg(self, torque): # openpilot steering output torque def _torque_cmd_msg(self, torque, steer_req=1): - values = {"Assist_Torque": abs(torque), "Assist_VZ": torque < 0} + values = {"HCA_01_LM_Offset": abs(torque), "HCA_01_LM_OffSign": torque < 0} return self.packer.make_can_msg_panda("HCA_01", 0, values) # Cruise control buttons From 862b03b5c53b33a6143608bedbf53eb4a6b8c6eb Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 15 Apr 2023 17:56:25 -0700 Subject: [PATCH 45/59] CI: add job to update pre-commit hooks (#1346) * CI: add job to update pre-commit hooks * rm push trigger --- .github/workflows/repo.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/workflows/repo.yml diff --git a/.github/workflows/repo.yml b/.github/workflows/repo.yml new file mode 100644 index 0000000000..1e3dcf9c0f --- /dev/null +++ b/.github/workflows/repo.yml @@ -0,0 +1,28 @@ +name: repo + +on: + schedule: + - cron: "0 15 * * 2" + workflow_dispatch: + +jobs: + pre-commit-autoupdate: + name: pre-commit autoupdate + runs-on: ubuntu-20.04 + container: + image: ghcr.io/commaai/panda:latest + steps: + - uses: actions/checkout@v3 + - name: pre-commit autoupdate + run: | + git config --global --add safe.directory '*' + pre-commit autoupdate + - name: Create Pull Request + uses: peter-evans/create-pull-request@5b4a9f6a9e2af26e5f02351490b90d01eb8ec1e5 + with: + token: ${{ secrets.ACTIONS_CREATE_PR_PAT }} + commit-message: Update pre-commit hook versions + title: 'pre-commit: autoupdate hooks' + branch: pre-commit-updates + base: master + delete-branch: true From 9cd01ac26371571432c42b901fe1ac54f393ee17 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sun, 16 Apr 2023 14:43:58 -0700 Subject: [PATCH 46/59] PandaDFU: retry SPI comms (#1348) * retry * set exc --------- Co-authored-by: Comma Device --- python/__init__.py | 7 +++++-- python/spi.py | 16 +++++++++++++--- tests/hitl/0_dfu.py | 27 ++++++++++++++++++++++++++- tests/hitl/1_program.py | 2 +- tests/hitl/conftest.py | 9 ++++++++- 5 files changed, 53 insertions(+), 8 deletions(-) diff --git a/python/__init__.py b/python/__init__.py index 6537893aa4..dd135c4ef0 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -421,7 +421,7 @@ def reconnect(self): except Exception: logging.debug("reconnecting is taking %d seconds...", i + 1) try: - dfu = PandaDFU(PandaDFU.st_serial_to_dfu_serial(self._serial, self._mcu_type)) + dfu = PandaDFU(self.get_dfu_serial()) dfu.recover() except Exception: pass @@ -493,7 +493,7 @@ def flash(self, fn=None, code=None, reconnect=True): self.reconnect() def recover(self, timeout: Optional[int] = None, reset: bool = True) -> bool: - dfu_serial = PandaDFU.st_serial_to_dfu_serial(self._serial, self._mcu_type) + dfu_serial = self.get_dfu_serial() if reset: self.reset(enter_bootstub=True) @@ -677,6 +677,9 @@ def get_usb_serial(self): """ return self._serial + def get_dfu_serial(self): + return PandaDFU.st_serial_to_dfu_serial(self._serial, self._mcu_type) + def get_uid(self): """ Returns the UID from the MCU diff --git a/python/spi.py b/python/spi.py index b742f28f8b..63be5e4493 100644 --- a/python/spi.py +++ b/python/spi.py @@ -149,7 +149,7 @@ def _transfer(self, spi, endpoint: int, data, timeout: int, max_rx_len: int = 10 return dat[:-1] except PandaSpiException as e: exc = e - logging.debug("SPI transfer failed, %d retries left", n, exc_info=True) + logging.debug("SPI transfer failed, %d retries left", MAX_XFER_RETRY_COUNT - n - 1, exc_info=True) raise exc # libusb1 functions @@ -201,7 +201,7 @@ def __init__(self): spi.xfer([self.SYNC, ]) try: self._get_ack(spi) - except PandaSpiNackResponse: + except (PandaSpiNackResponse, PandaSpiMissingAck): # NACK ok here, will only ACK the first time pass @@ -222,7 +222,7 @@ def _get_ack(self, spi, timeout=1.0): elif data != self.ACK: raise PandaSpiMissingAck - def _cmd(self, cmd: int, data: Optional[List[bytes]] = None, read_bytes: int = 0, predata=None) -> bytes: + def _cmd_no_retry(self, cmd: int, data: Optional[List[bytes]] = None, read_bytes: int = 0, predata=None) -> bytes: ret = b"" with self.dev.acquire() as spi: # sync + command @@ -252,6 +252,16 @@ def _cmd(self, cmd: int, data: Optional[List[bytes]] = None, read_bytes: int = 0 return bytes(ret) + def _cmd(self, cmd: int, data: Optional[List[bytes]] = None, read_bytes: int = 0, predata=None) -> bytes: + exc = PandaSpiException() + for n in range(MAX_XFER_RETRY_COUNT): + try: + return self._cmd_no_retry(cmd, data, read_bytes, predata) + except PandaSpiException as e: + exc = e + logging.debug("SPI transfer failed, %d retries left", MAX_XFER_RETRY_COUNT - n - 1, exc_info=True) + raise exc + def _checksum(self, data: bytes) -> bytes: if len(data) == 1: ret = data[0] ^ 0xFF diff --git a/tests/hitl/0_dfu.py b/tests/hitl/0_dfu.py index cff4c02c79..a8afa24a8a 100644 --- a/tests/hitl/0_dfu.py +++ b/tests/hitl/0_dfu.py @@ -1,8 +1,12 @@ +import pytest +import random + from panda import Panda, PandaDFU +from panda.python.spi import SpiDevice def test_dfu(p): app_mcu_type = p.get_mcu_type() - dfu_serial = PandaDFU.st_serial_to_dfu_serial(p.get_usb_serial(), p.get_mcu_type()) + dfu_serial = p.get_dfu_serial() p.reset(enter_bootstub=True) p.reset(enter_bootloader=True) @@ -16,3 +20,24 @@ def test_dfu(p): dfu._handle.clear_status() dfu.reset() p.reconnect() + + +@pytest.mark.test_panda_types((Panda.HW_TYPE_TRES, )) +def test_dfu_with_spam(p): + dfu_serial = p.get_dfu_serial() + + # enter DFU + p.reset(enter_bootstub=True) + p.reset(enter_bootloader=True) + assert Panda.wait_for_dfu(dfu_serial, timeout=20), "failed to enter DFU" + + # send junk + for _ in range(10): + speed = 1000000 * random.randint(1, 5) + d = SpiDevice(speed=speed) + with d.acquire() as spi: + dat = [random.randint(0, 255) for _ in range(random.randint(1, 100))] + spi.xfer(dat) + + # should still show up + assert dfu_serial in PandaDFU.list() diff --git a/tests/hitl/1_program.py b/tests/hitl/1_program.py index 6db3355096..eb0c6a5af3 100644 --- a/tests/hitl/1_program.py +++ b/tests/hitl/1_program.py @@ -33,7 +33,7 @@ def test_a_known_bootstub(p): p.reset(enter_bootstub=True) p.reset(enter_bootloader=True) - dfu_serial = PandaDFU.st_serial_to_dfu_serial(p._serial, p._mcu_type) + dfu_serial = p.get_dfu_serial() assert Panda.wait_for_dfu(dfu_serial, timeout=30) dfu = PandaDFU(dfu_serial) diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py index 669ae99c76..c01538ae76 100644 --- a/tests/hitl/conftest.py +++ b/tests/hitl/conftest.py @@ -3,7 +3,7 @@ import time import pytest -from panda import Panda +from panda import Panda, PandaDFU from panda_jungle import PandaJungle # pylint: disable=import-error from panda.tests.hitl.helpers import clear_can_buffers @@ -61,6 +61,8 @@ def init_all_pandas(): def init_jungle(): + if _panda_jungle is None: + return clear_can_buffers(_panda_jungle) _panda_jungle.set_panda_power(True) _panda_jungle.set_can_loopback(False) @@ -121,6 +123,11 @@ def func_fixture_panda(request, module_panda): yield p # Teardown + + # reconnect + if p.get_dfu_serial() in PandaDFU.list(): + PandaDFU(p.get_dfu_serial()).reset() + p.reconnect() if not p.connected: p.reconnect() if p.bootstub: From 2f7962189329ce5da5d68cb3261982aaa84b0deb Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sun, 16 Apr 2023 15:08:46 -0700 Subject: [PATCH 47/59] improve flash and recover scripts (#1349) * improve flash and recover scripts * update references * cleanup * less spaces --------- Co-authored-by: Comma Device --- board/README.md | 11 ++++------- board/flash.py | 17 +++++++++++++++++ board/flash.sh | 5 ----- board/recover.py | 26 ++++++++++++++++++++++++++ board/recover.sh | 8 -------- 5 files changed, 47 insertions(+), 20 deletions(-) create mode 100755 board/flash.py delete mode 100755 board/flash.sh create mode 100755 board/recover.py delete mode 100755 board/recover.sh diff --git a/board/README.md b/board/README.md index 4434d28afc..ad9091411f 100644 --- a/board/README.md +++ b/board/README.md @@ -4,18 +4,15 @@ Programming **Panda** ``` -./recover.sh # flash bootstub -``` - -``` -./flash.sh # flash application +./recover.py # flash bootstub +./flash.py # flash application ``` Troubleshooting ---- -If your panda will not flash and green LED is on, use `recover.sh`. -If panda is blinking fast with green LED, use `flash.sh`. +If your panda will not flash and green LED is on, use `recover.py`. +If panda is blinking fast with green LED, use `flash.py`. Otherwise if LED is off and panda can't be seen with `lsusb` command, use [panda paw](https://comma.ai/shop/products/panda-paw) to go into DFU mode. diff --git a/board/flash.py b/board/flash.py new file mode 100755 index 0000000000..d1a78e2c51 --- /dev/null +++ b/board/flash.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +import os +import subprocess + +from panda import Panda + +board_path = os.path.dirname(os.path.realpath(__file__)) + +if __name__ == "__main__": + subprocess.check_call(f"scons -C {board_path}/.. -j$(nproc) {board_path}", shell=True) + + serials = Panda.list() + print(f"found {len(serials)} panda(s) - {serials}") + for s in serials: + print("flashing", s) + with Panda(serial=s) as p: + p.flash() diff --git a/board/flash.sh b/board/flash.sh deleted file mode 100755 index cdb8f1be69..0000000000 --- a/board/flash.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env sh -set -e - -scons -u -j$(nproc) -printf %b 'from python import Panda\nfor serial in Panda.list(): Panda(serial).flash()' | PYTHONPATH=.. python3 diff --git a/board/recover.py b/board/recover.py new file mode 100755 index 0000000000..5284a4e295 --- /dev/null +++ b/board/recover.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python3 +import os +import time +import subprocess + +from panda import Panda, PandaDFU + +board_path = os.path.dirname(os.path.realpath(__file__)) + +if __name__ == "__main__": + subprocess.check_call(f"scons -C {board_path}/.. -j$(nproc) {board_path}", shell=True) + + for s in Panda.list(): + print("putting", s, "in DFU mode") + with Panda(serial=s) as p: + p.reset(enter_bootstub=True) + p.reset(enter_bootloader=True) + + # wait for reset pandas to come back up + time.sleep(1) + + dfu_serials = PandaDFU.list() + print(f"found {len(dfu_serials)} panda(s) in DFU - {dfu_serials}") + for s in dfu_serials: + print("flashing", s) + PandaDFU(s).recover() diff --git a/board/recover.sh b/board/recover.sh deleted file mode 100755 index f003cba679..0000000000 --- a/board/recover.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env sh -set -e - -scons -u -j$(nproc) -# Recovers panda from DFU mode only, use flash.sh after power cycling panda -printf %b 'from python import Panda\nfor serial in Panda.list(): Panda(serial).reset(enter_bootstub=True); Panda(serial).reset(enter_bootloader=True)' | PYTHONPATH=.. python3 -sleep 1 -printf %b 'from python import PandaDFU\nfor serial in PandaDFU.list(): PandaDFU(serial).recover()' | PYTHONPATH=.. python3 From 1f8b03666d8f8ed377e9af89cffc5b1bdd70e922 Mon Sep 17 00:00:00 2001 From: Justin Newberry Date: Mon, 17 Apr 2023 21:47:51 -0400 Subject: [PATCH 48/59] Subaru: add ability to send infotainment status message (#1333) * added infotainment status to panda * dont fwd infotainment status * revert placeholder and add test case * Fix test case * gen2 only * fix test case * Revert "fix test case" This reverts commit 8cc7579620100edec7ed89bf3277667a2124cd0a. * Revert "gen2 only" This reverts commit 6a511a997660010ea01658cfdb8e852b4f131855. --------- Co-authored-by: Shane Smiskol --- board/safety/safety_subaru.h | 9 ++++++--- tests/safety/test_subaru.py | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/board/safety/safety_subaru.h b/board/safety/safety_subaru.h index 44b399cd3b..450dabc4da 100644 --- a/board/safety/safety_subaru.h +++ b/board/safety/safety_subaru.h @@ -24,7 +24,8 @@ const CanMsg SUBARU_TX_MSGS[] = { {0x122, 0, 8}, {0x221, 0, 8}, {0x321, 0, 8}, - {0x322, 0, 8} + {0x322, 0, 8}, + {0x323, 0, 8}, }; #define SUBARU_TX_MSGS_LEN (sizeof(SUBARU_TX_MSGS) / sizeof(SUBARU_TX_MSGS[0])) @@ -32,7 +33,8 @@ const CanMsg SUBARU_GEN2_TX_MSGS[] = { {0x122, 0, 8}, {0x221, 1, 8}, {0x321, 0, 8}, - {0x322, 0, 8} + {0x322, 0, 8}, + {0x323, 0, 8} }; #define SUBARU_GEN2_TX_MSGS_LEN (sizeof(SUBARU_GEN2_TX_MSGS) / sizeof(SUBARU_GEN2_TX_MSGS[0])) @@ -157,7 +159,8 @@ static int subaru_fwd_hook(int bus_num, int addr) { // 0x122 ES_LKAS // 0x321 ES_DashStatus // 0x322 ES_LKAS_State - bool block_lkas = (addr == 0x122) || (addr == 0x321) || (addr == 0x322); + // 0x323 INFOTAINMENT_STATUS + bool block_lkas = (addr == 0x122) || (addr == 0x321) || (addr == 0x322) || (addr == 0x323); if (!block_lkas) { bus_fwd = 0; // Main CAN } diff --git a/tests/safety/test_subaru.py b/tests/safety/test_subaru.py index 2352792bbb..fea72bfce9 100755 --- a/tests/safety/test_subaru.py +++ b/tests/safety/test_subaru.py @@ -7,11 +7,11 @@ class TestSubaruSafety(common.PandaSafetyTest, common.DriverTorqueSteeringSafetyTest): - TX_MSGS = [[0x122, 0], [0x221, 0], [0x321, 0], [0x322, 0]] + TX_MSGS = [[0x122, 0], [0x221, 0], [0x321, 0], [0x322, 0], [0x323, 0]] STANDSTILL_THRESHOLD = 0 # kph RELAY_MALFUNCTION_ADDR = 0x122 RELAY_MALFUNCTION_BUS = 0 - FWD_BLACKLISTED_ADDRS = {2: [0x122, 0x321, 0x322]} + FWD_BLACKLISTED_ADDRS = {2: [0x122, 0x321, 0x322, 0x323]} FWD_BUS_LOOKUP = {0: 2, 2: 0} MAX_RATE_UP = 50 @@ -64,7 +64,7 @@ def _pcm_status_msg(self, enable): class TestSubaruGen2Safety(TestSubaruSafety): - TX_MSGS = [[0x122, 0], [0x221, 1], [0x321, 0], [0x322, 0]] + TX_MSGS = [[0x122, 0], [0x221, 1], [0x321, 0], [0x322, 0], [0x323, 0]] ALT_BUS = 1 MAX_RATE_UP = 40 From 237ffedcb31f8eb786aceac9b22e5224aed89b61 Mon Sep 17 00:00:00 2001 From: Robbe Derks Date: Tue, 18 Apr 2023 14:15:06 -0700 Subject: [PATCH 49/59] Dos fan fix (#1335) * hitl fan test * enable cooldown on dos as well * small cleanup * get expected RPM from panda class * fix * overshoot test * fix max RPM getting * fix percentage * revert cooldown fix * add cooldown for dos fan as well * remove feedforward from the fan controller to eliminate overshoot * update clip * cleanup * add that back --------- Co-authored-by: Comma Device Co-authored-by: Adeeb Shihadeh --- board/boards/dos.h | 2 +- board/drivers/fan.h | 11 ++++------- python/__init__.py | 6 ++++++ tests/fan_overshoot_test.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100755 tests/fan_overshoot_test.py diff --git a/board/boards/dos.h b/board/boards/dos.h index b6451bd6a6..2af19d7cb0 100644 --- a/board/boards/dos.h +++ b/board/boards/dos.h @@ -217,7 +217,7 @@ const board board_dos = { .fan_max_rpm = 6500U, .adc_scale = 8862U, .fan_stall_recovery = true, - .fan_enable_cooldown_time = 0U, + .fan_enable_cooldown_time = 3U, .init = dos_init, .enable_can_transceiver = dos_enable_can_transceiver, .enable_can_transceivers = dos_enable_can_transceivers, diff --git a/board/drivers/fan.h b/board/drivers/fan.h index cf4faa09be..bd3b59d112 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -48,7 +48,7 @@ void fan_tick(void){ current_board->set_fan_enabled(false); // clip integral, can't fully reset otherwise we may always be stuck in stall detection - fan_state.error_integral = MIN(50.0f, MAX(0.0f, fan_state.error_integral)); + fan_state.error_integral = MIN(0.0f, fan_state.error_integral); } } else { fan_state.stall_counter = 0U; @@ -57,13 +57,10 @@ void fan_tick(void){ } // Update controller - float feedforward = (fan_state.target_rpm * 100.0f) / current_board->fan_max_rpm; - float error = fan_state.target_rpm - fan_rpm_fast; + fan_state.error_integral += FAN_I * (fan_state.target_rpm - fan_rpm_fast); + fan_state.error_integral = MIN(100.0f, MAX(0.0f, fan_state.error_integral)); - fan_state.error_integral += FAN_I * error; - fan_state.error_integral = MIN(70.0f, MAX(-70.0f, fan_state.error_integral)); - - fan_state.power = MIN(100U, MAX(0U, feedforward + fan_state.error_integral)); + fan_state.power = MIN(100U, MAX(0U, fan_state.error_integral)); pwm_set(TIM3, 3, fan_state.power); // Cooldown counter diff --git a/python/__init__.py b/python/__init__.py index dd135c4ef0..eca478dbb6 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -194,6 +194,12 @@ class Panda: INTERNAL_DEVICES = (HW_TYPE_UNO, HW_TYPE_DOS, HW_TYPE_TRES) HAS_OBD = (HW_TYPE_BLACK_PANDA, HW_TYPE_UNO, HW_TYPE_DOS, HW_TYPE_RED_PANDA, HW_TYPE_RED_PANDA_V2, HW_TYPE_TRES) + MAX_FAN_RPMs = { + HW_TYPE_UNO: 5100, + HW_TYPE_DOS: 6500, + HW_TYPE_TRES: 6600, + } + # first byte is for EPS scaling factor FLAG_TOYOTA_ALT_BRAKE = (1 << 8) FLAG_TOYOTA_STOCK_LONGITUDINAL = (2 << 8) diff --git a/tests/fan_overshoot_test.py b/tests/fan_overshoot_test.py new file mode 100755 index 0000000000..56fd7650c0 --- /dev/null +++ b/tests/fan_overshoot_test.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 + +import time +from panda import Panda + +def get_overshoot_rpm(p, power): + # make sure the fan is stopped completely + p.set_fan_power(0) + while p.get_fan_rpm() > 100: + time.sleep(0.1) + time.sleep(1) + + # set it to 30% power to mimic going onroad + p.set_fan_power(power) + max_rpm = 0 + for _ in range(70): + max_rpm = max(max_rpm, p.get_fan_rpm()) + time.sleep(0.1) + + # tolerate 10% overshoot + expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 + overshoot = (max_rpm / expected_rpm) - 1 + + return overshoot, max_rpm + +if __name__ == "__main__": + p = Panda() + + for power in range(10, 101, 10): + overshoot, max_rpm = get_overshoot_rpm(p, power) + print(f"Fan power {power}% overshoot: {overshoot:.2f} Max RPM: {max_rpm}") From d3904a9f751248cd2e887e89d0db2f6b6239f4b6 Mon Sep 17 00:00:00 2001 From: Robbe Derks Date: Tue, 18 Apr 2023 14:47:22 -0700 Subject: [PATCH 50/59] HITL fan test (#1330) * hitl fan test * enable cooldown on dos as well * small cleanup * get expected RPM from panda class * fix * overshoot test * fix max RPM getting * fix percentage * revert cooldown fix --------- Co-authored-by: Comma Device Co-authored-by: Adeeb Shihadeh --- Jenkinsfile | 2 +- tests/hitl/7_embedded.py | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/hitl/7_embedded.py diff --git a/Jenkinsfile b/Jenkinsfile index 00b327291b..b60cd61bc4 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -83,7 +83,7 @@ pipeline { phone_steps("panda-dos", [ ["build", "scons -j4"], ["flash", "cd tests/ && ./ci_reset_internal_hw.py"], - ["test", "cd tests/hitl && HW_TYPES=6 pytest --durations=0 [2-6]*.py -k 'not test_send_recv'"], + ["test", "cd tests/hitl && HW_TYPES=6 pytest --durations=0 [2-7]*.py -k 'not test_send_recv'"], ]) } } diff --git a/tests/hitl/7_embedded.py b/tests/hitl/7_embedded.py new file mode 100644 index 0000000000..21eeaee129 --- /dev/null +++ b/tests/hitl/7_embedded.py @@ -0,0 +1,42 @@ +import time +import pytest + +from panda import Panda + +@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) +def test_fan_controller(p): + for power in [50, 100]: + p.set_fan_power(power) + time.sleep(10) + + expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 + assert 0.95 * expected_rpm <= p.get_fan_rpm() <= 1.05 * expected_rpm + +@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) +def test_fan_cooldown(p): + # if the fan cooldown doesn't work, we get high frequency noise on the tach line + # while the rotor spins down. this makes sure it never goes beyond the expected max RPM + p.set_fan_power(100) + time.sleep(3) + p.set_fan_power(0) + for _ in range(5): + assert p.get_fan_rpm() <= 7000 + time.sleep(0.5) + +@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) +def test_fan_overshoot(p): + # make sure it's stopped completely + p.set_fan_power(0) + while p.get_fan_rpm() > 100: + time.sleep(0.1) + + # set it to 30% power to mimic going onroad + p.set_fan_power(30) + max_rpm = 0 + for _ in range(50): + max_rpm = max(max_rpm, p.get_fan_rpm()) + time.sleep(0.1) + + # tolerate 10% overshoot + expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * 30 / 100 + assert max_rpm <= 1.1 * expected_rpm, f"Fan overshoot: {(max_rpm / expected_rpm * 100) - 100:.1f}%" From 0b03ef5468aa737247965c3717ccc248465cdc4f Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 18 Apr 2023 16:15:27 -0700 Subject: [PATCH 51/59] HITL tests: skip internal tests on uno (#1350) Co-authored-by: Bruce Wayne --- tests/hitl/{7_embedded.py => 7_internal.py} | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) rename tests/hitl/{7_embedded.py => 7_internal.py} (87%) diff --git a/tests/hitl/7_embedded.py b/tests/hitl/7_internal.py similarity index 87% rename from tests/hitl/7_embedded.py rename to tests/hitl/7_internal.py index 21eeaee129..804b672b9a 100644 --- a/tests/hitl/7_embedded.py +++ b/tests/hitl/7_internal.py @@ -3,7 +3,11 @@ from panda import Panda -@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) +pytestmark = [ + pytest.mark.skip_panda_types(Panda.HW_TYPE_UNO), + pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) +] + def test_fan_controller(p): for power in [50, 100]: p.set_fan_power(power) @@ -12,7 +16,6 @@ def test_fan_controller(p): expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 assert 0.95 * expected_rpm <= p.get_fan_rpm() <= 1.05 * expected_rpm -@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) def test_fan_cooldown(p): # if the fan cooldown doesn't work, we get high frequency noise on the tach line # while the rotor spins down. this makes sure it never goes beyond the expected max RPM @@ -23,7 +26,6 @@ def test_fan_cooldown(p): assert p.get_fan_rpm() <= 7000 time.sleep(0.5) -@pytest.mark.test_panda_types(Panda.INTERNAL_DEVICES) def test_fan_overshoot(p): # make sure it's stopped completely p.set_fan_power(0) From cf307348af6eb5d7bf4dc3f5a11e2d1f3feadfad Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Tue, 18 Apr 2023 22:17:11 -0700 Subject: [PATCH 52/59] CI: run HITL tests on tres (#1326) * run on tres * disable those for now --------- Co-authored-by: Comma Device --- Jenkinsfile | 1 + python/spi.py | 12 ++++++------ tests/hitl/conftest.py | 10 ++++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index b60cd61bc4..adf641aea2 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -94,6 +94,7 @@ pipeline { phone_steps("panda-tres", [ ["build", "scons -j4"], ["flash", "cd tests/ && ./ci_reset_internal_hw.py"], + ["test", "cd tests/hitl && HW_TYPES=9 pytest --durations=0 2*.py [5-7]*.py -k 'not test_fan_controller and not test_fan_overshoot'"], ]) } } diff --git a/python/spi.py b/python/spi.py index 63be5e4493..2e6beb5435 100644 --- a/python/spi.py +++ b/python/spi.py @@ -28,7 +28,7 @@ MIN_ACK_TIMEOUT_MS = 100 MAX_XFER_RETRY_COUNT = 5 -USB_MAX_SIZE = 0x40 +XFER_SIZE = 1000 DEV_PATH = "/dev/spidev0.0" @@ -167,17 +167,17 @@ def controlRead(self, request_type: int, request: int, value: int, index: int, l # TODO: implement these properly def bulkWrite(self, endpoint: int, data: List[int], timeout: int = TIMEOUT) -> int: with self.dev.acquire() as spi: - for x in range(math.ceil(len(data) / USB_MAX_SIZE)): - self._transfer(spi, endpoint, data[USB_MAX_SIZE*x:USB_MAX_SIZE*(x+1)], timeout) + for x in range(math.ceil(len(data) / XFER_SIZE)): + self._transfer(spi, endpoint, data[XFER_SIZE*x:XFER_SIZE*(x+1)], timeout) return len(data) def bulkRead(self, endpoint: int, length: int, timeout: int = TIMEOUT) -> bytes: ret: List[int] = [] with self.dev.acquire() as spi: - for _ in range(math.ceil(length / USB_MAX_SIZE)): - d = self._transfer(spi, endpoint, [], timeout, max_rx_len=USB_MAX_SIZE) + for _ in range(math.ceil(length / XFER_SIZE)): + d = self._transfer(spi, endpoint, [], timeout, max_rx_len=XFER_SIZE) ret += d - if len(d) < USB_MAX_SIZE: + if len(d) < XFER_SIZE: break return bytes(ret) diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py index c01538ae76..0110bc726a 100644 --- a/tests/hitl/conftest.py +++ b/tests/hitl/conftest.py @@ -17,6 +17,7 @@ JUNGLE_SERIAL = os.getenv("PANDAS_JUNGLE") PANDAS_EXCLUDE = os.getenv("PANDAS_EXCLUDE", "").strip().split(" ") PARTIAL_TESTS = os.environ.get("PARTIAL_TESTS", "0") == "1" +NO_JUNGLE = os.environ.get("NO_JUNGLE", "0") == "1" HW_TYPES = os.environ.get("HW_TYPES", None) class PandaGroup: @@ -40,9 +41,10 @@ class PandaGroup: _all_pandas = {} _panda_jungle = None def init_all_pandas(): - global _panda_jungle - _panda_jungle = PandaJungle(JUNGLE_SERIAL) - _panda_jungle.set_panda_power(True) + if not NO_JUNGLE: + global _panda_jungle + _panda_jungle = PandaJungle(JUNGLE_SERIAL) + _panda_jungle.set_panda_power(True) for serial in Panda.list(): if serial not in PANDAS_EXCLUDE and serial != PEDAL_SERIAL: @@ -141,7 +143,7 @@ def func_fixture_panda(request, module_panda): assert p.health()['fault_status'] == 0 # Check for SPI errors - assert p.health()['spi_checksum_error_count'] == 0 + #assert p.health()['spi_checksum_error_count'] == 0 # Check health of each CAN core after test, normal to fail for test_gen2_loopback on OBD bus, so skipping mark = request.node.get_closest_marker('panda_expect_can_error') From e6cad019554a7bd87d5a30ef648bd20fa91af510 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 19 Apr 2023 17:34:31 -0700 Subject: [PATCH 53/59] H7: fix CAN FD IRQ rate (#1332) --- board/stm32h7/stm32h7_config.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/board/stm32h7/stm32h7_config.h b/board/stm32h7/stm32h7_config.h index d1b19fa34d..6c4499cdee 100644 --- a/board/stm32h7/stm32h7_config.h +++ b/board/stm32h7/stm32h7_config.h @@ -14,8 +14,19 @@ #define BOOTLOADER_ADDRESS 0x1FF09804U -// Around (1Mbps / 8 bits/byte / 12 bytes per message) -#define CAN_INTERRUPT_RATE 12000U // FIXME: should raise to 16000 ? +/* +An IRQ is received on message RX/TX (or RX errors), with +separate IRQs for RX and TX. + +0-byte CAN FD frame as the worst case: +- 17 slow bits = SOF + 11 ID + R1 + IDE + EDL + R0 + BRS +- 23 fast bits = ESI + 4 DLC + 0 DATA + 17 CRC + CRC delimeter +- 12 slow bits = ACK + DEL + 7 EOF + 3 IFS +- all currently supported cars are 0.5 Mbps / 2 Mbps + +1 / ((29 bits / 0.5Mbps) + (23 bits / 2Mbps)) = 14388Hz +*/ +#define CAN_INTERRUPT_RATE 16000U #define MAX_LED_FADE 10240U From 0e2eb9c0f5b33a6650ec6e5638af6f33bb5b1d63 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Thu, 20 Apr 2023 13:00:40 -0700 Subject: [PATCH 54/59] fan debugging script (#1354) * fan debugging script * merge those * disable * better print --------- Co-authored-by: Comma Device --- board/config.h | 1 + board/drivers/fan.h | 8 ++++ tests/{ => fan}/fan_test.py | 7 +-- tests/fan/fan_tuning.py | 87 +++++++++++++++++++++++++++++++++++++ tests/fan_overshoot_test.py | 31 ------------- 5 files changed, 98 insertions(+), 36 deletions(-) rename tests/{ => fan}/fan_test.py (63%) create mode 100755 tests/fan/fan_tuning.py delete mode 100755 tests/fan_overshoot_test.py diff --git a/board/config.h b/board/config.h index 1eb7de69b5..6bc3f80df6 100644 --- a/board/config.h +++ b/board/config.h @@ -8,6 +8,7 @@ //#define DEBUG_SPI //#define DEBUG_FAULTS //#define DEBUG_COMMS +//#define DEBUG_FAN #define CAN_INIT_TIMEOUT_MS 500U #define DEEPSLEEP_WAKEUP_DELAY 3U diff --git a/board/drivers/fan.h b/board/drivers/fan.h index bd3b59d112..da6361c483 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -56,6 +56,14 @@ void fan_tick(void){ } } + #ifdef DEBUG_FAN + puth(fan_state.target_rpm); + print(" "); puth(fan_rpm_fast); + print(" "); puth(fan_state.power); + print(" "); puth(fan_state.stall_counter); + print("\n"); + #endif + // Update controller fan_state.error_integral += FAN_I * (fan_state.target_rpm - fan_rpm_fast); fan_state.error_integral = MIN(100.0f, MAX(0.0f, fan_state.error_integral)); diff --git a/tests/fan_test.py b/tests/fan/fan_test.py similarity index 63% rename from tests/fan_test.py rename to tests/fan/fan_test.py index 556576893f..36a11715b3 100755 --- a/tests/fan_test.py +++ b/tests/fan/fan_test.py @@ -1,14 +1,11 @@ #!/usr/bin/env python -import os -import sys import time -sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), "..")) -from panda import Panda # noqa: E402 +from panda import Panda -power = 0 if __name__ == "__main__": p = Panda() + power = 0 while True: p.set_fan_power(power) time.sleep(5) diff --git a/tests/fan/fan_tuning.py b/tests/fan/fan_tuning.py new file mode 100755 index 0000000000..82e3bf1ab4 --- /dev/null +++ b/tests/fan/fan_tuning.py @@ -0,0 +1,87 @@ +#!/usr/bin/env python3 +import json +import time +import threading + +from panda import Panda + +def drain_serial(p): + ret = [] + while True: + d = p.serial_read(0) + if len(d) == 0: + break + ret.append(d) + return ret + + +fan_cmd = 0. + +def logger(event): + # requires a build with DEBUG_FAN + with Panda(claim=False) as p, open('/tmp/fan_log', 'w') as f: + power = None + target_rpm = None + stall_count = None + rpm_fast = None + t = time.monotonic() + + drain_serial(p) + while not event.is_set(): + p.set_fan_power(fan_cmd) + + for l in drain_serial(p)[::-1]: + ns = l.decode('utf8').strip().split(' ') + if len(ns) == 4: + target_rpm, rpm_fast, power, stall_count = [int(n, 16) for n in ns] + break + + dat = { + 't': time.monotonic() - t, + 'cmd_power': fan_cmd, + 'pwm_power': power, + 'target_rpm': target_rpm, + 'rpm_fast': rpm_fast, + 'rpm': p.get_fan_rpm(), + 'stall_counter': stall_count, + } + f.write(json.dumps(dat) + '\n') + time.sleep(1/16.) + + +def get_overshoot_rpm(p, power): + global fan_cmd + + # make sure the fan is stopped completely + fan_cmd = 0. + while p.get_fan_rpm() > 100: + time.sleep(0.1) + time.sleep(3) + + # set it to 30% power to mimic going onroad + fan_cmd = power + max_rpm = 0 + max_power = 0 + for _ in range(70): + max_rpm = max(max_rpm, p.get_fan_rpm()) + max_power = max(max_power, p.health()['fan_power']) + time.sleep(0.1) + + # tolerate 10% overshoot + expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 + overshoot = (max_rpm / expected_rpm) - 1 + + return overshoot, max_rpm, max_power + + +if __name__ == "__main__": + event = threading.Event() + threading.Thread(target=logger, args=(event, )).start() + + try: + p = Panda() + for power in range(10, 101, 10): + overshoot, max_rpm, max_power = get_overshoot_rpm(p, power) + print(f"Fan power {power}%: overshoot {overshoot:.2%}, Max RPM {max_rpm}, Max power {max_power}%") + finally: + event.set() diff --git a/tests/fan_overshoot_test.py b/tests/fan_overshoot_test.py deleted file mode 100755 index 56fd7650c0..0000000000 --- a/tests/fan_overshoot_test.py +++ /dev/null @@ -1,31 +0,0 @@ -#!/usr/bin/env python3 - -import time -from panda import Panda - -def get_overshoot_rpm(p, power): - # make sure the fan is stopped completely - p.set_fan_power(0) - while p.get_fan_rpm() > 100: - time.sleep(0.1) - time.sleep(1) - - # set it to 30% power to mimic going onroad - p.set_fan_power(power) - max_rpm = 0 - for _ in range(70): - max_rpm = max(max_rpm, p.get_fan_rpm()) - time.sleep(0.1) - - # tolerate 10% overshoot - expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 - overshoot = (max_rpm / expected_rpm) - 1 - - return overshoot, max_rpm - -if __name__ == "__main__": - p = Panda() - - for power in range(10, 101, 10): - overshoot, max_rpm = get_overshoot_rpm(p, power) - print(f"Fan power {power}% overshoot: {overshoot:.2f} Max RPM: {max_rpm}") From 3c75a8bc00f503f59a32fead3098afa88139ba07 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Thu, 20 Apr 2023 13:30:56 -0700 Subject: [PATCH 55/59] add fan stall count to health (#1355) * add fan stall count to health * fix misra --------- Co-authored-by: Comma Device --- board/drivers/fan.h | 2 ++ board/health.h | 3 ++- board/main_comms.h | 1 + python/__init__.py | 5 +++-- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/board/drivers/fan.h b/board/drivers/fan.h index da6361c483..d34efbb05f 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -5,6 +5,7 @@ struct fan_state_t { uint8_t power; float error_integral; uint8_t stall_counter; + uint8_t total_stall_count; uint8_t cooldown_counter; } fan_state_t; struct fan_state_t fan_state; @@ -46,6 +47,7 @@ void fan_tick(void){ if (fan_state.stall_counter > FAN_STALL_THRESHOLD) { // Stall detected, power cycling fan controller current_board->set_fan_enabled(false); + fan_state.total_stall_count += 1U; // clip integral, can't fully reset otherwise we may always be stuck in stall detection fan_state.error_integral = MIN(0.0f, fan_state.error_integral); diff --git a/board/health.h b/board/health.h index 7f822401c1..de6f0e34ee 100644 --- a/board/health.h +++ b/board/health.h @@ -1,6 +1,6 @@ // When changing these structs, python/__init__.py needs to be kept up to date! -#define HEALTH_PACKET_VERSION 12 +#define HEALTH_PACKET_VERSION 13 struct __attribute__((packed)) health_t { uint32_t uptime_pkt; uint32_t voltage_pkt; @@ -26,6 +26,7 @@ struct __attribute__((packed)) health_t { uint8_t fan_power; uint8_t safety_rx_checks_invalid; uint16_t spi_checksum_error_count; + uint8_t fan_stall_count; }; #define CAN_HEALTH_PACKET_VERSION 4 diff --git a/board/main_comms.h b/board/main_comms.h index 4a011cf75e..3ccb07cd26 100644 --- a/board/main_comms.h +++ b/board/main_comms.h @@ -39,6 +39,7 @@ int get_health_pkt(void *dat) { health->interrupt_load = interrupt_load; health->fan_power = fan_state.power; + health->fan_stall_count = fan_state.total_stall_count; return sizeof(*health); } diff --git a/python/__init__.py b/python/__init__.py index eca478dbb6..ee09a55c87 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -182,9 +182,9 @@ class Panda: HW_TYPE_TRES = b'\x09' CAN_PACKET_VERSION = 4 - HEALTH_PACKET_VERSION = 12 + HEALTH_PACKET_VERSION = 13 CAN_HEALTH_PACKET_VERSION = 4 - HEALTH_STRUCT = struct.Struct(" Date: Thu, 20 Apr 2023 14:34:11 -0700 Subject: [PATCH 56/59] add CLAMP macro (#1356) * add CLAMP macro * fix --- board/utils.h | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/board/utils.h b/board/utils.h index 4232e0a979..3ab6b30bc8 100644 --- a/board/utils.h +++ b/board/utils.h @@ -1,16 +1,26 @@ -#define MIN(a,b) \ - ({ __typeof__ (a) _a = (a); \ - __typeof__ (b) _b = (b); \ - (_a < _b) ? _a : _b; }) +#define MIN(a, b) ({ \ + __typeof__ (a) _a = (a); \ + __typeof__ (b) _b = (b); \ + (_a < _b) ? _a : _b; \ +}) -#define MAX(a,b) \ - ({ __typeof__ (a) _a = (a); \ - __typeof__ (b) _b = (b); \ - (_a > _b) ? _a : _b; }) +#define MAX(a, b) ({ \ + __typeof__ (a) _a = (a); \ + __typeof__ (b) _b = (b); \ + (_a > _b) ? _a : _b; \ +}) -#define ABS(a) \ - ({ __typeof__ (a) _a = (a); \ - (_a > 0) ? _a : (-_a); }) +#define CLAMP(x, low, high) ({ \ + __typeof__(x) __x = (x); \ + __typeof__(low) __low = (low);\ + __typeof__(high) __high = (high);\ + (__x > __high) ? __high : ((__x < __low) ? __low : __x); \ +}) + +#define ABS(a) ({ \ + __typeof__ (a) _a = (a); \ + (_a > 0) ? _a : (-_a); \ +}) #ifndef NULL #define NULL ((void*)0) From 272b81e97b89dd10d8e9e3b01d1854fb1b742feb Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Thu, 20 Apr 2023 15:44:12 -0700 Subject: [PATCH 57/59] fan: fix stall detection (#1351) * cleanup and fix * dos must stall * clean those up * fix misra * more test * fix import * cleanup --------- Co-authored-by: Comma Device --- board/drivers/fan.h | 71 ++++++++++++++++++++++------------------ board/main.c | 2 +- board/stm32fx/llfan.h | 2 -- board/stm32h7/llfan.h | 2 -- tests/hitl/7_internal.py | 31 +++++++++++++++--- tests/hitl/conftest.py | 6 ++-- 6 files changed, 71 insertions(+), 43 deletions(-) diff --git a/board/drivers/fan.h b/board/drivers/fan.h index d34efbb05f..cec0527bd3 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -10,51 +10,50 @@ struct fan_state_t { } fan_state_t; struct fan_state_t fan_state; -#define FAN_I 0.001f -#define FAN_STALL_THRESHOLD 25U +const float FAN_I = 0.001f; -void fan_set_power(uint8_t percentage){ - fan_state.target_rpm = ((current_board->fan_max_rpm * MIN(100U, MAX(0U, percentage))) / 100U); +const uint8_t FAN_TICK_FREQ = 8U; +const uint8_t FAN_STALL_THRESHOLD = 3U; + + +void fan_set_power(uint8_t percentage) { + fan_state.target_rpm = ((current_board->fan_max_rpm * CLAMP(percentage, 0U, 100U)) / 100U); } -void fan_reset_cooldown(void){ - fan_state.cooldown_counter = current_board->fan_enable_cooldown_time * 8U; +void llfan_init(void); +void fan_init(void) { + fan_state.cooldown_counter = current_board->fan_enable_cooldown_time * FAN_TICK_FREQ; + llfan_init(); } -// Call this at 8Hz -void fan_tick(void){ +// Call this at FAN_TICK_FREQ +void fan_tick(void) { if (current_board->fan_max_rpm > 0U) { - // 4 interrupts per rotation - uint16_t fan_rpm_fast = fan_state.tach_counter * (60U * 8U / 4U); + // Measure fan RPM + uint16_t fan_rpm_fast = fan_state.tach_counter * (60U * FAN_TICK_FREQ / 4U); // 4 interrupts per rotation fan_state.tach_counter = 0U; fan_state.rpm = (fan_rpm_fast + (3U * fan_state.rpm)) / 4U; - // Enable fan if we want it to spin - current_board->set_fan_enabled((fan_state.target_rpm > 0U) || (fan_state.cooldown_counter > 0U)); - if (fan_state.target_rpm > 0U) { - fan_reset_cooldown(); - } - // Stall detection + bool fan_stalled = false; if (current_board->fan_stall_recovery) { - if (fan_state.power > 0U) { + if (fan_state.target_rpm > 0U) { if (fan_rpm_fast == 0U) { fan_state.stall_counter = MIN(fan_state.stall_counter + 1U, 255U); } else { fan_state.stall_counter = 0U; } - if (fan_state.stall_counter > FAN_STALL_THRESHOLD) { - // Stall detected, power cycling fan controller - current_board->set_fan_enabled(false); + if (fan_state.stall_counter > (FAN_STALL_THRESHOLD*FAN_TICK_FREQ)) { + fan_stalled = true; + fan_state.stall_counter = 0U; fan_state.total_stall_count += 1U; - // clip integral, can't fully reset otherwise we may always be stuck in stall detection - fan_state.error_integral = MIN(0.0f, fan_state.error_integral); + // datasheet gives this range as the minimum startup duty + fan_state.error_integral = CLAMP(fan_state.error_integral, 20.0f, 45.0f); } } else { fan_state.stall_counter = 0U; - fan_state.error_integral = 0.0f; } } @@ -66,16 +65,26 @@ void fan_tick(void){ print("\n"); #endif + // Cooldown counter + if (fan_state.target_rpm > 0U) { + fan_state.cooldown_counter = current_board->fan_enable_cooldown_time * FAN_TICK_FREQ; + } else { + if (fan_state.cooldown_counter > 0U) { + fan_state.cooldown_counter--; + } + } + // Update controller - fan_state.error_integral += FAN_I * (fan_state.target_rpm - fan_rpm_fast); - fan_state.error_integral = MIN(100.0f, MAX(0.0f, fan_state.error_integral)); + if (fan_state.target_rpm == 0U) { + fan_state.error_integral = 0.0f; + } else { + float error = fan_state.target_rpm - fan_rpm_fast; + fan_state.error_integral += FAN_I * error; + } + fan_state.power = CLAMP(fan_state.error_integral, 0U, 100U); - fan_state.power = MIN(100U, MAX(0U, fan_state.error_integral)); + // Set PWM and enable line pwm_set(TIM3, 3, fan_state.power); - - // Cooldown counter - if (fan_state.cooldown_counter > 0U) { - fan_state.cooldown_counter--; - } + current_board->set_fan_enabled(!fan_stalled && ((fan_state.target_rpm > 0U) || (fan_state.cooldown_counter > 0U))); } } diff --git a/board/main.c b/board/main.c index a7bae4e8aa..5fc8207fca 100644 --- a/board/main.c +++ b/board/main.c @@ -356,7 +356,7 @@ int main(void) { } if (current_board->fan_max_rpm > 0U) { - llfan_init(); + fan_init(); } microsecond_timer_init(); diff --git a/board/stm32fx/llfan.h b/board/stm32fx/llfan.h index f66fc187ff..9e3f0c654b 100644 --- a/board/stm32fx/llfan.h +++ b/board/stm32fx/llfan.h @@ -8,8 +8,6 @@ void EXTI2_IRQ_Handler(void) { } void llfan_init(void) { - fan_reset_cooldown(); - // 5000RPM * 4 tach edges / 60 seconds REGISTER_INTERRUPT(EXTI2_IRQn, EXTI2_IRQ_Handler, 700U, FAULT_INTERRUPT_RATE_TACH) diff --git a/board/stm32h7/llfan.h b/board/stm32h7/llfan.h index bbcda63e09..dce622503a 100644 --- a/board/stm32h7/llfan.h +++ b/board/stm32h7/llfan.h @@ -8,8 +8,6 @@ void EXTI2_IRQ_Handler(void) { } void llfan_init(void) { - fan_reset_cooldown(); - // 5000RPM * 4 tach edges / 60 seconds REGISTER_INTERRUPT(EXTI2_IRQn, EXTI2_IRQ_Handler, 700U, FAULT_INTERRUPT_RATE_TACH) diff --git a/tests/hitl/7_internal.py b/tests/hitl/7_internal.py index 804b672b9a..08374cbdbf 100644 --- a/tests/hitl/7_internal.py +++ b/tests/hitl/7_internal.py @@ -9,15 +9,35 @@ ] def test_fan_controller(p): - for power in [50, 100]: + start_health = p.health() + + for power in (30, 50, 80, 100): + p.set_fan_power(0) + while p.get_fan_rpm() > 0: + time.sleep(0.1) + + # wait until fan spins up (and recovers if needed), + # then wait a bit more for the RPM to converge p.set_fan_power(power) - time.sleep(10) + for _ in range(20): + time.sleep(1) + if p.get_fan_rpm() > 1000: + break + time.sleep(5) expected_rpm = Panda.MAX_FAN_RPMs[bytes(p.get_type())] * power / 100 - assert 0.95 * expected_rpm <= p.get_fan_rpm() <= 1.05 * expected_rpm + assert 0.9 * expected_rpm <= p.get_fan_rpm() <= 1.1 * expected_rpm + + # Ensure the stall detection is tested on dos + if p.get_type() == Panda.HW_TYPE_DOS: + stalls = p.health()['fan_stall_count'] - start_health['fan_stall_count'] + assert stalls >= 2 + print("stall count", stalls) + else: + assert p.health()['fan_stall_count'] == 0 def test_fan_cooldown(p): - # if the fan cooldown doesn't work, we get high frequency noise on the tach line + # if the fan cooldown doesn't work, we get high frequency noise on the tach line # while the rotor spins down. this makes sure it never goes beyond the expected max RPM p.set_fan_power(100) time.sleep(3) @@ -26,10 +46,11 @@ def test_fan_cooldown(p): assert p.get_fan_rpm() <= 7000 time.sleep(0.5) +@pytest.mark.skip(reason="fan controller overshoots on fans that need stall recovery") def test_fan_overshoot(p): # make sure it's stopped completely p.set_fan_power(0) - while p.get_fan_rpm() > 100: + while p.get_fan_rpm() > 0: time.sleep(0.1) # set it to 30% power to mimic going onroad diff --git a/tests/hitl/conftest.py b/tests/hitl/conftest.py index 0110bc726a..05fa21ffdb 100644 --- a/tests/hitl/conftest.py +++ b/tests/hitl/conftest.py @@ -4,9 +4,12 @@ import pytest from panda import Panda, PandaDFU -from panda_jungle import PandaJungle # pylint: disable=import-error from panda.tests.hitl.helpers import clear_can_buffers +NO_JUNGLE = os.environ.get("NO_JUNGLE", "0") == "1" +if not NO_JUNGLE: + from panda_jungle import PandaJungle # pylint: disable=import-error + SPEED_NORMAL = 500 SPEED_GMLAN = 33.3 @@ -17,7 +20,6 @@ JUNGLE_SERIAL = os.getenv("PANDAS_JUNGLE") PANDAS_EXCLUDE = os.getenv("PANDAS_EXCLUDE", "").strip().split(" ") PARTIAL_TESTS = os.environ.get("PARTIAL_TESTS", "0") == "1" -NO_JUNGLE = os.environ.get("NO_JUNGLE", "0") == "1" HW_TYPES = os.environ.get("HW_TYPES", None) class PandaGroup: From 08ed853bf9d3dccc0263f7bb0d0d6d3f7872f015 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Fri, 21 Apr 2023 13:47:13 -0700 Subject: [PATCH 58/59] log total stall count in fan tuning script --- tests/fan/fan_tuning.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/fan/fan_tuning.py b/tests/fan/fan_tuning.py index 82e3bf1ab4..e9158d609d 100755 --- a/tests/fan/fan_tuning.py +++ b/tests/fan/fan_tuning.py @@ -44,10 +44,11 @@ def logger(event): 'rpm_fast': rpm_fast, 'rpm': p.get_fan_rpm(), 'stall_counter': stall_count, + 'total_stall_count': p.health()['fan_stall_count'], } f.write(json.dumps(dat) + '\n') time.sleep(1/16.) - + p.set_fan_power(0) def get_overshoot_rpm(p, power): global fan_cmd From b342c2d724125b3d12b88a53c2f87ba01c8043d0 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Fri, 21 Apr 2023 15:41:26 -0700 Subject: [PATCH 59/59] fan: add stall threshold backoff (#1357) * fan: add stall threshold backoff * Update board/config.h * cleanup --------- Co-authored-by: Comma Device --- board/drivers/fan.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/board/drivers/fan.h b/board/drivers/fan.h index cec0527bd3..15296079ae 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -5,6 +5,7 @@ struct fan_state_t { uint8_t power; float error_integral; uint8_t stall_counter; + uint8_t stall_threshold; uint8_t total_stall_count; uint8_t cooldown_counter; } fan_state_t; @@ -13,7 +14,8 @@ struct fan_state_t fan_state; const float FAN_I = 0.001f; const uint8_t FAN_TICK_FREQ = 8U; -const uint8_t FAN_STALL_THRESHOLD = 3U; +const uint8_t FAN_STALL_THRESHOLD_MIN = 3U; +const uint8_t FAN_STALL_THRESHOLD_MAX = 8U; void fan_set_power(uint8_t percentage) { @@ -22,6 +24,7 @@ void fan_set_power(uint8_t percentage) { void llfan_init(void); void fan_init(void) { + fan_state.stall_threshold = FAN_STALL_THRESHOLD_MIN; fan_state.cooldown_counter = current_board->fan_enable_cooldown_time * FAN_TICK_FREQ; llfan_init(); } @@ -44,9 +47,10 @@ void fan_tick(void) { fan_state.stall_counter = 0U; } - if (fan_state.stall_counter > (FAN_STALL_THRESHOLD*FAN_TICK_FREQ)) { + if (fan_state.stall_counter > (fan_state.stall_threshold*FAN_TICK_FREQ)) { fan_stalled = true; fan_state.stall_counter = 0U; + fan_state.stall_threshold = CLAMP(fan_state.stall_threshold + 2U, FAN_STALL_THRESHOLD_MIN, FAN_STALL_THRESHOLD_MAX); fan_state.total_stall_count += 1U; // datasheet gives this range as the minimum startup duty @@ -54,6 +58,7 @@ void fan_tick(void) { } } else { fan_state.stall_counter = 0U; + fan_state.stall_threshold = FAN_STALL_THRESHOLD_MIN; } }