From 5f318faf93d7a9acb31fa694633c3881e9e5283f Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Fri, 22 Sep 2023 17:10:36 -0700 Subject: [PATCH 1/6] model replay: add cameraOdometry (#30007) * model replay: add back cameraOdometry coverage * update refs * try again * fix cmp log --- selfdrive/test/process_replay/model_replay.py | 28 ++++--------------- .../process_replay/model_replay_ref_commit | 2 +- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/selfdrive/test/process_replay/model_replay.py b/selfdrive/test/process_replay/model_replay.py index f576e07a40..0cdaf861e8 100755 --- a/selfdrive/test/process_replay/model_replay.py +++ b/selfdrive/test/process_replay/model_replay.py @@ -7,7 +7,6 @@ from typing import Any import cereal.messaging as messaging from openpilot.common.params import Params -from openpilot.common.spinner import Spinner from openpilot.system.hardware import PC from openpilot.selfdrive.manager.process_config import managed_processes from openpilot.selfdrive.test.openpilotci import BASE_URL, get_url @@ -105,14 +104,6 @@ def nav_model_replay(lr): def model_replay(lr, frs): - if not PC: - spinner = Spinner() - spinner.update("starting model replay") - else: - spinner = None - - log_msgs = [] - # modeld is using frame pairs modeld_logs = trim_logs_to_max_frames(lr, MAX_FRAMES, {"roadCameraState", "wideRoadCameraState"}, {"roadEncodeIdx", "wideRoadEncodeIdx"}) dmodeld_logs = trim_logs_to_max_frames(lr, MAX_FRAMES, {"driverCameraState"}, {"driverEncodeIdx"}) @@ -128,18 +119,9 @@ def model_replay(lr, frs): modeld = get_process_config("modeld") dmonitoringmodeld = get_process_config("dmonitoringmodeld") - try: - if spinner: - spinner.update("running model replay") - modeld_msgs = replay_process(modeld, modeld_logs, frs) - dmonitoringmodeld_msgs = replay_process(dmonitoringmodeld, dmodeld_logs, frs) - log_msgs.extend([m for m in modeld_msgs if m.which() == "modelV2"]) - log_msgs.extend([m for m in dmonitoringmodeld_msgs if m.which() == "driverStateV2"]) - finally: - if spinner: - spinner.close() - - return log_msgs + modeld_msgs = replay_process(modeld, modeld_logs, frs) + dmonitoringmodeld_msgs = replay_process(dmonitoringmodeld, dmodeld_logs, frs) + return modeld_msgs + dmonitoringmodeld_msgs if __name__ == "__main__": @@ -196,8 +178,8 @@ if __name__ == "__main__": cmp_log = [] # logs are ordered based on type: modelV2, driverStateV2, nav messages (navThumbnail, mapRenderState, navModel) - model_start_index = next(i for i, m in enumerate(all_logs) if m.which() == "modelV2") - cmp_log += all_logs[model_start_index:model_start_index + MAX_FRAMES] + model_start_index = next(i for i, m in enumerate(all_logs) if m.which() in ("modelV2", "cameraOdometry")) + cmp_log += all_logs[model_start_index:model_start_index + MAX_FRAMES*2] dmon_start_index = next(i for i, m in enumerate(all_logs) if m.which() == "driverStateV2") cmp_log += all_logs[dmon_start_index:dmon_start_index + MAX_FRAMES] if not NO_NAV: diff --git a/selfdrive/test/process_replay/model_replay_ref_commit b/selfdrive/test/process_replay/model_replay_ref_commit index 8ae7d62a04..b2b326d7a7 100644 --- a/selfdrive/test/process_replay/model_replay_ref_commit +++ b/selfdrive/test/process_replay/model_replay_ref_commit @@ -1 +1 @@ -2af877ca0ce6996a4c1cf54cd11abf5a1f4e0576 +3bb23e270a3a7219cfeedadd8d085c32fc571c0d From f223b66b7ba3f0b5f9cfb86b6350e8bb1d53d0f3 Mon Sep 17 00:00:00 2001 From: Justin Newberry Date: Fri, 22 Sep 2023 19:59:38 -0700 Subject: [PATCH 2/6] CI: remove warning messages (#30013) remove ci warnings --- .github/workflows/selfdrive_tests.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/selfdrive_tests.yaml b/.github/workflows/selfdrive_tests.yaml index 92b84f0af9..ee7a8af833 100644 --- a/.github/workflows/selfdrive_tests.yaml +++ b/.github/workflows/selfdrive_tests.yaml @@ -198,7 +198,7 @@ jobs: $DOCKER_LOGIN - uses: ./.github/workflows/setup-with-retry with: - git-lfs: false + git_lfs: false docker_hub_pat: ${{ secrets.DOCKER_HUB_PAT }} - name: Build and push CL Docker image if: matrix.arch == 'x86_64' @@ -309,7 +309,7 @@ jobs: id: print-diff if: always() run: cat selfdrive/test/process_replay/diff.txt - - uses: actions/upload-artifact@v2 + - uses: actions/upload-artifact@v3 if: always() continue-on-error: true with: From a0c8df3d730eec50901235181a201cee91143e12 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 22 Sep 2023 21:38:07 -0700 Subject: [PATCH 3/6] ui: update calibration limits text (#30014) * update translations * update text * round down --- selfdrive/ui/qt/offroad/settings.cc | 2 +- selfdrive/ui/translations/main_ar.ts | 4 ++-- selfdrive/ui/translations/main_de.ts | 4 ++-- selfdrive/ui/translations/main_fr.ts | 4 ++-- selfdrive/ui/translations/main_ja.ts | 4 ++-- selfdrive/ui/translations/main_ko.ts | 4 ++-- selfdrive/ui/translations/main_pt-BR.ts | 4 ++-- selfdrive/ui/translations/main_th.ts | 4 ++-- selfdrive/ui/translations/main_tr.ts | 4 ++-- selfdrive/ui/translations/main_zh-CHS.ts | 4 ++-- selfdrive/ui/translations/main_zh-CHT.ts | 4 ++-- 11 files changed, 21 insertions(+), 21 deletions(-) diff --git a/selfdrive/ui/qt/offroad/settings.cc b/selfdrive/ui/qt/offroad/settings.cc index f74e671d29..669de5eae9 100644 --- a/selfdrive/ui/qt/offroad/settings.cc +++ b/selfdrive/ui/qt/offroad/settings.cc @@ -283,7 +283,7 @@ DevicePanel::DevicePanel(SettingsWindow *parent) : ListWidget(parent) { void DevicePanel::updateCalibDescription() { QString desc = tr("openpilot requires the device to be mounted within 4° left or right and " - "within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required."); + "within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required."); std::string calib_bytes = params.get("CalibrationParams"); if (!calib_bytes.empty()) { try { diff --git a/selfdrive/ui/translations/main_ar.ts b/selfdrive/ui/translations/main_ar.ts index dd9acffb03..a5da450221 100644 --- a/selfdrive/ui/translations/main_ar.ts +++ b/selfdrive/ui/translations/main_ar.ts @@ -226,8 +226,8 @@ أطفاء - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - يتطلب openpilot أن يتم تركيب الجهاز في حدود 4 درجات يسارًا أو يمينًا و 5 درجات لأعلى أو 8 درجات لأسفل. يقوم برنامج openpilot بالمعايرة بشكل مستمر ، ونادراً ما تكون إعادة الضبط مطلوبة. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + يتطلب openpilot أن يتم تركيب الجهاز في حدود 4 درجات يسارًا أو يمينًا و 5 درجات لأعلى أو 9 درجات لأسفل. يقوم برنامج openpilot بالمعايرة بشكل مستمر ، ونادراً ما تكون إعادة الضبط مطلوبة. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_de.ts b/selfdrive/ui/translations/main_de.ts index d1820fd423..c53134c5b3 100644 --- a/selfdrive/ui/translations/main_de.ts +++ b/selfdrive/ui/translations/main_de.ts @@ -226,8 +226,8 @@ Ausschalten - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - Damit Openpilot funktioniert, darf die Installationsposition nicht mehr als 4° nach rechts/links, 5° nach oben und 8° nach unten abweichen. Openpilot kalibriert sich durchgehend, ein Zurücksetzen ist selten notwendig. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + Damit Openpilot funktioniert, darf die Installationsposition nicht mehr als 4° nach rechts/links, 5° nach oben und 9° nach unten abweichen. Openpilot kalibriert sich durchgehend, ein Zurücksetzen ist selten notwendig. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_fr.ts b/selfdrive/ui/translations/main_fr.ts index 79c4ae5596..95afab46a7 100644 --- a/selfdrive/ui/translations/main_fr.ts +++ b/selfdrive/ui/translations/main_fr.ts @@ -234,8 +234,8 @@ Éteindre - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot nécessite que l'appareil soit monté à 4° à gauche ou à droite et à 5° vers le haut ou 8° vers le bas. openpilot se calibre en continu, la réinitialisation est rarement nécessaire. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot nécessite que l'appareil soit monté à 4° à gauche ou à droite et à 5° vers le haut ou 9° vers le bas. openpilot se calibre en continu, la réinitialisation est rarement nécessaire. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_ja.ts b/selfdrive/ui/translations/main_ja.ts index 16595f8ebf..8abd794c60 100644 --- a/selfdrive/ui/translations/main_ja.ts +++ b/selfdrive/ui/translations/main_ja.ts @@ -226,8 +226,8 @@ 電源を切る - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilotの本体は、左右4°以内、上5°、下8°以内の角度で取付ける必要があります。継続してキャリブレーションを続けているので、手動でリセットを行う必要はほぼありません。 + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilotの本体は、左右4°以内、上5°、下9°以内の角度で取付ける必要があります。継続してキャリブレーションを続けているので、手動でリセットを行う必要はほぼありません。 Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_ko.ts b/selfdrive/ui/translations/main_ko.ts index cbd8e668ac..11a9ec9d09 100644 --- a/selfdrive/ui/translations/main_ko.ts +++ b/selfdrive/ui/translations/main_ko.ts @@ -226,8 +226,8 @@ 전원 종료 - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot 장치는 좌우측 4° 이내, 위쪽 5° 아래쪽 8° 이내로 장착되어야 합니다. openpilot은 지속적으로 보정되며 재설정은 거의 필요하지 않습니다. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot 장치는 좌우측 4° 이내, 위쪽 5° 아래쪽 9° 이내로 장착되어야 합니다. openpilot은 지속적으로 보정되며 재설정은 거의 필요하지 않습니다. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_pt-BR.ts b/selfdrive/ui/translations/main_pt-BR.ts index a55d31034e..3f429c2acf 100644 --- a/selfdrive/ui/translations/main_pt-BR.ts +++ b/selfdrive/ui/translations/main_pt-BR.ts @@ -226,8 +226,8 @@ Desligar - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - O openpilot requer que o dispositivo seja montado dentro de 4° esquerda ou direita e dentro de 5° para cima ou 8° para baixo. O openpilot está continuamente calibrando, resetar raramente é necessário. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + O openpilot requer que o dispositivo seja montado dentro de 4° esquerda ou direita e dentro de 5° para cima ou 9° para baixo. O openpilot está continuamente calibrando, resetar raramente é necessário. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_th.ts b/selfdrive/ui/translations/main_th.ts index abc6210956..5b6ecea49d 100644 --- a/selfdrive/ui/translations/main_th.ts +++ b/selfdrive/ui/translations/main_th.ts @@ -226,8 +226,8 @@ ปิดเครื่อง - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot กำหนดให้ติดตั้งอุปกรณ์ โดยสามารถเอียงด้านซ้ายหรือขวาไม่เกิน 4° และเอียงขึ้นด้านบนไม่เกิน 5° หรือเอียงลงด้านล่างไม่เกิน 8° openpilot ทำการคาลิเบรทอย่างต่อเนื่อง แทบจะไม่จำเป็นต้องทำการรีเซ็ตการคาลิเบรท + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot กำหนดให้ติดตั้งอุปกรณ์ โดยสามารถเอียงด้านซ้ายหรือขวาไม่เกิน 4° และเอียงขึ้นด้านบนไม่เกิน 5° หรือเอียงลงด้านล่างไม่เกิน 9° openpilot ทำการคาลิเบรทอย่างต่อเนื่อง แทบจะไม่จำเป็นต้องทำการรีเซ็ตการคาลิเบรท Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_tr.ts b/selfdrive/ui/translations/main_tr.ts index febded8f59..97e1282c68 100644 --- a/selfdrive/ui/translations/main_tr.ts +++ b/selfdrive/ui/translations/main_tr.ts @@ -226,8 +226,8 @@ Sistemi kapat - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot, cihazın 4° sola veya 5° yukarı yada 8° aşağı bakıcak şekilde monte edilmesi gerekmektedir. openpilot sürekli kendisini kalibre edilmektedir ve nadiren sıfırlama gerebilir. + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot, cihazın 4° sola veya 5° yukarı yada 9° aşağı bakıcak şekilde monte edilmesi gerekmektedir. openpilot sürekli kendisini kalibre edilmektedir ve nadiren sıfırlama gerebilir. Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_zh-CHS.ts b/selfdrive/ui/translations/main_zh-CHS.ts index 040dae0b30..9253d922f5 100644 --- a/selfdrive/ui/translations/main_zh-CHS.ts +++ b/selfdrive/ui/translations/main_zh-CHS.ts @@ -226,8 +226,8 @@ 关机 - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot要求设备安装的偏航角在左4°和右4°之间，俯仰角在上5°和下8°之间。一般来说，openpilot会持续更新校准，很少需要重置。 + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot要求设备安装的偏航角在左4°和右4°之间，俯仰角在上5°和下9°之间。一般来说，openpilot会持续更新校准，很少需要重置。 Your device is pointed %1° %2 and %3° %4. diff --git a/selfdrive/ui/translations/main_zh-CHT.ts b/selfdrive/ui/translations/main_zh-CHT.ts index 0ffef3bb7b..3a2040bc3b 100644 --- a/selfdrive/ui/translations/main_zh-CHT.ts +++ b/selfdrive/ui/translations/main_zh-CHT.ts @@ -226,8 +226,8 @@ 關機 - openpilot requires the device to be mounted within 4° left or right and within 5° up or 8° down. openpilot is continuously calibrating, resetting is rarely required. - openpilot 需要將設備固定在左右偏差 4° 以內，朝上偏差 5° 以內或朝下偏差 8° 以內。鏡頭在後台會持續自動校準，很少有需要重設的情況。 + openpilot requires the device to be mounted within 4° left or right and within 5° up or 9° down. openpilot is continuously calibrating, resetting is rarely required. + openpilot 需要將設備固定在左右偏差 4° 以內，朝上偏差 5° 以內或朝下偏差 9° 以內。鏡頭在後台會持續自動校準，很少有需要重設的情況。 Your device is pointed %1° %2 and %3° %4. From 2ebf0b186e76e0027098fb0e39980760f38723f3 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Fri, 22 Sep 2023 21:52:29 -0700 Subject: [PATCH 4/6] Hyundai: use EV/hybrid car lists for ELECT gear signal (#30009) * remove duplicate hybrid/ev list * test --- selfdrive/car/hyundai/carstate.py | 12 ++++++------ selfdrive/car/hyundai/tests/test_hyundai.py | 8 ++++++-- selfdrive/car/hyundai/values.py | 6 +----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/selfdrive/car/hyundai/carstate.py b/selfdrive/car/hyundai/carstate.py index 8090bbcd8f..bd3ef828f8 100644 --- a/selfdrive/car/hyundai/carstate.py +++ b/selfdrive/car/hyundai/carstate.py @@ -132,12 +132,12 @@ class CarState(CarStateBase): # Gear Selection via Cluster - For those Kia/Hyundai which are not fully discovered, we can use the Cluster Indicator for Gear Selection, # as this seems to be standard over all cars, but is not the preferred method. - if self.CP.carFingerprint in CAN_GEARS["use_cluster_gears"]: + if self.CP.carFingerprint in (HYBRID_CAR | EV_CAR): + gear = cp.vl["ELECT_GEAR"]["Elect_Gear_Shifter"] + elif self.CP.carFingerprint in CAN_GEARS["use_cluster_gears"]: gear = cp.vl["CLU15"]["CF_Clu_Gear"] elif self.CP.carFingerprint in CAN_GEARS["use_tcu_gears"]: gear = cp.vl["TCU12"]["CUR_GR"] - elif self.CP.carFingerprint in CAN_GEARS["use_elect_gears"]: - gear = cp.vl["ELECT_GEAR"]["Elect_Gear_Shifter"] else: gear = cp.vl["LVR12"]["CF_Lvr_Gear"] @@ -285,12 +285,12 @@ class CarState(CarStateBase): ("EMS16", 100), ] - if CP.carFingerprint in CAN_GEARS["use_cluster_gears"]: + if CP.carFingerprint in (HYBRID_CAR | EV_CAR): + messages.append(("ELECT_GEAR", 20)) + elif CP.carFingerprint in CAN_GEARS["use_cluster_gears"]: pass elif CP.carFingerprint in CAN_GEARS["use_tcu_gears"]: messages.append(("TCU12", 100)) - elif CP.carFingerprint in CAN_GEARS["use_elect_gears"]: - messages.append(("ELECT_GEAR", 20)) else: messages.append(("LVR12", 100)) diff --git a/selfdrive/car/hyundai/tests/test_hyundai.py b/selfdrive/car/hyundai/tests/test_hyundai.py index eb7deb2679..eba548bfbd 100755 --- a/selfdrive/car/hyundai/tests/test_hyundai.py +++ b/selfdrive/car/hyundai/tests/test_hyundai.py @@ -5,7 +5,7 @@ import unittest from cereal import car from openpilot.selfdrive.car.fw_versions import build_fw_dict from openpilot.selfdrive.car.hyundai.values import CAMERA_SCC_CAR, CANFD_CAR, CAN_GEARS, CAR, CHECKSUM, DATE_FW_ECUS, \ - EV_CAR, FW_QUERY_CONFIG, FW_VERSIONS, LEGACY_SAFETY_MODE_CAR, \ + HYBRID_CAR, EV_CAR, FW_QUERY_CONFIG, FW_VERSIONS, LEGACY_SAFETY_MODE_CAR, \ UNSUPPORTED_LONGITUDINAL_CAR, PLATFORM_CODE_ECUS, get_platform_codes Ecu = car.CarParams.Ecu @@ -37,7 +37,11 @@ NO_DATES_PLATFORMS = { class TestHyundaiFingerprint(unittest.TestCase): - def test_canfd_not_in_can_features(self): + def test_can_features(self): + # Test no EV/HEV in any gear lists (should all use ELECT_GEAR) + self.assertEqual(set.union(*CAN_GEARS.values()) & (HYBRID_CAR | EV_CAR), set()) + + # Test CAN FD car not in CAN feature lists can_specific_feature_list = set.union(*CAN_GEARS.values(), *CHECKSUM.values(), LEGACY_SAFETY_MODE_CAR, UNSUPPORTED_LONGITUDINAL_CAR, CAMERA_SCC_CAR) for car_model in CANFD_CAR: self.assertNotIn(car_model, can_specific_feature_list, "CAN FD car unexpectedly found in a CAN feature list") diff --git a/selfdrive/car/hyundai/values.py b/selfdrive/car/hyundai/values.py index 432e63b4f5..2831b3bc4e 100644 --- a/selfdrive/car/hyundai/values.py +++ b/selfdrive/car/hyundai/values.py @@ -2014,13 +2014,9 @@ CHECKSUM = { } CAN_GEARS = { - # which message has the gear + # which message has the gear. hybrid and EV use ELECT_GEAR "use_cluster_gears": {CAR.ELANTRA, CAR.KONA}, "use_tcu_gears": {CAR.KIA_OPTIMA_G4, CAR.KIA_OPTIMA_G4_FL, CAR.SONATA_LF, CAR.VELOSTER, CAR.TUCSON}, - "use_elect_gears": {CAR.KIA_NIRO_EV, CAR.KIA_NIRO_PHEV, CAR.KIA_NIRO_HEV_2021, CAR.KIA_OPTIMA_H, CAR.KIA_OPTIMA_H_G4_FL, CAR.IONIQ_EV_LTD, - CAR.KONA_EV, CAR.IONIQ, CAR.IONIQ_EV_2020, CAR.IONIQ_PHEV, CAR.ELANTRA_HEV_2021, CAR.SONATA_HYBRID, - CAR.KONA_HEV, CAR.IONIQ_HEV_2022, CAR.SANTA_FE_HEV_2022, CAR.SANTA_FE_PHEV_2022, CAR.IONIQ_PHEV_2019, - CAR.KONA_EV_2022, CAR.KIA_K5_HEV_2020, CAR.AZERA_HEV_6TH_GEN}, } CANFD_CAR = {CAR.KIA_EV6, CAR.IONIQ_5, CAR.IONIQ_6, CAR.TUCSON_4TH_GEN, CAR.TUCSON_HYBRID_4TH_GEN, CAR.KIA_SPORTAGE_HYBRID_5TH_GEN, From c43325fa17c270f9b5eef6c01edaf82cb360df29 Mon Sep 17 00:00:00 2001 From: coffee-cake-isaac <48419673+coffee-cake-isaac@users.noreply.github.com> Date: Sat, 23 Sep 2023 00:53:31 -0400 Subject: [PATCH 5/6] joystick mode: add curvature control (#30010) * Added curvature controls to joystick mode * same line --------- Co-authored-by: Shane Smiskol --- selfdrive/controls/controlsd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/selfdrive/controls/controlsd.py b/selfdrive/controls/controlsd.py index 1dc9dd39f7..770c39d3f2 100755 --- a/selfdrive/controls/controlsd.py +++ b/selfdrive/controls/controlsd.py @@ -629,8 +629,8 @@ class Controls: if CC.latActive: steer = clip(self.sm['testJoystick'].axes[1], -1, 1) - # max angle is 45 for angle-based cars - actuators.steer, actuators.steeringAngleDeg = steer, steer * 45. + # max angle is 45 for angle-based cars, max curvature is 0.02 + actuators.steer, actuators.steeringAngleDeg, actuators.curvature = steer, steer * 45., steer * -0.02 lac_log.active = self.active lac_log.steeringAngleDeg = CS.steeringAngleDeg From c3d0bf7b5e09de43484613e98a02f2606642bbd0 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Sat, 23 Sep 2023 02:13:58 -0700 Subject: [PATCH 6/6] Toyota: improve platform code understanding (#30015) * return a dict where minor version is not in keys * limit valid chunks to 3 (max seen) * First short version character is always 3 (we were using wrong platform code) * docs updates * not here * fixes for printing new platform code format * ecu notes * notes * platform code tests * no tuple * can visualize the whole thing now * make it clear there's no major versions make it clear there's no major versions * static analysis * two minor versions * fix * not using dsu * comment * comment * comment * forgot this one --- .../car/toyota/tests/print_platform_codes.py | 21 ++++- selfdrive/car/toyota/tests/test_toyota.py | 78 +++++++++++++++++-- selfdrive/car/toyota/values.py | 44 ++++++----- 3 files changed, 113 insertions(+), 30 deletions(-) diff --git a/selfdrive/car/toyota/tests/print_platform_codes.py b/selfdrive/car/toyota/tests/print_platform_codes.py index 94badc5cde..636a013242 100755 --- a/selfdrive/car/toyota/tests/print_platform_codes.py +++ b/selfdrive/car/toyota/tests/print_platform_codes.py @@ -1,4 +1,5 @@ #!/usr/bin/env python3 +from collections import defaultdict from cereal import car from openpilot.selfdrive.car.toyota.values import FW_VERSIONS, PLATFORM_CODE_ECUS, get_platform_codes @@ -6,6 +7,8 @@ Ecu = car.CarParams.Ecu ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()} if __name__ == "__main__": + parts_for_ecu: dict = defaultdict(set) + cars_for_code: dict = defaultdict(lambda: defaultdict(set)) for car_model, ecus in FW_VERSIONS.items(): print() print(car_model) @@ -14,8 +17,18 @@ if __name__ == "__main__": continue platform_codes = get_platform_codes(ecus[ecu]) - codes = {code for code, _ in platform_codes} - dates = {date for _, date in platform_codes if date is not None} + parts_for_ecu[ecu] |= {code.split(b'-')[0] for code in platform_codes if code.count(b'-') > 1} + for code in platform_codes: + cars_for_code[ecu][b'-'.join(code.split(b'-')[:2])] |= {car_model} print(f' (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}):') - print(f' Codes: {codes}') - print(f' Versions: {dates}') + print(f' Codes: {platform_codes}') + + print('\nECU parts:') + for ecu, parts in parts_for_ecu.items(): + print(f' (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}): {parts}') + + print('\nCar models vs. platform codes (no major versions):') + for ecu, codes in cars_for_code.items(): + print(f' (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}):') + for code, cars in codes.items(): + print(f' {code!r}: {sorted(cars)}') diff --git a/selfdrive/car/toyota/tests/test_toyota.py b/selfdrive/car/toyota/tests/test_toyota.py index 0c503ebbf0..5241a95a99 100755 --- a/selfdrive/car/toyota/tests/test_toyota.py +++ b/selfdrive/car/toyota/tests/test_toyota.py @@ -4,7 +4,7 @@ import unittest from cereal import car from openpilot.selfdrive.car.toyota.values import CAR, DBC, TSS2_CAR, ANGLE_CONTROL_CAR, RADAR_ACC_CAR, FW_VERSIONS, \ - get_platform_codes + PLATFORM_CODE_ECUS, get_platform_codes Ecu = car.CarParams.Ecu ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()} @@ -43,6 +43,8 @@ class TestToyotaInterfaces(unittest.TestCase): class TestToyotaFingerprint(unittest.TestCase): + # Tests for part numbers, platform codes, and sub-versions which Toyota will use to fuzzy + # fingerprint in the absence of full FW matches: @settings(max_examples=100) @given(data=st.data()) def test_platform_codes_fuzzy_fw(self, data): @@ -50,13 +52,73 @@ class TestToyotaFingerprint(unittest.TestCase): fws = data.draw(fw_strategy) get_platform_codes(fws) - def test_fw_pattern(self): - """Asserts all ECUs can be parsed""" - for ecus in FW_VERSIONS.values(): - for fws in ecus.values(): - for fw in fws: - ret = get_platform_codes([fw]) - self.assertTrue(len(ret)) + def test_platform_code_ecus_available(self): + # Asserts ECU keys essential for fuzzy fingerprinting are available on all platforms + for car_model, ecus in FW_VERSIONS.items(): + with self.subTest(car_model=car_model): + for platform_code_ecu in PLATFORM_CODE_ECUS: + if platform_code_ecu == Ecu.eps and car_model in (CAR.PRIUS_V, CAR.LEXUS_CTH,): + continue + if platform_code_ecu == Ecu.abs and car_model in (CAR.ALPHARD_TSS2,): + continue + self.assertIn(platform_code_ecu, [e[0] for e in ecus]) + + def test_fw_format(self): + # Asserts: + # - every supported ECU FW version returns one platform code + # - every supported ECU FW version has a part number + # - expected parsing of ECU sub-versions + + for car_model, ecus in FW_VERSIONS.items(): + with self.subTest(car_model=car_model): + for ecu, fws in ecus.items(): + if ecu[0] not in PLATFORM_CODE_ECUS: + continue + + codes = dict() + for fw in fws: + result = get_platform_codes([fw]) + # Check only one platform code and sub-version + self.assertEqual(1, len(result), f"Unable to parse FW: {fw}") + self.assertEqual(1, len(list(result.values())[0]), f"Unable to parse FW: {fw}") + codes |= result + + # Toyota places the ECU part number in their FW versions, assert all parsable + # Note that there is only one unique part number per ECU across the fleet, so this + # is not important for identification, just a sanity check. + self.assertTrue(all(code.count(b"-") > 1 for code in codes), + f"FW does not have part number: {fw} {codes}") + + def test_platform_codes_spot_check(self): + # Asserts basic platform code parsing behavior for a few cases + results = get_platform_codes([ + b"F152607140\x00\x00\x00\x00\x00\x00", + b"F152607171\x00\x00\x00\x00\x00\x00", + b"F152607110\x00\x00\x00\x00\x00\x00", + b"F152607180\x00\x00\x00\x00\x00\x00", + ]) + self.assertEqual(results, {b"F1526-07-1": {b"10", b"40", b"71", b"80"}}) + + results = get_platform_codes([ + b"\x028646F4104100\x00\x00\x00\x008646G5301200\x00\x00\x00\x00", + b"\x028646F4104100\x00\x00\x00\x008646G3304000\x00\x00\x00\x00", + ]) + self.assertEqual(results, {b"8646F-41-04": {b"100"}}) + + # Short version has no part number + results = get_platform_codes([ + b"\x0235870000\x00\x00\x00\x00\x00\x00\x00\x00A0202000\x00\x00\x00\x00\x00\x00\x00\x00", + b"\x0235883000\x00\x00\x00\x00\x00\x00\x00\x00A0202000\x00\x00\x00\x00\x00\x00\x00\x00", + ]) + self.assertEqual(results, {b"58-70": {b"000"}, b"58-83": {b"000"}}) + + results = get_platform_codes([ + b"F152607110\x00\x00\x00\x00\x00\x00", + b"F152607140\x00\x00\x00\x00\x00\x00", + b"\x028646F4104100\x00\x00\x00\x008646G5301200\x00\x00\x00\x00", + b"\x0235879000\x00\x00\x00\x00\x00\x00\x00\x00A4701000\x00\x00\x00\x00\x00\x00\x00\x00", + ]) + self.assertEqual(results, {b"F1526-07-1": {b"10", b"40"}, b"8646F-41-04": {b"100"}, b"58-79": {b"000"}}) if __name__ == "__main__": diff --git a/selfdrive/car/toyota/values.py b/selfdrive/car/toyota/values.py index bb9672fb0e..cf7f329dd0 100644 --- a/selfdrive/car/toyota/values.py +++ b/selfdrive/car/toyota/values.py @@ -2,7 +2,7 @@ import re from collections import defaultdict from dataclasses import dataclass, field from enum import Enum, IntFlag -from typing import Dict, List, Set, Tuple, Union +from typing import Dict, List, Set, Union from cereal import car from openpilot.common.conversions import Conversions as CV @@ -236,8 +236,9 @@ STATIC_DSU_MSGS = [ ] -def get_platform_codes(fw_versions: List[bytes]) -> Set[Tuple[bytes, bytes]]: - codes = set() # (Optional[part]-platform-major_version, minor_version) +def get_platform_codes(fw_versions: List[bytes]) -> Dict[bytes, Set[bytes]]: + # Returns sub versions in a dict so comparisons can be made within part-platform-major_version combos + codes = defaultdict(set) # Optional[part]-platform-major_version: set of sub_version for fw in fw_versions: # FW versions returned from UDS queries can return multiple fields/chunks of data (different ECU calibrations, different data?) # and are prefixed with a byte that describes how many chunks of data there are. @@ -262,42 +263,49 @@ def get_platform_codes(fw_versions: List[bytes]) -> Set[Tuple[bytes, bytes]]: fw_match = SHORT_FW_PATTERN.search(first_chunk) if fw_match is not None: platform, major_version, sub_version = fw_match.groups() - # codes.add((platform + b'-' + major_version, sub_version)) - codes.add((b'-'.join((platform, major_version)), sub_version)) + codes[b'-'.join((platform, major_version))].add(sub_version) elif len(first_chunk) == 10: fw_match = MEDIUM_FW_PATTERN.search(first_chunk) if fw_match is not None: part, platform, major_version, sub_version = fw_match.groups() - codes.add((b'-'.join((part, platform, major_version)), sub_version)) + codes[b'-'.join((part, platform, major_version))].add(sub_version) elif len(first_chunk) == 12: fw_match = LONG_FW_PATTERN.search(first_chunk) if fw_match is not None: part, platform, major_version, sub_version = fw_match.groups() - codes.add((b'-'.join((part, platform, major_version)), sub_version)) + codes[b'-'.join((part, platform, major_version))].add(sub_version) - return codes + return dict(codes) # Regex patterns for parsing more general platform-specific identifiers from FW versions. # - Part number: Toyota part number (usually last character needs to be ignored to find a match). -# - Platform: usually multiple codes per an openpilot platform, however this has the less variability and +# Each ECU address has just one part number. +# - Platform: usually multiple codes per an openpilot platform, however this is the least variable and # is usually shared across ECUs and model years signifying this describes something about the specific platform. -# - Major version: second least variable part of the FW version. Seen splitting cars by model year such as RAV4 2022/2023 and Prius. +# This describes more generational changes (TSS-P vs TSS2), or manufacture region. +# - Major version: second least variable part of the FW version. Seen splitting cars by model year/API such as +# RAV4 2022/2023 and Avalon. Used to differentiate cars where API has changed slightly, but is not a generational change. # It is important to note that these aren't always consecutive, for example: -# Prius TSS-P has these major versions over 16 FW: 2, 3, 4, 6, 8 while Prius TSS2 has: 5 -# - Sub version: exclusive to major version, but shared with other cars. Should only be used for further filtering, -# more exploration is needed. -SHORT_FW_PATTERN = re.compile(b'(?P[A-Z0-9]{2})(?P[A-Z0-9]{2})(?P[A-Z0-9]{4})') +# Avalon 2016-18's fwdCamera has these major versions: 01, 03 while 2019 has: 02 +# - Sub version: exclusive to major version, but shared with other cars. Should only be used for further filtering. +# Seen bumped in TSB FW updates, and describes other minor differences. +SHORT_FW_PATTERN = re.compile(b'[A-Z0-9](?P[A-Z0-9]{2})(?P[A-Z0-9]{2})(?P[A-Z0-9]{3})') MEDIUM_FW_PATTERN = re.compile(b'(?P[A-Z0-9]{5})(?P[A-Z0-9]{2})(?P[A-Z0-9]{1})(?P[A-Z0-9]{2})') LONG_FW_PATTERN = re.compile(b'(?P[A-Z0-9]{5})(?P[A-Z0-9]{2})(?P[A-Z0-9]{2})(?P[A-Z0-9]{3})') -FW_LEN_CODE = re.compile(b'^[\x01-\x05]') # 5 chunks max. highest seen is 3 chunks, 16 bytes each +FW_LEN_CODE = re.compile(b'^[\x01-\x03]') # highest seen is 3 chunks, 16 bytes each FW_CHUNK_LEN = 16 -# List of ECUs expected to have platform codes -# TODO: use hybrid ECU, splits many similar ICE and hybrid variants -PLATFORM_CODE_ECUS = [Ecu.abs, Ecu.engine, Ecu.eps, Ecu.dsu, Ecu.fwdCamera, Ecu.fwdRadar] +# List of ECUs that are most unique across openpilot platforms +# TODO: use hybrid ECU, splits similar ICE and hybrid variants +# - fwdCamera: describes actual features related to ADAS. For example, on the Avalon it describes +# when TSS-P became standard, whether the car supports stop and go, and whether it's TSS2. +# On the RAV4, it describes the move to the radar doing ACC, and the use of LTA for lane keeping. +# - abs: differentiates hybrid/ICE on most cars (Corolla TSS2 is an exception) +# - eps: describes lateral API changes for the EPS, such as using LTA for lane keeping and rejecting LKA messages +PLATFORM_CODE_ECUS = [Ecu.fwdCamera, Ecu.abs, Ecu.eps] # Some ECUs that use KWP2000 have their FW versions on non-standard data identifiers.