Skip to content

Commit

Permalink
getVVTPosition returns expected<>
Browse files Browse the repository at this point in the history
  • Loading branch information
@mck1117
mck1117 committed Sep 13, 2024
1 parent 85e3122 commit ec043ad
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 54 deletions.
2 changes: 1 addition & 1 deletion firmware/controllers/engine_cycle/high_pressure_fuel_pump.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ angle_t HpfpLobe::findNextLobe() {
// TODO: Is the sign correct here? + means ATDC?
vvt = engine->triggerCentral.getVVTPosition(
(engineConfiguration->hpfpCam - 1) / 2 & 1, // Bank
(engineConfiguration->hpfpCam - 1) & 1); // Cam
(engineConfiguration->hpfpCam - 1) & 1).value_or(0); // Cam
}

return engineConfiguration->hpfpPeakPos + vvt + next_index * 720 / lobes;
Expand Down
14 changes: 11 additions & 3 deletions firmware/controllers/trigger/trigger_central.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,14 @@ expected<angle_t> TriggerCentral::getVVTPosition(uint8_t bankIndex, uint8_t camI
}

// TODO: return unexpected if timed out
auto& vvt = vvtPosition[bankIndex][camIndex];

return vvtPosition[bankIndex][camIndex];
if (vvt.t.hasElapsedSec(1)) {
// 1s timeout on VVT angle
return unexpected;
} else {
return vvt.angle;
}
}

