diff --git a/apps/hardware_test/Makefile b/apps/hardware_test/Makefile index 1465a8307..577e59bb5 100644 --- a/apps/hardware_test/Makefile +++ b/apps/hardware_test/Makefile @@ -8,6 +8,7 @@ app_src += $(addprefix apps/hardware_test/,\ keyboard_test_controller.cpp \ keyboard_view.cpp \ lcd_data_test_controller.cpp \ + lcd_timing_test_controller.cpp \ led_test_controller.cpp \ pop_up_controller.cpp \ serial_number_controller.cpp \ diff --git a/apps/hardware_test/app.cpp b/apps/hardware_test/app.cpp index a9f110771..7ffb5c131 100644 --- a/apps/hardware_test/app.cpp +++ b/apps/hardware_test/app.cpp @@ -21,6 +21,7 @@ App::App(Container * container, Snapshot * snapshot) : App::WizardViewController::WizardViewController(Responder * parentResponder) : BankViewController(parentResponder), m_batteryTestController(this), + m_lcdTimingTestController(this), m_colorsLCDTestController(this), m_deadPixelsTestController(this), m_keyboardController(this), @@ -32,12 +33,13 @@ App::WizardViewController::WizardViewController(Responder * parentResponder) : } int App::WizardViewController::numberOfChildren() { - return 8; + return 9; } ViewController * App::WizardViewController::childAtIndex(int i) { ViewController * children[] = { &m_vBlankTestController, + &m_lcdTimingTestController, &m_colorsLCDTestController, &m_lcdDataTestController, &m_deadPixelsTestController, diff --git a/apps/hardware_test/app.h b/apps/hardware_test/app.h index d4ca1b607..3774173e7 100644 --- a/apps/hardware_test/app.h +++ b/apps/hardware_test/app.h @@ -7,6 +7,7 @@ #include "dead_pixels_test_controller.h" #include "keyboard_test_controller.h" #include "lcd_data_test_controller.h" +#include "lcd_timing_test_controller.h" #include "led_test_controller.h" #include "serial_number_controller.h" #include "vblank_test_controller.h" @@ -31,6 +32,7 @@ private: bool handleEvent(Ion::Events::Event event) override; private: BatteryTestController m_batteryTestController; + LCDTimingTestController m_lcdTimingTestController; ColorsLCDTestController m_colorsLCDTestController; DeadPixelsTestController m_deadPixelsTestController; KeyboardTestController m_keyboardController; diff --git a/apps/hardware_test/colors_lcd_test_controller.cpp b/apps/hardware_test/colors_lcd_test_controller.cpp index bf708894d..25a14f535 100644 --- a/apps/hardware_test/colors_lcd_test_controller.cpp +++ b/apps/hardware_test/colors_lcd_test_controller.cpp @@ -17,7 +17,7 @@ bool ColorsLCDTestController::handleEvent(Ion::Events::Event event) { } void ColorsLCDTestController::viewWillAppear() { - bool testOK = Shared::POSTAndHardwareTests::ColorsLCDOK(); + bool testOK = Shared::POSTAndHardwareTests::ColorsLCDPixelFailures() <= k_numberOfAcceptablesGlyphErrors; m_view.setColor(testOK ? KDColorGreen : KDColorRed); m_view.colorsLCDStateTextView()->setText(testOK ? k_colorsLCDOKText : k_colorsLCDFailTest); } diff --git a/apps/hardware_test/colors_lcd_test_controller.h b/apps/hardware_test/colors_lcd_test_controller.h index 02aae289c..91cd15e73 100644 --- a/apps/hardware_test/colors_lcd_test_controller.h +++ b/apps/hardware_test/colors_lcd_test_controller.h @@ -30,6 +30,7 @@ private: } BufferTextView m_colorsLCDStateView; }; + constexpr static int k_numberOfAcceptablesGlyphErrors = 1; constexpr static const char * k_colorsLCDOKText = "COLORS LCD: OK"; constexpr static const char * k_colorsLCDFailTest = "COLORS LCD: FAIL"; ContentView m_view; diff --git a/apps/hardware_test/lcd_data_test_controller.cpp b/apps/hardware_test/lcd_data_test_controller.cpp index 1d5be7c38..d7294f3be 100644 --- a/apps/hardware_test/lcd_data_test_controller.cpp +++ b/apps/hardware_test/lcd_data_test_controller.cpp @@ -1,13 +1,20 @@ #include "lcd_data_test_controller.h" #include #include +#include using namespace Poincare; namespace HardwareTest { +void LCDDataTestController::runTest() { + int pixelFailures = Shared::POSTAndHardwareTests::LCDDataGlyphFailures(); + m_testSuccessful = pixelFailures <= k_errorLimit; + m_view.setStatus(m_testSuccessful, pixelFailures); +} + bool LCDDataTestController::handleEvent(Ion::Events::Event event) { - if (event == Ion::Events::OK && strcmp(m_view.lcdDataStateTextView()->text(), k_lcdDataOKText) == 0) { + if (event == Ion::Events::OK && m_testSuccessful) { // Handled in WizardViewController return false; } @@ -15,24 +22,30 @@ bool LCDDataTestController::handleEvent(Ion::Events::Event event) { } void LCDDataTestController::viewWillAppear() { - bool testOK = Shared::POSTAndHardwareTests::LCDDataOK(); - m_view.lcdDataStateTextView()->setText(testOK ? k_lcdDataOKText : k_lcdDataFailTest); - m_view.setColor(testOK ? KDColorGreen : KDColorRed); + runTest(); } LCDDataTestController::ContentView::ContentView() : SolidColorView(KDColorWhite), - m_lcdDataStateView(KDFont::LargeFont) + m_lcdDataStateView(KDFont::LargeFont), + m_lcdNumberPixelFailuresView(KDFont::SmallFont) { } -void LCDDataTestController::ContentView::setColor(KDColor color) { - SolidColorView::setColor(color); - m_lcdDataStateView.setBackgroundColor(color); +void LCDDataTestController::ContentView::setStatus(bool success, int numberOfErrors) { + KDColor backgroundColor = (success ? KDColorGreen : KDColorRed); + m_lcdDataStateView.setBackgroundColor(backgroundColor); + m_lcdNumberPixelFailuresView.setBackgroundColor(backgroundColor); + m_lcdDataStateView.setText(success ? k_lcdDataPassTest : k_lcdDataFailTest); + constexpr int bufferSize = 20; + char buffer[bufferSize] = {0}; + Poincare::PrintInt::Left(numberOfErrors, buffer, bufferSize); + m_lcdNumberPixelFailuresView.setText(buffer); } void LCDDataTestController::ContentView::layoutSubviews() { m_lcdDataStateView.setFrame(KDRect(0, 0, Ion::Display::Width, Ion::Display::Height)); + m_lcdNumberPixelFailuresView.setFrame(KDRect(10, 10, Ion::Display::Width, 20)); } } diff --git a/apps/hardware_test/lcd_data_test_controller.h b/apps/hardware_test/lcd_data_test_controller.h index eacb69689..48c85ae5b 100644 --- a/apps/hardware_test/lcd_data_test_controller.h +++ b/apps/hardware_test/lcd_data_test_controller.h @@ -10,6 +10,7 @@ class LCDDataTestController : public ViewController { public: LCDDataTestController(Responder * parentResponder) : ViewController(parentResponder), + m_testSuccessful(false), m_view() {} View * view() override { return &m_view; } @@ -19,20 +20,22 @@ private: class ContentView : public SolidColorView { public: ContentView(); - BufferTextView * lcdDataStateTextView() { return &m_lcdDataStateView; } - void setColor(KDColor color) override; + void setStatus(bool success, int numberOfErrors); private: + constexpr static const char * k_lcdDataPassTest = "LCD DATA: OK"; + constexpr static const char * k_lcdDataFailTest = "LCD DATA: FAIL"; void layoutSubviews() override; - int numberOfSubviews() const override { return 1; } + int numberOfSubviews() const override { return 2; } View * subviewAtIndex(int index) override { - assert(index == 0); - return &m_lcdDataStateView; + assert(index >= 0 && index < 2); + return index == 0 ? &m_lcdDataStateView : &m_lcdNumberPixelFailuresView; } BufferTextView m_lcdDataStateView; + BufferTextView m_lcdNumberPixelFailuresView; }; - constexpr static const char * k_lcdDataOKText = "LCD DATA: OK"; - constexpr static const char * k_lcdDataFailTest = "LCD DATA: FAIL"; - + static constexpr int k_errorLimit = 1; + void runTest(); + bool m_testSuccessful; ContentView m_view; }; diff --git a/apps/hardware_test/lcd_timing_test_controller.cpp b/apps/hardware_test/lcd_timing_test_controller.cpp new file mode 100644 index 000000000..4f759a3f9 --- /dev/null +++ b/apps/hardware_test/lcd_timing_test_controller.cpp @@ -0,0 +1,51 @@ +#include "lcd_timing_test_controller.h" +#include +#include +#include + +using namespace Poincare; + +namespace HardwareTest { + +void LCDTimingTestController::runTest() { + int pixelFailures = Shared::POSTAndHardwareTests::LCDTimingGlyphFailures(); + m_testSuccessful = pixelFailures < k_errorLimit; + m_view.setStatus(m_testSuccessful, pixelFailures); +} + +bool LCDTimingTestController::handleEvent(Ion::Events::Event event) { + if (event == Ion::Events::OK && m_testSuccessful) { + // Handled in WizardViewController + return false; + } + return true; +} + +void LCDTimingTestController::viewWillAppear() { + runTest(); +} + +LCDTimingTestController::ContentView::ContentView() : + SolidColorView(KDColorWhite), + m_lcdTimingStateView(KDFont::LargeFont), + m_lcdNumberGlyphFailuresView(KDFont::SmallFont) +{ +} + +void LCDTimingTestController::ContentView::setStatus(bool success, int numberOfErrors) { + KDColor backgroundColor = (success ? KDColorGreen : KDColorRed); + m_lcdTimingStateView.setBackgroundColor(backgroundColor); + m_lcdNumberGlyphFailuresView.setBackgroundColor(backgroundColor); + m_lcdTimingStateView.setText(success ? k_lcdTimingPassTest : k_lcdTimingFailTest); + constexpr int bufferSize = 20; + char buffer[bufferSize] = {0}; + Poincare::PrintInt::Left(numberOfErrors, buffer, bufferSize); + m_lcdNumberGlyphFailuresView.setText(buffer); +} + +void LCDTimingTestController::ContentView::layoutSubviews() { + m_lcdTimingStateView.setFrame(KDRect(0, 0, Ion::Display::Width, Ion::Display::Height)); + m_lcdNumberGlyphFailuresView.setFrame(KDRect(10, 10, Ion::Display::Width, 20)); +} + +} diff --git a/apps/on_boarding/power_on_self_test.cpp b/apps/on_boarding/power_on_self_test.cpp index 921be6b26..f83e30347 100644 --- a/apps/on_boarding/power_on_self_test.cpp +++ b/apps/on_boarding/power_on_self_test.cpp @@ -8,13 +8,8 @@ KDColor PowerOnSelfTest::Perform() { KDColor previousLEDColor = Ion::LED::getColor(); KDColor resultColor = KDColorGreen; - /* Screen tests - * CAUTION: Timing is important. TextLCDTestOK fails only if done at an - * unknown time, which happens to be for VBlankOK being done just before. - * TextLCDTestOK is done many times in a row in the HardwareTest, so if the - * screen passes the POST and fails the Hardware Test, we will need to find - * the right time to sleep here before launching TextLCDTestOK. */ - bool screenTestsOK = Shared::POSTAndHardwareTests::VBlankOK() && Shared::POSTAndHardwareTests::TextLCDTestOK(); + // Screen tests + bool screenTestsOK = Shared::POSTAndHardwareTests::VBlankOK() && (Shared::POSTAndHardwareTests::TextLCDGlyphFailures() <= k_textErrorsLimit); // We push a white screen so that the LCD Data test is invisible for the user. Ion::Display::pushRectUniform(KDRect(0, 0, Ion::Display::Width, Ion::Display::Height), KDColorWhite); Ion::Display::waitForVBlank(); diff --git a/apps/on_boarding/power_on_self_test.h b/apps/on_boarding/power_on_self_test.h index 58d45e699..aa5991a1d 100644 --- a/apps/on_boarding/power_on_self_test.h +++ b/apps/on_boarding/power_on_self_test.h @@ -11,7 +11,7 @@ public: * returns the LED color previous to the tests. */ static KDColor Perform(); private: - constexpr static int k_LCDTestIterationsCount = 5; + constexpr static int k_textErrorsLimit = 0; }; } diff --git a/apps/shared/post_and_hardware_tests.cpp b/apps/shared/post_and_hardware_tests.cpp index 992485b6e..f830ac28b 100644 --- a/apps/shared/post_and_hardware_tests.cpp +++ b/apps/shared/post_and_hardware_tests.cpp @@ -4,6 +4,8 @@ #include #include #include +#include + namespace Shared { @@ -19,66 +21,63 @@ bool POSTAndHardwareTests::VBlankOK() { return result; } -bool POSTAndHardwareTests::TextLCDTestOK() { - const char * text = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +int POSTAndHardwareTests::LCDDataGlyphFailures() { + Ion::Device::Display::initPanel(); + return Ion::Display::displayColoredTilingSize10(); +} + +int POSTAndHardwareTests::LCDTimingGlyphFailures() { + Ion::Device::Display::initPanel(); + int numberOfFailures = 0; + for (int i = 0; i < 500; i++) { + Ion::Display::POSTPushMulticolor(k_stampSize); + KDColor stamp[k_stampSize*k_stampSize]; + for (int i = 0; i < 3; i++) { // TODO LEA 1? + for (int j = 0; j < 3; j++) { + Ion::Display::pullRect(KDRect(i * k_stampSize, j * k_stampSize, k_stampSize, k_stampSize), stamp); + int shift = (i+j) % 16; + uint16_t color = (uint16_t)(1 << shift); + for (int k = 0; k < k_stampSize*k_stampSize; k++) { + if (stamp[k] != color) { + numberOfFailures++; + break; + } + color ^= 0xFFFF; + } + } + } + Ion::Timing::msleep(10); + } + return numberOfFailures; +} + +int POSTAndHardwareTests::ColorsLCDPixelFailures() { + int result = 0; + constexpr KDColor k_colors[] = {KDColorBlack, KDColorRed, KDColorBlue, KDColorGreen, KDColorWhite}; + for (KDColor c : k_colors) { + result += Ion::Display::displayUniformTilingSize10(c); + } + return result; +} + +int POSTAndHardwareTests::TextLCDGlyphFailures() { + const char * text = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const KDFont * font = KDFont::SmallFont; KDCoordinate glyphHeight = font->glyphSize().height(); // Fill the screen + KDIonContext * context = KDIonContext::sharedContext(); + context->setOrigin(KDPointZero); + context->setClippingRect(KDRect(KDPointZero, Ion::Display::Width, Ion::Display::Height)); for (int i = 0; i < Ion::Display::Height / glyphHeight; i++) { - KDIonContext::sharedContext()->drawString(text, KDPoint(0, i * glyphHeight), font); + context->drawString(text, KDPoint(0, i * glyphHeight), font); } // Check the drawing int numberOfFailures = 0; for (int i = 0; i < Ion::Display::Height / glyphHeight; i++) { - numberOfFailures += KDIonContext::sharedContext()->checkDrawnString(text, KDPoint(0, i * glyphHeight), font); - if (numberOfFailures > k_acceptableNumberOfFailures) { - return false; - } + numberOfFailures += context->checkDrawnString(text, KDPoint(0, i * glyphHeight), font); } - return true; -} - -bool POSTAndHardwareTests::LCDDataOK() { - for (int iteration = 0; iteration < k_numberOfLCDIterations; iteration++) { - if (!TextLCDTestOK() || !TilingLCDTestOK()) { - return false; - } - } - return true; -} - -bool POSTAndHardwareTests::ColorsLCDOK() { - constexpr KDColor k_colors[] = {KDColorBlack, KDColorRed, KDColorBlue, KDColorGreen, KDColorWhite}; - for (KDColor c : k_colors) { - if (Ion::Display::displayUniformTilingSize10(c) > k_acceptableNumberOfFailures) { - return false; - } - } - return true; -} - -bool POSTAndHardwareTests::TilingLCDTestOK() { - Ion::Display::POSTPushMulticolor(k_stampSize); - KDColor stamp[k_stampSize*k_stampSize]; - int numberOfFailures = 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) % 16; - uint16_t color = (uint16_t)(1 << shift); - for (int k = 0; k < k_stampSize*k_stampSize; k++) { - if (stamp[k] != color) { - numberOfFailures++; - if (numberOfFailures > k_acceptableNumberOfFailures) { - return false; - } - } - color ^= 0xFFFF; - } - } - } - return true; + return numberOfFailures; } } diff --git a/apps/shared/post_and_hardware_tests.h b/apps/shared/post_and_hardware_tests.h index 4e2dcb9f7..561e1e9a2 100644 --- a/apps/shared/post_and_hardware_tests.h +++ b/apps/shared/post_and_hardware_tests.h @@ -6,29 +6,17 @@ namespace Shared { +// TODO LEA: comment about ColorsLCDPixelFailures and detected screen issues class POSTAndHardwareTests { public: static bool BatteryOK(); static bool VBlankOK(); - static bool TextLCDTestOK(); - /* LCDDataOK carefully tests the LCD controller. It verifies that: - * - Command/data switching is OK, - * - Data is correctly sent, - * - There are no short-circuits between the data wires. - * LCDDataOK thus sends 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, 16 - * colors are used: 2**k with 0 <= k < 16. */ - static bool LCDDataOK(); - static bool ColorsLCDOK(); + static int LCDDataGlyphFailures(); + static int LCDTimingGlyphFailures(); + static int ColorsLCDPixelFailures(); + static int TextLCDGlyphFailures(); private: - constexpr static int k_stampSize = 8; - constexpr static int k_numberOfLCDIterations = 20; - constexpr static int k_acceptableNumberOfFailures = 2; - static bool TilingLCDTestOK(); - 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."); + static constexpr int k_stampSize = 8; }; } diff --git a/ion/include/ion/display.h b/ion/include/ion/display.h index 5869e1c54..af98239a3 100644 --- a/ion/include/ion/display.h +++ b/ion/include/ion/display.h @@ -29,8 +29,9 @@ constexpr int WidthInTenthOfMillimeter = 576; constexpr int HeightInTenthOfMillimeter = 432; // For Power On Self tests +int displayUniformTilingSize10(KDColor c); +int displayColoredTilingSize10(); void POSTPushMulticolor(int tileSize); -int displayUniformTilingSize10(KDColor color); } } diff --git a/ion/src/device/bench/Makefile b/ion/src/device/bench/Makefile index 34beb84bb..096a93182 100644 --- a/ion/src/device/bench/Makefile +++ b/ion/src/device/bench/Makefile @@ -18,6 +18,7 @@ bench_src += $(addprefix ion/src/device/bench/command/, \ keyboard.cpp \ lcd_data.o \ lcd_pins.o \ + lcd_timing.o \ led.cpp \ mcu_serial.cpp \ ping.cpp \ diff --git a/ion/src/device/bench/bench.cpp b/ion/src/device/bench/bench.cpp index e992a730b..05b042278 100644 --- a/ion/src/device/bench/bench.cpp +++ b/ion/src/device/bench/bench.cpp @@ -20,6 +20,7 @@ constexpr CommandHandler handles[] = { CommandHandler("KEYBOARD", Command::Keyboard), CommandHandler("LCD_DATA", Command::LCDData), CommandHandler("LCD_PINS", Command::LCDPins), + CommandHandler("LCD_TIMING", Command::LCDTiming), CommandHandler("LED", Command::LED), CommandHandler("MCU_SERIAL", Command::MCUSerial), CommandHandler("PING", Command::Ping), diff --git a/ion/src/device/bench/command/command.h b/ion/src/device/bench/command/command.h index f00ce6305..7e904501d 100644 --- a/ion/src/device/bench/command/command.h +++ b/ion/src/device/bench/command/command.h @@ -21,6 +21,7 @@ void Jump(const char * input); void Keyboard(const char * input); void LCDData(const char * input); void LCDPins(const char * input); +void LCDTiming(const char * input); void LED(const char * input); void MCUSerial(const char * input); void Ping(const char * input); diff --git a/ion/src/device/bench/command/lcd_data.cpp b/ion/src/device/bench/command/lcd_data.cpp index 32e0fb672..705bf70ac 100644 --- a/ion/src/device/bench/command/lcd_data.cpp +++ b/ion/src/device/bench/command/lcd_data.cpp @@ -1,5 +1,6 @@ #include "command.h" #include +#include namespace Ion { namespace Device { @@ -11,10 +12,16 @@ void LCDData(const char * input) { reply(sSyntaxError); return; } - /* TODO: Find better place for LCDDataOK than + /* TODO: Find better place for LCDDataGlyphFailures than * apps/shared/post_and_hardware_tests. Problem is it needs Kandinsky so Ion * might not be the best place. Maybe move the bench out of Ion? */ - reply(Shared::POSTAndHardwareTests::LCDDataOK() ? sOK : sKO); + constexpr int bufferSize = 6+10+1; // crc is a uint32_t so 10 digits long. + char buffer[bufferSize] = {'D', 'E', 'L', 'T', 'A', '='}; + for (int i = 6; i < bufferSize; i++) { + buffer[i] = 0; + } + Poincare::PrintInt::Left(Shared::POSTAndHardwareTests::LCDDataGlyphFailures(), buffer+4, bufferSize - 4 - 1); + reply(buffer); } } diff --git a/ion/src/device/bench/command/lcd_timing.cpp b/ion/src/device/bench/command/lcd_timing.cpp new file mode 100644 index 000000000..f02bc4b48 --- /dev/null +++ b/ion/src/device/bench/command/lcd_timing.cpp @@ -0,0 +1,30 @@ +#include "command.h" +#include +#include + +namespace Ion { +namespace Device { +namespace Bench { +namespace Command { + +void LCDTiming(const char * input) { + if (input != nullptr) { + reply(sSyntaxError); + return; + } + /* TODO: Find better place for LCDDataGlyphFailures than + * apps/shared/post_and_hardware_tests. Problem is it needs Kandinsky so Ion + * might not be the best place. Maybe move the bench out of Ion? */ + constexpr int bufferSize = 6+10+1; // crc is a uint32_t so 10 digits long. + char buffer[bufferSize] = {'D', 'E', 'L', 'T', 'A', '='}; + for (int i = 6; i < bufferSize; i++) { + buffer[i] = 0; + } + Poincare::PrintInt::Left(Shared::POSTAndHardwareTests::LCDTimingGlyphFailures(), buffer+4, bufferSize - 4 - 1); + reply(buffer); +} + +} +} +} +} diff --git a/ion/src/device/shared/drivers/display.cpp b/ion/src/device/shared/drivers/display.cpp index 630c0b0ef..59e72b054 100644 --- a/ion/src/device/shared/drivers/display.cpp +++ b/ion/src/device/shared/drivers/display.cpp @@ -87,8 +87,8 @@ bool waitForVBlank() { } void POSTPushMulticolor(int tileSize) { - const int maxI = Ion::Display::Width / tileSize; - const int maxJ = Ion::Display::Height / tileSize; + const int maxI = 3; // TODO 1 ? + const int maxJ = 3; for (int i = 0; i < maxI; i++) { for (int j = 0; j < maxJ; j++) { uint16_t k = (i+j) % 16; @@ -104,30 +104,36 @@ int displayUniformTilingSize10(KDColor c) { constexpr int stampWidth = 10; static_assert(Ion::Display::Width % stampWidth == 0, "Stamps must tesselate the display"); static_assert(Ion::Display::Height % stampHeight == 0, "Stamps must tesselate the display"); - static_assert(stampHeight % 2 == 0 || stampWidth % 2 == 0, "Even number of XOR needed."); - KDColor stamp[stampWidth*stampHeight]; - for (int i=0;i