[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.

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);
 }

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;
 };

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;
 }
 

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;
 };
 
 }

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;
         }
       }
     }

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.");
 };
 
 }

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);
+
 }
 }
 

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;
   }
 }
 

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);
 
 }
 }

ion/src/shared/dummy/display.cpp

@@ -1,6 +1,6 @@
 #include <ion/display.h>
 
-void Ion::Display::POSTPushBlackWhite() {
+void Ion::Display::POSTPushMulticolor(int shift, int tileSize) {
 }
 
 void Ion::Display::waitForVBlank() {