/**
Expand Down Expand Up @@ -341,11 +347,13 @@ void hwHandleVvtCamSignal(bool isRising, efitick_t nowNt, int index) {
}

// Only record VVT position if we have full engine sync - may be bogus before that point
auto& vvtPos = tc->vvtPosition[bankIndex][camIndex];
if (tc->triggerState.hasSynchronizedPhase()) {
tc->vvtPosition[bankIndex][camIndex] = vvtPosition;
vvtPos.angle = vvtPosition;
} else {
tc->vvtPosition[bankIndex][camIndex] = 0;
vvtPos.angle = 0;
}
vvtPos.t.reset(nowNt);
}

int triggerReentrant = 0;
Expand Down
6 changes: 5 additions & 1 deletion firmware/controllers/trigger/trigger_central.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,11 @@ class TriggerCentral final : public trigger_central_s {
#endif // EFI_UNIT_TEST

// synchronization event position
angle_t vvtPosition[BANKS_COUNT][CAMS_PER_BANK];
struct VvtPos {
angle_t angle;
Timer t;
};
VvtPos vvtPosition[BANKS_COUNT][CAMS_PER_BANK];

#if EFI_SHAFT_POSITION_INPUT
PrimaryTriggerDecoder triggerState;
Expand Down
3 changes: 2 additions & 1 deletion unit_tests/tests/actuators/test_vvt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ TEST(Vvt, TestSetPoint) {
TEST(Vvt, observePlant) {
EngineTestHelper eth(engine_type_e::TEST_ENGINE);

engine->triggerCentral.vvtPosition[0][0] = 23;
engine->triggerCentral.vvtPosition[0][0].t.reset();
engine->triggerCentral.vvtPosition[0][0].angle = 23;

VvtController dut(0, 0, 0);
dut.init(nullptr, nullptr);
Expand Down
12 changes: 8 additions & 4 deletions unit_tests/tests/test_hpfp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ TEST(HPFP, Lobe) {
engineConfiguration->hpfpPeakPos = 123;
engineConfiguration->hpfpCamLobes = 3;

engine->triggerCentral.vvtPosition[0][0] = 20; // Bank 0
engine->triggerCentral.vvtPosition[0][1] = 40;
engine->triggerCentral.vvtPosition[1][0] = 60; // Bank 1
engine->triggerCentral.vvtPosition[1][1] = 80;
engine->triggerCentral.vvtPosition[0][0].angle = 20; // Bank 0
engine->triggerCentral.vvtPosition[0][0].t.reset();
engine->triggerCentral.vvtPosition[0][1].angle = 40;
engine->triggerCentral.vvtPosition[0][1].t.reset();
engine->triggerCentral.vvtPosition[1][0].angle = 60; // Bank 1
engine->triggerCentral.vvtPosition[1][0].t.reset();
engine->triggerCentral.vvtPosition[1][1].angle = 80;
engine->triggerCentral.vvtPosition[1][1].t.reset();

HpfpLobe lobe;

Expand Down
6 changes: 3 additions & 3 deletions unit_tests/tests/trigger/test_cam_vvt_input.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ TEST(trigger, testCamInput) {

// asserting that error code has cleared
ASSERT_EQ(0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testCamInput #3";
EXPECT_NEAR_M3(71, engine->triggerCentral.getVVTPosition(0, 0));
EXPECT_NEAR_M3(71, engine->triggerCentral.getVVTPosition(0, 0).value_or(0));
}

TEST(trigger, testNB2CamInput) {
Expand Down Expand Up @@ -170,7 +170,7 @@ TEST(trigger, testNB2CamInput) {
eth.moveTimeForwardUs(MS2US(10));
hwHandleVvtCamSignal(true, getTimeNowNt(), 0);

ASSERT_FLOAT_EQ(0, engine->triggerCentral.getVVTPosition(0, 0));
ASSERT_FALSE(engine->triggerCentral.getVVTPosition(0, 0));
ASSERT_EQ(totalRevolutionCountBeforeVvtSync, engine->triggerCentral.triggerState.getCrankSynchronizationCounter());

// Third gap - long
Expand All @@ -180,7 +180,7 @@ TEST(trigger, testNB2CamInput) {
eth.moveTimeForwardUs(MS2US( 30));
hwHandleVvtCamSignal(true, getTimeNowNt(), 0);

EXPECT_NEAR(297.5f, engine->triggerCentral.getVVTPosition(0, 0), EPS2D);
EXPECT_NEAR(297.5f, engine->triggerCentral.getVVTPosition(0, 0).value_or(0), EPS2D);
// actually position based on VVT!
ASSERT_EQ(totalRevolutionCountBeforeVvtSync + 3, engine->triggerCentral.triggerState.getCrankSynchronizationCounter());

Expand Down
6 changes: 3 additions & 3 deletions unit_tests/tests/trigger/test_nissan_vq_vvt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ TEST(nissan, vq_vvt) {

ASSERT_TRUE(tc->vvtState[0][0].getShaftSynchronized());
// let's celebrate that vvtPosition stays the same
ASSERT_NEAR(34, tc->vvtPosition[0][0], EPS2D) << "queueIndex=" << queueIndex;
ASSERT_NEAR(34, tc->getVVTPosition(0, 0).value_or(0), EPS2D) << "queueIndex=" << queueIndex;
queueIndex++;
}
ASSERT_TRUE(queueIndex == 422) << "Total queueIndex=" << queueIndex;

ASSERT_TRUE(tc->vvtState[1][0].getShaftSynchronized());

ASSERT_NEAR(34, tc->vvtPosition[0][0], EPS2D);
ASSERT_NEAR(34, tc->vvtPosition[1][0], EPS2D);
ASSERT_NEAR(34, tc->getVVTPosition(0, 0).value_or(0), EPS2D);
ASSERT_NEAR(34, tc->getVVTPosition(1, 0).value_or(0), EPS2D);

EXPECT_EQ(0, eth.recentWarnings()->getCount());
}
24 changes: 12 additions & 12 deletions unit_tests/tests/trigger/test_quad_cam.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ TEST(trigger, testQuadCam) {
int secondCamSecondBank = secondBank * CAMS_PER_BANK + secondCam;

// Cams should have no position yet
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(firstBank, firstCam));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(firstBank, secondCam));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(secondBank, firstCam));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(secondBank, secondCam));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(firstBank, firstCam).value_or(0));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(firstBank, secondCam).value_or(0));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(secondBank, firstCam).value_or(0));
ASSERT_EQ(0, engine->triggerCentral.getVVTPosition(secondBank, secondCam).value_or(0));

hwHandleVvtCamSignal(true, getTimeNowNt(), firstCam);
hwHandleVvtCamSignal(true, getTimeNowNt(), secondCam);
Expand All @@ -68,10 +68,10 @@ TEST(trigger, testQuadCam) {
float basePos = -80.2f;

// All four cams should now have the same position
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(firstBank, firstCam));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(firstBank, secondCam));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(secondBank, firstCam));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(secondBank, secondCam));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(firstBank, firstCam).value_or(0));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(firstBank, secondCam).value_or(0));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(secondBank, firstCam).value_or(0));
EXPECT_NEAR_M3(basePos, engine->triggerCentral.getVVTPosition(secondBank, secondCam).value_or(0));

// Now fire cam events again, but with time gaps between each
eth.moveTimeForwardMs(1);
Expand All @@ -85,8 +85,8 @@ TEST(trigger, testQuadCam) {

// All four cams should have different positions, each retarded by 1ms from the last
float oneMsDegrees = 1000 / engine->rpmCalculator.oneDegreeUs;
EXPECT_NEAR(basePos - oneMsDegrees * 1, engine->triggerCentral.getVVTPosition(firstBank, firstCam), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 2, engine->triggerCentral.getVVTPosition(firstBank, secondCam), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 3, engine->triggerCentral.getVVTPosition(secondBank, firstCam), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 4, engine->triggerCentral.getVVTPosition(secondBank, secondCam), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 1, engine->triggerCentral.getVVTPosition(firstBank, firstCam).value_or(0), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 2, engine->triggerCentral.getVVTPosition(firstBank, secondCam).value_or(0), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 3, engine->triggerCentral.getVVTPosition(secondBank, firstCam).value_or(0), EPS3D);
EXPECT_NEAR(basePos - oneMsDegrees * 4, engine->triggerCentral.getVVTPosition(secondBank, secondCam).value_or(0), EPS3D);
}
6 changes: 3 additions & 3 deletions unit_tests/tests/trigger/test_real_cas_24_plus_1.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ TEST(realCas24Plus1, spinningOnBench) {
EXPECT_NEAR(rpm, 915.08f, 0.1);
}

float vvt = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
if (vvt != 0) {
auto vvt = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
if (vvt) {
// cam position should never be reported outside of correct range
EXPECT_TRUE(vvt > -10 && vvt < -9);
EXPECT_TRUE(vvt.Value > -10 && vvt.Value < -9);
}
}

Expand Down
18 changes: 9 additions & 9 deletions unit_tests/tests/trigger/test_real_cranking_nissan_vq40.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,27 @@ static void test(int engineSyncCam, float camOffsetAdd) {

while (reader.haveMore()) {
reader.processLine(&eth);
float vvt1 = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
float vvt2 = engine->triggerCentral.getVVTPosition(/*bankIndex*/1, /*camIndex*/0);
auto vvt1 = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
auto vvt2 = engine->triggerCentral.getVVTPosition(/*bankIndex*/1, /*camIndex*/0);

