From c85358967b8622dd8b0331013d66b56649d08430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Wed, 22 May 2019 11:01:48 +0200 Subject: [PATCH] [POST/HardwareTest] Change pattern in LCDDataTest The pattern is a tiling of checkers of colors 2**k with k between 0 and 15 and their contraries. This fixed a problem of failing LCD screens not detected. --- .../lcd_data_test_controller.cpp | 2 +- apps/hardware_test/lcd_data_test_controller.h | 18 +-- apps/on_boarding/power_on_self_test.cpp | 26 ++-- apps/on_boarding/power_on_self_test.h | 2 + apps/shared/post_and_hardware_tests.cpp | 141 +++--------------- apps/shared/post_and_hardware_tests.h | 18 +-- ion/include/ion/display.h | 6 +- ion/src/device/shared/drivers/display.cpp | 23 ++- ion/src/device/shared/drivers/display.h | 2 +- ion/src/shared/dummy/display.cpp | 2 +- 10 files changed, 72 insertions(+), 168 deletions(-) diff --git a/apps/hardware_test/lcd_data_test_controller.cpp b/apps/hardware_test/lcd_data_test_controller.cpp index 07147c483..befab0e3b 100644 --- a/apps/hardware_test/lcd_data_test_controller.cpp +++ b/apps/hardware_test/lcd_data_test_controller.cpp @@ -15,7 +15,7 @@ bool LCDDataTestController::handleEvent(Ion::Events::Event event) { } void LCDDataTestController::viewWillAppear() { - bool testOK = Shared::POSTAndHardwareTests::LCDDataOK() && Shared::POSTAndHardwareTests::FastLCDDataOK(); + bool testOK = Shared::POSTAndHardwareTests::LCDDataOK(k_LCDTestIterationsCount); m_view.lcdDataStateTextView()->setText(testOK ? k_lcdDataOKText : k_lcdDataFailTest); m_view.setColor(testOK ? KDColorGreen : KDColorRed); } diff --git a/apps/hardware_test/lcd_data_test_controller.h b/apps/hardware_test/lcd_data_test_controller.h index f51560a54..e349761e4 100644 --- a/apps/hardware_test/lcd_data_test_controller.h +++ b/apps/hardware_test/lcd_data_test_controller.h @@ -8,15 +8,14 @@ namespace HardwareTest { class LCDDataTestController : public ViewController { -/* There are three types of tests, where a pattern is pushed to the screen and - * the number of invalid pixels then counted. - * - Test 1: Tile the screen with color patches. Tiling increases the number - * of border mistakes. - * - Test 2: Push one color to the whole screen in one step. It shows errors - * that appear on large and fast pushes. - * - Test 3: Color the screen by alterning one pixel black and one pixel - * white (maximal data difference), at maximal data writing speed. - * Tests 1 and 2 are done for a few different colors. */ +/* We want to test that: + * - Command/data switching is OK, + * - Data is correctly sent, + * - There are no short-circuits between the data wires. + * We thus send a tiled pattern (to test command/data switching), where each + * tile is a checker of a color and its contrary (to tests that Data is sent + * OK). To test each of the 16 data wires for short-circuits, we use 16 colors: + * 2**k with 0 <= k < 16. */ public: LCDDataTestController(Responder * parentResponder) : @@ -43,6 +42,7 @@ private: }; constexpr static const char * k_lcdDataOKText = "LCD DATA: OK"; constexpr static const char * k_lcdDataFailTest = "LCD DATA: FAIL"; + constexpr static int k_LCDTestIterationsCount = 20; ContentView m_view; }; diff --git a/apps/on_boarding/power_on_self_test.cpp b/apps/on_boarding/power_on_self_test.cpp index c522ea439..3d3d4e22a 100644 --- a/apps/on_boarding/power_on_self_test.cpp +++ b/apps/on_boarding/power_on_self_test.cpp @@ -6,17 +6,25 @@ namespace OnBoarding { KDColor PowerOnSelfTest::Perform() { KDColor previousLEDColor = Ion::LED::getColor(); + + /* Light up the LED in blue now. If VBlank test fails, we end up in an + * infinite loop and the LED will still be lit up in blue. */ Ion::LED::setColor(KDColorBlue); - if (Shared::POSTAndHardwareTests::VBlankOK() - && Shared::POSTAndHardwareTests::FastLCDDataOK()) { - /* If VBlank test fails, we end up in an infinite loop and the LED will be - * lit up in blue. */ - if (Shared::POSTAndHardwareTests::BatteryOK()) { - Ion::LED::setColor(KDColorGreen); - } else { - Ion::LED::setColor(KDColorRed); - } + + // Screen tests + bool screenTestsOK = Shared::POSTAndHardwareTests::VBlankOK() && Shared::POSTAndHardwareTests::LCDDataOK(k_LCDTestIterationsCount); + + // We push a white screen so that the LCD Data test is invisible for the user. + Ion::Display::waitForVBlank(); + Ion::Display::pushRectUniform(KDRect(0, 0, Ion::Display::Width, Ion::Display::Height), KDColorWhite); + Ion::Display::waitForVBlank(); + + if (screenTestsOK) { + // Battery test + bool batteryTestOK = Shared::POSTAndHardwareTests::BatteryOK(); + Ion::LED::setColor(batteryTestOK ? KDColorGreen : KDColorRed); } + return previousLEDColor; } diff --git a/apps/on_boarding/power_on_self_test.h b/apps/on_boarding/power_on_self_test.h index d405aa53b..58d45e699 100644 --- a/apps/on_boarding/power_on_self_test.h +++ b/apps/on_boarding/power_on_self_test.h @@ -10,6 +10,8 @@ public: /* Performs self tests, lights up the LED to indicate the tests results and * returns the LED color previous to the tests. */ static KDColor Perform(); +private: + constexpr static int k_LCDTestIterationsCount = 5; }; } diff --git a/apps/shared/post_and_hardware_tests.cpp b/apps/shared/post_and_hardware_tests.cpp index bbbb7fe85..e1661d7e7 100644 --- a/apps/shared/post_and_hardware_tests.cpp +++ b/apps/shared/post_and_hardware_tests.cpp @@ -16,131 +16,24 @@ bool POSTAndHardwareTests::VBlankOK() { return true; } -bool POSTAndHardwareTests::FastLCDDataOK() { - /* We separate TestDisplayColorTiling and TestDisplayColorUniform so that - * errors in TestDisplayColorUniform do not disappear just because the - * previous screen was all white. */ - if (!TestDisplayColorTiling(KDColorWhite)) { - return false; - } - bool result = TestDisplayMulticolor(); - // We end with a white screen so that the test is invisible for the user. - return TestDisplayColorUniform(KDColorWhite) && result; -} - -bool POSTAndHardwareTests::LCDDataOK() { - KDColor testColors[] = { - KDColorRed, KDColorGreen, KDColorBlue, - KDColor::RGB24(0xFFFF00), KDColor::RGB24(0xFF00FF), KDColor::RGB24(0x00FFFF), - KDColorWhite - }; - for (KDColor c : testColors) { - if (!TestDisplayColorTiling(c)) { - return false; - } - } - /* We separate TestDisplayColorTiling and TestDisplayColorUniform so that - * errors in TestDisplayColorUniform do not disappear just because the - * previous screen was with the same color. */ - for (KDColor c : testColors) { - if (!TestDisplayColorUniform(c)) { - return false; - } - } - return TestDisplayBlackWhite(); -} - -void POSTAndHardwareTests::ColorPixelBuffer(KDColor * pixels, int numberOfPixels, KDColor c) { - for (int i = 0; i < numberOfPixels; i++) { - pixels[i] = c; - } -} - -bool POSTAndHardwareTests::TestDisplayColorTiling(KDColor c) { - KDColor stamp[k_stampWidth*k_stampHeight]; - - // Tiling test with pushRect - ColorPixelBuffer(stamp, k_stampWidth * k_stampHeight, c); - for (int i = 0; i < Ion::Display::Width / k_stampWidth; i++) { - for (int j = 0; j < Ion::Display::Height / k_stampHeight; j++) { - Ion::Display::pushRect(KDRect(i * k_stampWidth, j * k_stampHeight, k_stampWidth, k_stampHeight), stamp); - } - } - return NumberOfNonColoredPixels(c) <= k_invalidPixelsLimit; -} - -bool POSTAndHardwareTests::TestDisplayColorUniform(KDColor c) { - // Test with pushRectUniform - Ion::Display::pushRectUniform(KDRect(KDPointZero, Ion::Display::Width, Ion::Display::Height), c); - return NumberOfNonColoredPixels(c) < k_invalidPixelsLimit; -} - -int POSTAndHardwareTests::NumberOfNonColoredPixels(KDColor wantedColor) { - KDColor stamp[k_stampWidth*k_stampHeight]; - int numberOfInvalidPixels = 0; - for (int i = 0; i < Ion::Display::Width / k_stampWidth; i++) { - for (int j = 0; j < Ion::Display::Height / k_stampHeight; j++) { - ColorPixelBuffer(stamp, k_stampWidth * k_stampHeight, wantedColor == KDColorBlack ? KDColorRed : KDColorBlack); - Ion::Display::pullRect(KDRect(i * k_stampWidth, j * k_stampHeight, k_stampWidth, k_stampHeight), stamp); - for (int k = 0; k < k_stampWidth * k_stampHeight; k++) { - if (stamp[k] != wantedColor) { - numberOfInvalidPixels++; - } - } - } - } - return numberOfInvalidPixels; -} - -bool POSTAndHardwareTests::TestDisplayBlackWhite() { - Ion::Display::POSTPushBlackWhite(); - KDColor stamp[k_stampWidth*k_stampHeight]; - int numberOfInvalidPixels = 0; - for (int i = 0; i < Ion::Display::Width/k_stampWidth; i++) { - ColorPixelBuffer(stamp, k_stampWidth * k_stampHeight, KDColorRed); - Ion::Display::pullRect(KDRect(i*k_stampWidth, 0, k_stampWidth, k_stampHeight), stamp); - for (int k = 0; k < k_stampWidth * k_stampHeight; k++) { - if (stamp[k] != ((k%2 == 0) ? KDColorWhite : KDColorBlack)) { - numberOfInvalidPixels++; - } - } - } - return numberOfInvalidPixels <= k_invalidPixelsLimit; -} - -KDColor colorSequence(bool reset) { - static uint16_t currentColor = 0; - if (reset) { - currentColor = 0; - } - return KDColor::RGB16(currentColor--); -} - -bool POSTAndHardwareTests::TestDisplayMulticolor() { - constexpr int numberOfPixels = k_stampWidth*k_stampHeight; - KDColor stamp[numberOfPixels]; - colorSequence(true); - // Multi-colouring of the display - for (int i = 0; i < Ion::Display::Width / k_stampWidth; i++) { - for (int j = 0; j < Ion::Display::Height / k_stampHeight; j++) { - for (int k = 0; k < numberOfPixels; k++) { - stamp[k] = colorSequence(false); - } - Ion::Display::pushRect(KDRect(i * k_stampWidth, j * k_stampHeight, k_stampWidth, k_stampHeight), stamp); - } - } - // Check the data is ok - colorSequence(true); - int numberOfInvalidPixels = 0; - for (int i = 0; i < Ion::Display::Width / k_stampWidth; i++) { - for (int j = 0; j < Ion::Display::Height / k_stampHeight; j++) { - Ion::Display::pullRect(KDRect(i * k_stampWidth, j * k_stampHeight, k_stampWidth, k_stampHeight), stamp); - for (int k = 0; k < numberOfPixels; k++) { - if (stamp[k] != colorSequence(false)) { - numberOfInvalidPixels++; - if (numberOfInvalidPixels > k_invalidPixelsLimit) { - return false; +bool POSTAndHardwareTests::LCDDataOK(int numberOfIterations) { + for (int iteration = 0; iteration < numberOfIterations; iteration++) { + Ion::Display::POSTPushMulticolor(iteration, k_stampSize); + KDColor stamp[k_stampSize*k_stampSize]; + int numberOfInvalidPixels = 0; + for (int i = 0; i < Ion::Display::Width / k_stampSize; i++) { + for (int j = 0; j < Ion::Display::Height / k_stampSize; j++) { + Ion::Display::pullRect(KDRect(i * k_stampSize, j * k_stampSize, k_stampSize, k_stampSize), stamp); + int shift = (i+j+iteration) % 16; + uint16_t color = (uint16_t)(1 << shift); + for (int k = 0; k < k_stampSize*k_stampSize; k++) { + if (stamp[k] != color) { + numberOfInvalidPixels++; + if (numberOfInvalidPixels > k_invalidPixelsLimit) { + return false; + } } + color ^= 0xFFFF; } } } diff --git a/apps/shared/post_and_hardware_tests.h b/apps/shared/post_and_hardware_tests.h index ba9a7c02a..37875b601 100644 --- a/apps/shared/post_and_hardware_tests.h +++ b/apps/shared/post_and_hardware_tests.h @@ -10,21 +10,13 @@ class POSTAndHardwareTests { public: static bool BatteryOK(); static bool VBlankOK(); - static bool FastLCDDataOK(); - static bool LCDDataOK(); + static bool LCDDataOK(int numberOfIterations); private: - constexpr static int k_stampHeight = 10; - constexpr static int k_stampWidth = 10; + constexpr static int k_stampSize = 8; constexpr static int k_invalidPixelsLimit = 2; - static_assert(Ion::Display::Width % k_stampWidth == 0, "Stamps must tesselate the display"); - static_assert(Ion::Display::Height % k_stampHeight == 0, "Stamps must tesselate the display"); - static_assert(k_stampHeight % 2 == 0 || k_stampWidth % 2 == 0, "Even number of XOR needed."); - static void ColorPixelBuffer(KDColor * pixels, int numberOfPixels, KDColor c); - static bool TestDisplayColorTiling(KDColor c); - static bool TestDisplayColorUniform(KDColor c); - static int NumberOfNonColoredPixels(KDColor wantedColor); - static bool TestDisplayBlackWhite(); - static bool TestDisplayMulticolor(); + static_assert(Ion::Display::Width % k_stampSize == 0, "Stamps must tesselate the display"); + static_assert(Ion::Display::Height % k_stampSize == 0, "Stamps must tesselate the display"); + static_assert(k_stampSize % 2 == 0, "Even number of XOR needed."); }; } diff --git a/ion/include/ion/display.h b/ion/include/ion/display.h index 88fa48c74..5eb383cfd 100644 --- a/ion/include/ion/display.h +++ b/ion/include/ion/display.h @@ -23,14 +23,14 @@ void pullRect(KDRect r, KDColor * pixels); void waitForVBlank(); -// For Power On Self Test -void POSTPushBlackWhite(); - constexpr int Width = 320; constexpr int Height = 240; constexpr int WidthInTenthOfMillimeter = 576; constexpr int HeightInTenthOfMillimeter = 432; +// For Power On Self tests +void POSTPushMulticolor(int shift, int tileSize); + } } diff --git a/ion/src/device/shared/drivers/display.cpp b/ion/src/device/shared/drivers/display.cpp index 72533b84b..0665177b6 100644 --- a/ion/src/device/shared/drivers/display.cpp +++ b/ion/src/device/shared/drivers/display.cpp @@ -55,9 +55,17 @@ void waitForVBlank() { } } -void POSTPushBlackWhite() { - setDrawingArea(KDRect(0,0,Ion::Display::Width, Ion::Display::Height), Orientation::Landscape); - pushBlackWhitePixels(); +void POSTPushMulticolor(int shift, int tileSize) { + const int maxI = Ion::Display::Width / tileSize; + const int maxJ = Ion::Display::Height / tileSize; + for (int i = 0; i < maxI; i++) { + for (int j = 0; j < maxJ; j++) { + uint16_t k = (i+j+shift) % 16; + uint16_t color = 1 << k; + setDrawingArea(KDRect(i*tileSize,j*tileSize,tileSize, tileSize), Orientation::Landscape); + pushColorAndContraryPixels(color, tileSize*tileSize); + } + } } } @@ -391,11 +399,12 @@ void pullPixels(KDColor * pixels, size_t numberOfPixels) { send_command(Command::PixelFormatSet, 0x05); } -void pushBlackWhitePixels() { +void pushColorAndContraryPixels(uint16_t value, int count) { send_command(Command::MemoryWrite); - int numberOfPixels = Ion::Display::Width * Ion::Display::Height; - while (numberOfPixels--) { - send_data(numberOfPixels % 2 == 0 ? KDColorBlack : KDColorWhite); + uint16_t color = value; + while (count-- > 0) { + send_data(color); + color ^= 0xFFFF; } } diff --git a/ion/src/device/shared/drivers/display.h b/ion/src/device/shared/drivers/display.h index 516ad3cac..744e414ba 100644 --- a/ion/src/device/shared/drivers/display.h +++ b/ion/src/device/shared/drivers/display.h @@ -63,7 +63,7 @@ static volatile Command * const CommandAddress = (Command *)(FSMCBankAddress); static volatile uint16_t * const DataAddress = (uint16_t *)(FSMCBankAddress | (1<<(FSMCDataCommandAddressBit+1))); // For Power On Self tests -void pushBlackWhitePixels(); +void pushColorAndContraryPixels(uint16_t color, int count); } } diff --git a/ion/src/shared/dummy/display.cpp b/ion/src/shared/dummy/display.cpp index dde7dd686..c1b14b4ac 100644 --- a/ion/src/shared/dummy/display.cpp +++ b/ion/src/shared/dummy/display.cpp @@ -1,6 +1,6 @@ #include -void Ion::Display::POSTPushBlackWhite() { +void Ion::Display::POSTPushMulticolor(int shift, int tileSize) { } void Ion::Display::waitForVBlank() {