From e0acec2a63d238e188893f84ea18c092cd5f0a87 Mon Sep 17 00:00:00 2001 From: rusefi Date: Thu, 8 Aug 2019 22:57:22 -0400 Subject: [PATCH] WTF is wrong with MRE_miata_na6 config? operationMode complexity #898 fatal error is the best I can come up with quickly --- firmware/controllers/algo/engine.cpp | 9 +++++++++ firmware/controllers/algo/obd_error_codes.h | 2 +- .../controllers/trigger/decoders/trigger_mazda.cpp | 6 ++++++ .../trigger/decoders/trigger_structure.cpp | 1 + .../controllers/trigger/decoders/trigger_structure.h | 10 ++++++++++ unit_tests/tests/test_issue_898.cpp | 11 ++++++----- 6 files changed, 33 insertions(+), 6 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 7373da3d6a..88b7912229 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -62,6 +62,15 @@ void Engine::eInitializeTriggerShape(Logging *logger DECLARE_ENGINE_PARAMETER_SU engineConfiguration->ambiguousOperationMode, engineConfiguration->useOnlyRisingEdgeForTrigger, &engineConfiguration->trigger)); + if (TRIGGER_SHAPE(bothFrontsRequired) && engineConfiguration->useOnlyRisingEdgeForTrigger) { +#if EFI_PROD_CODE || EFI_SIMULATOR + firmwareError(CUSTOM_ERR_BOTH_FRONTS_REQUIRED, "Inconsistent trigger setup"); +#else + warning(CUSTOM_ERR_BOTH_FRONTS_REQUIRED, "Inconsistent trigger setup"); +#endif + } + + if (!TRIGGER_SHAPE(shapeDefinitionError)) { /** * this instance is used only to initialize 'this' TriggerShape instance diff --git a/firmware/controllers/algo/obd_error_codes.h b/firmware/controllers/algo/obd_error_codes.h index 91e134ac76..0310b44830 100644 --- a/firmware/controllers/algo/obd_error_codes.h +++ b/firmware/controllers/algo/obd_error_codes.h @@ -2066,7 +2066,7 @@ typedef enum { CUSTOM_CJ125_1 = 6701, CUSTOM_CJ125_2 = 6702, CUSTOM_ERR_6703 = 6703, - CUSTOM_ERR_6704 = 6704, + CUSTOM_ERR_BOTH_FRONTS_REQUIRED = 6704, CUSTOM_TLE8888 = 6705, CUSTOM_ERR_6706 = 6706, CUSTOM_ERR_6707 = 6707, diff --git a/firmware/controllers/trigger/decoders/trigger_mazda.cpp b/firmware/controllers/trigger/decoders/trigger_mazda.cpp index 48e84799b4..7efeb78498 100644 --- a/firmware/controllers/trigger/decoders/trigger_mazda.cpp +++ b/firmware/controllers/trigger/decoders/trigger_mazda.cpp @@ -25,6 +25,8 @@ void initializeMazdaMiataNaShape(TriggerShape *s) { s->setTriggerSynchronizationGap2(1.4930 * 0.6f, 1.4930 * 1.3f); s->useRiseEdge = false; + s->bothFrontsRequired = true; + s->tdcPosition = 294; s->isSynchronizationNeeded = true; @@ -104,6 +106,8 @@ static void initializeMazdaMiataNb1ShapeWithOffset(TriggerShape *s, float offset s->setTriggerSynchronizationGap(0.11f); s->useRiseEdge = false; + s->bothFrontsRequired = true; + s->invertOnAdd = true; s->tdcPosition = 276; @@ -158,6 +162,8 @@ void configureMazdaProtegeSOHC(TriggerShape *s) { // float w = 720 / 4 * 0.215; float a = 5; + s->bothFrontsRequired = true; + float z = 0.093; a = 180; s->addEvent720(a - z * 720, T_PRIMARY, TV_RISE); diff --git a/firmware/controllers/trigger/decoders/trigger_structure.cpp b/firmware/controllers/trigger/decoders/trigger_structure.cpp index dfad39a3dd..19223b971b 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.cpp +++ b/firmware/controllers/trigger/decoders/trigger_structure.cpp @@ -62,6 +62,7 @@ TriggerShape::TriggerShape() : void TriggerShape::initialize(operation_mode_e operationMode, bool needSecondTriggerInput) { isSynchronizationNeeded = true; // that's default value + bothFrontsRequired = false; this->needSecondTriggerInput = needSecondTriggerInput; memset(expectedDutyCycle, 0, sizeof(expectedDutyCycle)); memset(eventAngles, 0, sizeof(eventAngles)); diff --git a/firmware/controllers/trigger/decoders/trigger_structure.h b/firmware/controllers/trigger/decoders/trigger_structure.h index b48b3162f6..a5cc57fd4f 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.h +++ b/firmware/controllers/trigger/decoders/trigger_structure.h @@ -103,6 +103,16 @@ public: */ bool shapeDefinitionError; + /** + * https://github.com/rusefi/rusefi/issues/898 + * User can choose for example Miata trigger which is not compatible with useOnlyRisingEdgeForTrigger option + * Such contradictory configuration causes a very hard to identify issue and for the sake of usability it's better to + * just crash with a very visible fatal error + * + * One day a nicer implementation could be simply ignoring 'useOnlyRisingEdgeForTrigger' in case of 'bothFrontsRequired' + */ + bool bothFrontsRequired; + /** * this variable is incremented after each trigger shape redefinition * See also diff --git a/unit_tests/tests/test_issue_898.cpp b/unit_tests/tests/test_issue_898.cpp index 299bb9dc32..d18216209d 100644 --- a/unit_tests/tests/test_issue_898.cpp +++ b/unit_tests/tests/test_issue_898.cpp @@ -6,6 +6,7 @@ */ #include "engine_test_helper.h" +extern WarningCodeState unitTestWarningCodeState; static void boardConfigurationForIssue898(engine_configuration_s *engineConfiguration) { setOperationMode(engineConfiguration, FOUR_STROKE_CRANK_SENSOR); @@ -14,12 +15,12 @@ static void boardConfigurationForIssue898(engine_configuration_s *engineConfigur } TEST(issues, issue898) { -// works without extra board settings - EngineTestHelper eth(MRE_MIATA_NA6); -// fails like this with self-contradictory trigger definition -// EngineTestHelper eth(MRE_MIATA_NA6, &boardConfigurationForIssue898); + EngineTestHelper eth(MRE_MIATA_NA6, &boardConfigurationForIssue898); EXPAND_EngineTestHelper; + ASSERT_EQ(TRUE, engine->triggerCentral.triggerShape.shapeDefinitionError) << "MRE_MIATA_NA6 shapeDefinitionError"; - ASSERT_EQ(0, engine->triggerCentral.triggerShape.shapeDefinitionError) << "MRE_MIATA_NA6 shapeDefinitionError"; + ASSERT_EQ( 2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testFuelSchedulerBug299smallAndMedium"; + ASSERT_EQ(CUSTOM_ERR_BOTH_FRONTS_REQUIRED, unitTestWarningCodeState.recentWarnings.get(0)); + ASSERT_EQ(CUSTOM_ERR_TRIGGER_SYNC, unitTestWarningCodeState.recentWarnings.get(1)); }