if (vvt1 != 0) {
if (vvt1 && vvt1.Value != 0) {
if (!hasSeenFirstVvt) {
EXPECT_NEAR(vvt1, 1.4, /*precision*/1);
EXPECT_NEAR(vvt1.Value, 1.4, /*precision*/1);
hasSeenFirstVvt = true;
}

// cam position should never be reported outside of correct range
EXPECT_TRUE(vvt1 > -3 && vvt1 < 3);
EXPECT_TRUE(vvt1.Value > -3 && vvt1.Value < 3);
}

if (vvt2 != 0) {
if (vvt2) {
// cam position should never be reported outside of correct range
EXPECT_TRUE(vvt2 > -3 && vvt2 < 3);
EXPECT_TRUE(vvt2.Value > -3 && vvt2.Value < 3);
}
}

EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0), 1.351, 1e-2);
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/1, /*camIndex*/0), 1.548, 1e-2);
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0).value_or(0), 1.351, 1e-2);
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/1, /*camIndex*/0).value_or(0), 1.548, 1e-2);
ASSERT_EQ(102, round(Sensor::getOrZero(SensorType::Rpm)))<< reader.lineIndex();

// TODO: why warnings?
Expand Down
6 changes: 3 additions & 3 deletions unit_tests/tests/trigger/test_real_k20.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ TEST(realk20, cranking) {
// EXPECT_TRUE(vvtI > -10 && vvtI < 10);
// }

float vvtE = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/1);
if (vvtE != 0) {
EXPECT_TRUE(vvtE > -10 && vvtE < 10);
auto vvtE = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/1);
if (vvtE) {
EXPECT_TRUE(vvtE.Value > -10 && vvtE.Value < 10);
}

}
Expand Down
10 changes: 6 additions & 4 deletions unit_tests/tests/trigger/test_real_nb2_cranking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ TEST(realCrankingNB2, normalCranking) {
}

