From aef0732509c332e0c5d50318673758837a0a16d4 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 12 Jul 2021 13:29:07 -0700 Subject: [PATCH] fix vvt indication (#2926) * wrap logic * rename gauges * names and ranges * binary log * we actually don't need that warning * values auto wrap * values auto wrap * bye warnings * comparison --- firmware/controllers/trigger/trigger_central.cpp | 15 ++++++++++----- unit_tests/tests/trigger/test_cam_vvt_input.cpp | 4 ++-- unit_tests/tests/trigger/test_nissan_vq_vvt.cpp | 3 +-- unit_tests/tests/trigger/test_quad_cam.cpp | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index d15f2bef2f..52177472b6 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -265,12 +265,17 @@ void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt, int index DECL tc->vvtSyncTimeNt[bankIndex][camIndex] = nowNt; - // we do NOT clamp VVT position into the [0, engineCycle) range - we expect vvtOffset to be configured so that - // it's not necessary - tc->vvtPosition[bankIndex][camIndex] = engineConfiguration->vvtOffsets[bankIndex * CAMS_PER_BANK + camIndex] - currentPosition; - if (tc->vvtPosition[bankIndex][camIndex] < -ENGINE(engineCycle) / 2 || tc->vvtPosition[bankIndex][camIndex] > ENGINE(engineCycle) / 2) { - warning(CUSTOM_ERR_VVT_OUT_OF_RANGE, "Please adjust vvtOffset since position %f", tc->vvtPosition); + auto vvtPosition = engineConfiguration->vvtOffsets[bankIndex * CAMS_PER_BANK + camIndex] - currentPosition; + + // Wrap VVT position in to the range [-360, 360) + while (vvtPosition < -360) { + vvtPosition += 720; } + while (vvtPosition >= 360) { + vvtPosition -= 720; + } + + tc->vvtPosition[bankIndex][camIndex] = vvtPosition; if (index != 0) { // at the moment we use only primary VVT to sync crank phase diff --git a/unit_tests/tests/trigger/test_cam_vvt_input.cpp b/unit_tests/tests/trigger/test_cam_vvt_input.cpp index 4a765be527..8e53edeade 100644 --- a/unit_tests/tests/trigger/test_cam_vvt_input.cpp +++ b/unit_tests/tests/trigger/test_cam_vvt_input.cpp @@ -120,7 +120,7 @@ TEST(trigger, testCamInput) { ASSERT_NEAR(360 - 181, engine->triggerCentral.getVVTPosition(0, 0), EPS3D); } -TEST(sensors, testNB2CamInput) { +TEST(trigger, testNB2CamInput) { WITH_ENGINE_TEST_HELPER(MAZDA_MIATA_2003); // this crank trigger would be easier to test, crank shape is less important for this test @@ -167,7 +167,7 @@ TEST(sensors, testNB2CamInput) { // this third important front would give us first comparison between two real gaps hwHandleVvtCamSignal(TV_RISE, getTimeNowNt(), 0 PASS_ENGINE_PARAMETER_SUFFIX); - ASSERT_NEAR(-67.6 - 720 - 720, engine->triggerCentral.getVVTPosition(0, 0), EPS3D); + ASSERT_NEAR(-67.6, engine->triggerCentral.getVVTPosition(0, 0), EPS3D); // actually position based on VVT! ASSERT_EQ(totalRevolutionCountBeforeVvtSync + 2, engine->triggerCentral.triggerState.getTotalRevolutionCounter()); diff --git a/unit_tests/tests/trigger/test_nissan_vq_vvt.cpp b/unit_tests/tests/trigger/test_nissan_vq_vvt.cpp index 8fbc4ab075..1886439d35 100644 --- a/unit_tests/tests/trigger/test_nissan_vq_vvt.cpp +++ b/unit_tests/tests/trigger/test_nissan_vq_vvt.cpp @@ -132,6 +132,5 @@ TEST(nissan, vq_vvt) { ASSERT_NEAR(-testVvtOffset, tc->vvtPosition[0][0], EPS2D); ASSERT_NEAR(-testVvtOffset, tc->vvtPosition[1][0], EPS2D); -// todo: reducing warning here is a separate story - EXPECT_EQ(1, eth.recentWarnings()->getCount()); + EXPECT_EQ(0, eth.recentWarnings()->getCount()); } diff --git a/unit_tests/tests/trigger/test_quad_cam.cpp b/unit_tests/tests/trigger/test_quad_cam.cpp index e34a628d7b..39e169fe37 100644 --- a/unit_tests/tests/trigger/test_quad_cam.cpp +++ b/unit_tests/tests/trigger/test_quad_cam.cpp @@ -87,7 +87,7 @@ TEST(trigger, testQuadCam) { // this third important front would give us first comparison between two real gaps hwHandleVvtCamSignal(TV_RISE, getTimeNowNt(), secondCamSecondBank PASS_ENGINE_PARAMETER_SUFFIX); - ASSERT_NEAR(-2571.4, engine->triggerCentral.getVVTPosition(secondBank, secondCam), EPS3D); + ASSERT_NEAR(308.6, engine->triggerCentral.getVVTPosition(secondBank, secondCam), EPS3D); // actually position based on VVT! ASSERT_EQ(totalRevolutionCountBeforeVvtSync, engine->triggerCentral.triggerState.getTotalRevolutionCounter());