From b31a613045018ca4b4c345b9c91630b564065794 Mon Sep 17 00:00:00 2001 From: Martin Siggel Date: Sat, 10 Feb 2024 22:42:08 +0100 Subject: [PATCH 1/2] Fixed issues #2857 The main reason was a wrong digital / analog sync, if decimal shifts were != 0. Also fixed hanging digits for late transition, which happened at my own installation only. Also reverted back to the improved unit test system of Slider0007, which seemed to be mistakenly overwritten by someone. --- .../ClassFlowPostProcessing.cpp | 35 ++++++++--- .../ClassFlowPostProcessing.h | 3 +- .../test_flowpostprocessing.cpp | 62 ++++++++++++++++++- code/test/test_suite_flowcontroll.cpp | 37 +++++------ 4 files changed, 103 insertions(+), 34 deletions(-) diff --git a/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.cpp b/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.cpp index dbe1f11fd..3c773fa38 100644 --- a/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.cpp +++ b/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.cpp @@ -776,18 +776,21 @@ bool inRange(float val, float min, float max) * * @return The synchronized values as a string */ -std::string syncDigitalAnalog(const std::string& value, const std::string& digitalPrecision, - double analogDigitalShift) +std::string syncDigitalAnalog(const std::string& invalue, const std::string& digitalPrecision, + double analogDigitalShift, int decimalShift) { if (digitalPrecision.empty()) { - return value; + return invalue; } + // temporarily revert decimal shift + std::string value = ClassFlowPostProcessing::ShiftDecimal(invalue, -decimalShift); + const auto pos = value.find('.'); if (pos == std::string::npos) { - return value; + return invalue; } // disassemble value into pre and post comma part @@ -804,13 +807,19 @@ std::string syncDigitalAnalog(const std::string& value, const std::string& digit // Determine phase const bool inTransition = digitalPostComma > 0. && digitalPostComma < 10.; const bool postTransition = !inTransition && inRange(analogPostComma*10., analogDigitalShift, wrapAround(analogDigitalShift + 5.)); - const bool preTransition = !inTransition && inRange(analogPostComma*10., 0, analogDigitalShift); + const bool preTransition = inRange(analogPostComma*10., 0, analogDigitalShift); if (inRange(analogDigitalShift, 0.5, 5.)) { + // prevent hanging digit + if (digitalPostComma >= 9.0) + { + digitalPreComma += 1; + } + // late transition, last digit starts transition, when analog is between [0.5, 5) - if (inTransition || preTransition) + if ((digitalPostComma > 0. && digitalPostComma < 9.) || preTransition) { ESP_LOGD("syncDigitalAnalog", "Late digital transition. Increase digital value by 1."); digitalPreComma += 1; @@ -830,10 +839,15 @@ std::string syncDigitalAnalog(const std::string& value, const std::string& digit if (inTransition && analogPostComma < 0.5) { digitalPreComma += 1; } + else if (analogPostComma*10 < analogDigitalShift && digitalPostComma >= 9.) + { + // hanging digit + digitalPreComma += 1; + } } else { - return value; + return invalue; } // assemble result into string again, pad with zeros @@ -841,7 +855,9 @@ std::string syncDigitalAnalog(const std::string& value, const std::string& digit for (size_t i = preCommaNew.size(); i < nPreComma; ++i) preCommaNew = "0" + preCommaNew; - const std::string result = preCommaNew + "." + postComma; + // apply again decimal shift + const std::string result = ClassFlowPostProcessing::ShiftDecimal(preCommaNew + "." + postComma, + decimalShift); #if debugSync ESP_LOGD("syncDigitalAnalog", "result: %s", result.c_str()); @@ -977,7 +993,8 @@ bool ClassFlowPostProcessing::doFlow(string zwtime) if (NUMBERS[j]->digit_roi && NUMBERS[j]->analog_roi) { // Synchronize potential misalignment between analog and digital part - NUMBERS[j]->ReturnValue = syncDigitalAnalog(NUMBERS[j]->ReturnValue, digitalPrecision, NUMBERS[j]->AnalogDigitalTransitionStart); + NUMBERS[j]->ReturnValue = syncDigitalAnalog(NUMBERS[j]->ReturnValue, digitalPrecision, NUMBERS[j]->AnalogDigitalTransitionStart, + NUMBERS[j]->DecimalShift); } #ifdef SERIAL_DEBUG diff --git a/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.h b/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.h index aae28e6a3..ef2496691 100644 --- a/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.h +++ b/code/components/jomjol_flowcontroll/ClassFlowPostProcessing.h @@ -31,7 +31,6 @@ class ClassFlowPostProcessing : ClassFlowTakeImage *flowTakeImage; bool LoadPreValue(void); - string ShiftDecimal(string in, int _decShift); string ErsetzteN(string, double _prevalue); float checkDigitConsistency(double input, int _decilamshift, bool _isanalog, double _preValue); @@ -78,6 +77,8 @@ class ClassFlowPostProcessing : std::vector* GetNumbers(){return &NUMBERS;}; string name(){return "ClassFlowPostProcessing";}; + + static string ShiftDecimal(string in, int _decShift); }; diff --git a/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp b/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp index 715d7a91a..e921e0695 100644 --- a/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp +++ b/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp @@ -543,12 +543,13 @@ void test_doFlowPP4() { std::string postProcess(std::vector digits, std::vector analogs, - float analog2DigitalTransition=0.0) + float analog2DigitalTransition=0.0, + int decimalShift=0) { std::unique_ptr undertestPost(init_do_flow(std::move(analogs), std::move(digits), Digital100, - false, false)); + false, false, decimalShift)); setAnalogdigitTransistionStart(undertestPost.get(), analog2DigitalTransition); return process_doFlow(undertestPost.get()); @@ -600,5 +601,62 @@ void test_doFlowEarlyTransition() } +void test_doFlowIssue2857() +{ + // reported by gec75 + float a2dt = 9.2; + int decimalShift = 3; + TEST_ASSERT_EQUAL_STRING("252090.0", postProcess({ 2.0, 5.0, 1.9}, { 0.8, 8.8, 9.9, 0.1}, + a2dt, decimalShift).c_str()); + + // reported by Kornelius777 + decimalShift = 0; + TEST_ASSERT_EQUAL_STRING("1017.8099", postProcess({ 0.0, 1.0, 0.0, 1.0, 7.0}, { 8.2, 0.9, 9.9, 9.8}, + a2dt, decimalShift).c_str()); + + // with hanging digit + TEST_ASSERT_EQUAL_STRING("1017.8099", postProcess({ 0.0, 1.0, 0.0, 1.0, 6.9}, { 8.2, 0.9, 9.9, 9.8}, + a2dt, decimalShift).c_str()); + + // and deccimal shift + decimalShift = -2; + TEST_ASSERT_EQUAL_STRING("10.178099", postProcess({ 0.0, 1.0, 0.0, 1.0, 6.9}, { 8.2, 0.9, 9.9, 9.8}, + a2dt, decimalShift).c_str()); + + + + // reported by marcniedersachsen + decimalShift = 0; + TEST_ASSERT_EQUAL_STRING("778.1480", postProcess({ 0.0, 7.0, 7.0, 7.9}, { 1.4, 4.7, 8.0, 0.5}, + a2dt, decimalShift).c_str()); + + decimalShift = 3; + TEST_ASSERT_EQUAL_STRING("778148.0", postProcess({ 0.0, 7.0, 7.0, 7.9}, { 1.4, 4.7, 8.0, 0.5}, + a2dt, decimalShift).c_str()); + + // reported by ohkaja + decimalShift = 0; + TEST_ASSERT_EQUAL_STRING("1052.6669", postProcess({ 0.0, 1.0, 10.0, 4.9, 2.0}, { 6.7, 6.7, 6.9, 9.1}, + a2dt, decimalShift).c_str()); + +} + + +void test_doFlowLateTransitionHanging() +{ + float a2dt = 3.6; + + // meter shows 012.4210 , this is after transition + TEST_ASSERT_EQUAL_STRING("12.4210", postProcess({0.0, 1.0, 1.9}, {4.3, 2.2, 1.0, 0.0}, a2dt).c_str()); + TEST_ASSERT_EQUAL_STRING("12.6210", postProcess({0.0, 1.0, 1.9}, {6.3, 2.2, 1.0, 0.0}, a2dt).c_str()); + + TEST_ASSERT_EQUAL_STRING("13.1210", postProcess({0.0, 1.0, 1.9}, {1.2, 2.2, 1.0, 0.0}, a2dt).c_str()); + + TEST_ASSERT_EQUAL_STRING("13.3610", postProcess({0.0, 1.0, 2.5}, {3.5, 6.2, 1.0, 0.0}, a2dt).c_str()); + + TEST_ASSERT_EQUAL_STRING("13.4510", postProcess({0.0, 1.0, 3.0}, {4.5, 5.2, 1.0, 0.0}, a2dt).c_str()); + + TEST_ASSERT_EQUAL_STRING("13.4510", postProcess({0.0, 1.0, 2.9}, {4.5, 5.2, 1.0, 0.0}, a2dt).c_str()); +} diff --git a/code/test/test_suite_flowcontroll.cpp b/code/test/test_suite_flowcontroll.cpp index 2819ac2b1..d5027a13c 100644 --- a/code/test/test_suite_flowcontroll.cpp +++ b/code/test/test_suite_flowcontroll.cpp @@ -138,6 +138,14 @@ void task_UnityTesting(void *pvParameter) RUN_TEST(test_doFlowPP3); printf("---------------------------------------------------------------------------\n"); RUN_TEST(test_doFlowPP4); + printf("---------------------------------------------------------------------------\n"); + RUN_TEST(test_doFlowLateTransition); + printf("---------------------------------------------------------------------------\n"); + RUN_TEST(test_doFlowEarlyTransition); + printf("---------------------------------------------------------------------------\n"); + RUN_TEST(test_doFlowIssue2857); + printf("---------------------------------------------------------------------------\n"); + RUN_TEST(test_doFlowLateTransitionHanging); UNITY_END(); while(1); @@ -149,26 +157,11 @@ void task_UnityTesting(void *pvParameter) */ extern "C" void app_main() { - initGPIO(); - Init_NVS_SDCard(); - esp_log_level_set("*", ESP_LOG_DEBUG); // set all components to ERROR level - - UNITY_BEGIN(); - RUN_TEST(testNegative_Issues); - RUN_TEST(testNegative); - /* - RUN_TEST(test_analogToDigit_Standard); - RUN_TEST(test_analogToDigit_Transition); - RUN_TEST(test_doFlowPP); - RUN_TEST(test_doFlowPP1); - RUN_TEST(test_doFlowPP2); - RUN_TEST(test_doFlowPP3); - RUN_TEST(test_doFlowPP4); - RUN_TEST(test_doFlowLateTransition); - RUN_TEST(test_doFlowEarlyTransition); - - // getReadoutRawString test - RUN_TEST(test_getReadoutRawString); - */ - UNITY_END(); + initGPIO(); + Init_NVS_SDCard(); + esp_log_level_set("*", ESP_LOG_DEBUG); // set all components to DEBUG level + + // Create dedicated testing task (heap size can be configured - large enough to handle a lot of testing cases) + // ******************************************** + xTaskCreate(&task_UnityTesting, "task_UnityTesting", 12 * 1024, NULL, tskIDLE_PRIORITY+2, NULL); } From 16a2bf53c47bafa33a963ba074509b2899529aab Mon Sep 17 00:00:00 2001 From: Martin Siggel Date: Mon, 12 Feb 2024 20:00:37 +0100 Subject: [PATCH 2/2] Added more tests from issue #2857 --- .../test_flowpostprocessing.cpp | 41 ++++++++++++++++++- code/test/test_suite_flowcontroll.cpp | 2 + 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp b/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp index e921e0695..897f568b4 100644 --- a/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp +++ b/code/test/components/jomjol-flowcontroll/test_flowpostprocessing.cpp @@ -598,7 +598,17 @@ void test_doFlowEarlyTransition() TEST_ASSERT_EQUAL_STRING("12.8123", postProcess({0.0, 1.0, 3.0}, {8.1, 1.2, 2.3, 3.0}, a2dt).c_str()); TEST_ASSERT_EQUAL_STRING("13.1234", postProcess({0.0, 1.0, 3.0}, {1.2, 2.3, 3.4, 4.0}, a2dt).c_str()); +} + + +void test_doFlowEarlyTransitionEdgeCase() +{ + float a2dt = 8.; + + // THIS TEST FAILS and reports 9.50. Not sure yet why. Predecessor value seems to play in here. + TEST_ASSERT_EQUAL_STRING("99.50", postProcess({0.0, 0.0, 9.9, 9.0}, {5.0, 0.0}, a2dt).c_str()); + TEST_ASSERT_EQUAL_STRING("99.95", postProcess({0.0, 1.0, 0.0, 0.0}, {9.5, 5.0}, a2dt).c_str()); } void test_doFlowIssue2857() @@ -624,7 +634,6 @@ void test_doFlowIssue2857() a2dt, decimalShift).c_str()); - // reported by marcniedersachsen decimalShift = 0; TEST_ASSERT_EQUAL_STRING("778.1480", postProcess({ 0.0, 7.0, 7.0, 7.9}, { 1.4, 4.7, 8.0, 0.5}, @@ -639,7 +648,37 @@ void test_doFlowIssue2857() TEST_ASSERT_EQUAL_STRING("1052.6669", postProcess({ 0.0, 1.0, 10.0, 4.9, 2.0}, { 6.7, 6.7, 6.9, 9.1}, a2dt, decimalShift).c_str()); + // FrankCGN01 + decimalShift = -3; + TEST_ASSERT_EQUAL_STRING("159.3659", postProcess({ 0.9, 4.8, 9.0, 3.0, 6.0, 5.0}, { 9.6}, + a2dt, decimalShift).c_str()); + + // THIS TEST FAILS + // THIS TEST CASE COULD BE INVALID + // + // Second test in https://github.com/jomjol/AI-on-the-edge-device/issues/2857#issuecomment-1937452352 + // The last digit seems to be falsely recongnized. It looks like a regular "2" (no transition) + // but it is recognized by the inference as "2.5". + // + // @TODO: Feedback required by @FrankCGN01 + decimalShift = -3; + TEST_ASSERT_EQUAL_STRING("159.5022", postProcess({ 0.9, 4.9, 9.0, 5.0, 0.0, 2.5}, { 2.2}, + a2dt, decimalShift).c_str()); + + // THIS TEST FAILS + // + // reported by penapena + // Note: this is a strange example, as the last digit (4.4) seems to have very early transition + // + // @TODO: The analog to digital value needs to be determined. Feed required by @penapena + decimalShift = 0; + TEST_ASSERT_EQUAL_STRING("124.4981", postProcess({ 0.0, 1.0, 2.0, 4.4}, { 5.1, 9.8, 8.3, 1.6}, + a2dt, decimalShift).c_str()); + // reported by warnmat + decimalShift = 0; + TEST_ASSERT_EQUAL_STRING("51.653", postProcess({ 0.1, 0.1, 5.0, 1.0}, { 6.7, 5.4, 3.1}, + a2dt, decimalShift).c_str()); } diff --git a/code/test/test_suite_flowcontroll.cpp b/code/test/test_suite_flowcontroll.cpp index d5027a13c..cf62eecb3 100644 --- a/code/test/test_suite_flowcontroll.cpp +++ b/code/test/test_suite_flowcontroll.cpp @@ -143,6 +143,8 @@ void task_UnityTesting(void *pvParameter) printf("---------------------------------------------------------------------------\n"); RUN_TEST(test_doFlowEarlyTransition); printf("---------------------------------------------------------------------------\n"); + RUN_TEST(test_doFlowEarlyTransitionEdgeCase); + printf("---------------------------------------------------------------------------\n"); RUN_TEST(test_doFlowIssue2857); printf("---------------------------------------------------------------------------\n"); RUN_TEST(test_doFlowLateTransitionHanging);