// VVT position nearly zero!
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(0, 0), 11.2627f, 1e-4);
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(0, 0).value_or(0), 11.2627f, 1e-4);

// Check the number of times VVT information was used to adjust crank phase
// This should happen exactly once: once we sync, we shouldn't lose it.
Expand All @@ -42,10 +42,12 @@ TEST(realCrankingNB2, crankingMissingInjector) {

while (reader.haveMore()) {
reader.processLine(&eth);
}

// VVT position nearly zero!
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(0, 0), 4.476928f, 1e-4);
if (auto vvt = engine->triggerCentral.getVVTPosition(0, 0)) {
// VVT position nearly zero!
EXPECT_NEAR(vvt.Value, 0, 20);
}
}

ASSERT_EQ(316, round(Sensor::getOrZero(SensorType::Rpm)));

Expand Down
14 changes: 7 additions & 7 deletions unit_tests/tests/trigger/test_toyota_3_tooth_cam.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ TEST(Toyota3ToothCam, RealEngineRunning) {

while (reader.haveMore()) {
reader.processLine(&eth);
float vvt1 = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);
auto vvt1 = engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0);

if (vvt1 != 0) {
if (vvt1) {
if (!hasSeenFirstVvt) {
EXPECT_NEAR(vvt1, 0, /*precision*/1);
EXPECT_NEAR(vvt1.Value, 0, /*precision*/1);
hasSeenFirstVvt = true;
}

// cam position should never be reported outside of correct range
EXPECT_TRUE(vvt1 > -3 && vvt1 < 3);
EXPECT_TRUE(vvt1.Value > -3 && vvt1.Value < 3);
}
}

EXPECT_EQ(getTriggerCentral()->triggerState.camResyncCounter, 0);

EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0), 0, 1);
EXPECT_NEAR(engine->triggerCentral.getVVTPosition(/*bankIndex*/0, /*camIndex*/0).value_or(0), 0, 1);
ASSERT_EQ(3078, round(Sensor::getOrZero(SensorType::Rpm)));

// TODO: why warnings?
Expand Down Expand Up @@ -103,14 +103,14 @@ static void test3tooth(size_t revsBeforeVvt, size_t teethBeforeVvt, bool expectS
EXPECT_EQ(expectSync, getTriggerCentral()->triggerState.hasSynchronizedPhase());

if (expectSync) {
EXPECT_EQ(5, getTriggerCentral()->getVVTPosition(0, 0));
EXPECT_EQ(5, getTriggerCentral()->getVVTPosition(0, 0).value_or(0));

// Bump ahead by one tooth
eth.fireFall(1);
eth.fireRise(9);
ASSERT_EQ(60, getTriggerCentral()->currentEngineDecodedPhase);
} else {
EXPECT_EQ(0, getTriggerCentral()->getVVTPosition(0, 0));
EXPECT_EQ(0, getTriggerCentral()->getVVTPosition(0, 0).value_or(0));
}
}

Expand Down

0 comments on commit ec043ad

Please sign in to comment.