diff --git a/DisplayInterface.h b/DisplayInterface.h index 4ccb5a3..df76819 100644 --- a/DisplayInterface.h +++ b/DisplayInterface.h @@ -19,6 +19,7 @@ #define DISPLAYINTERFACE_H #include "Logger.h" +#include "Screen.h" /// @brief Class to abstract away all physical display implementation to enable multiple display types class DisplayInterface { @@ -29,16 +30,9 @@ class DisplayInterface { /// @brief Clear the entire screen virtual void clearScreen() = 0; - /// @brief Display a row of text on the display - /// @param row Row number as specified in the SCREEN() command (not pixels) - /// @param text Text to be displayed on this row - /// @param underlined (Optional) Flag to underline this row - default false - /// @param column Column number to start displaying at (based on text width, not pixels) - virtual void displayRow(uint8_t row, const char *text, bool underlined = false, uint8_t column = 0) = 0; - - /// @brief Clear the specified row - /// @param row Row number as specified in the SCREEN() command (not pixels) - virtual void clearRow(uint8_t row) = 0; + /// @brief Display the specified Screen on this display + /// @param screen Pointer to the Screen to display + virtual void displayScreen(Screen *screen) = 0; /// @brief Display the startup screen with software version /// @param version EX-Display version diff --git a/DisplayManager.cpp b/DisplayManager.cpp index 9f5d4df..a53ec1e 100644 --- a/DisplayManager.cpp +++ b/DisplayManager.cpp @@ -95,21 +95,9 @@ void DisplayManager::update(ScreenManager *screenManager) { } // Get the current screen for this display Screen *screen = screenManager->getScreenById(screenId); - // If there is one, display the rows + // If there is one, display it if (screen) { - // If this display needs redrawing, clear first then process rows - // Must set a local redraw flag here so we can clear the instance for next time - bool redraw = display->needsRedraw(); - if (redraw) { - display->clearScreen(); - } - for (ScreenRow *row = screen->getFirstScreenRow(); row; row = row->getNext()) { - if (row->needsRedraw() || redraw) { - display->displayRow(row->getId(), row->getText(), false, 0); - } - } - // Now we've redrawn, clear the flag - display->setNeedsRedraw(false); + display->displayScreen(screen); } } } diff --git a/TFT_eSPIDisplay.cpp b/TFT_eSPIDisplay.cpp index d2a3594..e5a755c 100644 --- a/TFT_eSPIDisplay.cpp +++ b/TFT_eSPIDisplay.cpp @@ -76,6 +76,21 @@ void TFT_eSPIDisplay::clearScreen() { _tft->fillScreen(_backgroundColour); } +void TFT_eSPIDisplay::displayScreen(Screen *screen) { + // If this display needs redrawing, clear first then process rows + // Must set a local redraw flag here so we can clear the instance for next time + if (_needsRedraw) { + clearScreen(); + } + for (ScreenRow *row = screen->getFirstScreenRow(); row; row = row->getNext()) { + if (row->needsRedraw() || _needsRedraw) { + displayRow(row->getId(), row->getText(), false, 0); + } + } + // Now we've redrawn, clear the flag + _needsRedraw = false; +} + void TFT_eSPIDisplay::displayRow(uint8_t row, const char *text, bool underlined, uint8_t column) { if (text == nullptr) { return; diff --git a/TFT_eSPIDisplay.h b/TFT_eSPIDisplay.h index f0634a5..c29a129 100644 --- a/TFT_eSPIDisplay.h +++ b/TFT_eSPIDisplay.h @@ -65,16 +65,20 @@ class TFT_eSPIDisplay : public DisplayInterface { /// @brief Clear the entire screen void clearScreen() override; + /// @brief Display the specified Screen on this display + /// @param screen Pointer to the Screen to display + void displayScreen(Screen *screen) override; + /// @brief Display a row of text on the display /// @param row Row number as specified in the SCREEN() command (not pixels) /// @param text Text to be displayed on this row /// @param underlined (Optional) Flag to underline this row - default false /// @param column (Optional) Column to start displaying the text, column being width of a character (not pixels) - void displayRow(uint8_t row, const char *text, bool underlined = false, uint8_t column = 0) override; + void displayRow(uint8_t row, const char *text, bool underlined = false, uint8_t column = 0); /// @brief Clear the specified row /// @param row Row number as specified in the SCREEN() command (not pixels) - void clearRow(uint8_t row) override; + void clearRow(uint8_t row); /// @brief Display the startup screen with software version /// @param version EX-Display version diff --git a/Version.h b/Version.h index abc8355..1caeb18 100644 --- a/Version.h +++ b/Version.h @@ -2,8 +2,11 @@ #define VERSION_H // Numeric version here: major.minor.patch -#define VERSION "0.0.20" +#define VERSION "0.0.21" +// 0.0.21 includes: +// - Refactor DisplayInterface to accept an entire Screen to display to let the derived classes control the entire +// physical display process // 0.0.20 includes: // - Update log levels with LOG_ preface to avoid STM32 framework conflicts for ERROR // 0.0.19 includes: diff --git a/test/integration/test_Controller/test_ControllerDisplayUpdate.cpp b/test/integration/test_Controller/test_ControllerDisplayUpdate.cpp index e0bc563..77437e0 100644 --- a/test/integration/test_Controller/test_ControllerDisplayUpdate.cpp +++ b/test/integration/test_Controller/test_ControllerDisplayUpdate.cpp @@ -62,14 +62,10 @@ TEST_F(ControllerDisplayUpdateTests, OneScreenOneDisplay) { // Ensure the buffer contains it as expected EXPECT_THAT(commandStation.buffer, testing::HasSubstr(screen0row0)); - // We should always get a clearScreen() first when a display needs redrawing - EXPECT_CALL(*display0, clearScreen()).Times(1); - // Set up expectation that our display will have displayRow(uint8_t row, const char *text, bool underlined, uint8_t - // column) called once for each row - EXPECT_CALL(*display0, displayRow(Truly([=](uint8_t row) { return row == 0; }), StrEq("Screen 0 row 0"), - Truly([=](bool underlined) { return underlined == false; }), - Truly([=](uint8_t column) { return column == 0; }))) - .Times(1); + // Expect displayScreen to be called for every character going through the buffer + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 0; + }))).Times(47); for (size_t i = 0; i < strlen(screen0row0); i++) { controller->update(); @@ -86,13 +82,6 @@ TEST_F(ControllerDisplayUpdateTests, OneScreenOneDisplay) { // Ensure the buffer contains it as expected EXPECT_THAT(commandStation.buffer, testing::HasSubstr(screen0row2)); - // Set up expectation that our display will have displayRow(uint8_t row, const char *text, bool underlined, uint8_t - // column) called once for each row - EXPECT_CALL(*display0, displayRow(Truly([=](uint8_t row) { return row == 2; }), StrEq("Screen 0 row 2"), - Truly([=](bool underlined) { return underlined == false; }), - Truly([=](uint8_t column) { return column == 0; }))) - .Times(1); - for (size_t i = 0; i < strlen(screen0row2); i++) { controller->update(); } @@ -103,13 +92,6 @@ TEST_F(ControllerDisplayUpdateTests, OneScreenOneDisplay) { // Ensure the buffer contains it as expected EXPECT_THAT(commandStation.buffer, testing::HasSubstr(screen0row5)); - // Set up expectation that our display will have displayRow(uint8_t row, const char *text, bool underlined, uint8_t - // column) called once for each row - EXPECT_CALL(*display0, displayRow(Truly([=](uint8_t row) { return row == 5; }), StrEq("Screen 0 row 5"), - Truly([=](bool underlined) { return underlined == false; }), - Truly([=](uint8_t column) { return column == 0; }))) - .Times(1); - for (size_t i = 0; i < strlen(screen0row5); i++) { controller->update(); } diff --git a/test/integration/test_Controller/test_ControllerInputAction.cpp b/test/integration/test_Controller/test_ControllerInputAction.cpp index 7ceda40..175ec44 100644 --- a/test/integration/test_Controller/test_ControllerInputAction.cpp +++ b/test/integration/test_Controller/test_ControllerInputAction.cpp @@ -86,32 +86,56 @@ TEST_F(ControllerInputActionTests, SwitchActiveScreen) { .WillOnce(Invoke([this]() { controller->onInputAction(InputAction::PRESS_RIGHT); })) // Second press right .WillOnce(Return()); // Last will have no return with no input - // Everytime a display switches screens, a redraw should trigger clearScreen() - // Therefore we expect this to be called 6 times - 1 to start, then one with each switch - EXPECT_CALL(*display0, clearScreen()).Times(6); - + // First update should mean display screen 0 + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 0; + }))).Times(1); + // Expect a single controller->update() should now ensure display is set to screen 0 (first) controller->update(); EXPECT_EQ(display0->getScreenId(), 0); + // Next update after left press should mean display screen 8 + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 8; + }))).Times(1); + // Now controller->update() should action the left press controller->update(); // Display0's screen ID should now be 8 EXPECT_EQ(display0->getScreenId(), 8); + // Next left press should mean display screen 2 + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 2; + }))).Times(1); + // Now call controller->update() should action the left press controller->update(); // Display0's screen ID should now be 2 EXPECT_EQ(display0->getScreenId(), 2); + // Next left press should mean display screen 0 again + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 0; + }))).Times(1); + // Now call controller->update() should action the left press controller->update(); // Display0's screen ID should now be 0 EXPECT_EQ(display0->getScreenId(), 0); + // Now the right presses should show 2 then 8 + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 2; + }))).Times(1); + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 8; + }))).Times(1); + // Call controller->update() twice controller->update(); controller->update(); @@ -119,6 +143,11 @@ TEST_F(ControllerInputActionTests, SwitchActiveScreen) { // Display0's screen ID should now be 8 EXPECT_EQ(display0->getScreenId(), 8); + // Final update should be showing 8 again + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 8; + }))).Times(1); + controller->update(); // Display0's screen ID should be unchanged at 8 diff --git a/test/integration/test_Display/test_DisplayScreens.cpp b/test/integration/test_Display/test_DisplayScreens.cpp index 84f2a5a..9d7ac52 100644 --- a/test/integration/test_Display/test_DisplayScreens.cpp +++ b/test/integration/test_Display/test_DisplayScreens.cpp @@ -53,8 +53,10 @@ TEST_F(DisplayScreenTests, CreateDisplay) { screenManager->updateScreen(0); EXPECT_EQ(screenManager->getFirstScreen()->getId(), 0); - // First update of a display with a screen should cause a clearScreen() call - EXPECT_CALL(*display0, clearScreen()).Times(1); + // An update should call displayScreen() for the display + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 0; + }))).Times(1); // Call controller->update() and display should still have the first screen ID displayManager->update(screenManager); @@ -82,19 +84,9 @@ TEST_F(DisplayScreenTests, UpdateDisplays) { screenManager->getScreenById(0)->updateScreenRow(0, "Screen 0 row 0"); screenManager->getScreenById(1)->updateScreenRow(0, "Screen 1 row 0"); - // When calling displayManager->update(screenManager), both displays should have update called - EXPECT_CALL(*display0, displayRow(Truly([=](int row) { return row == 0; }), StrEq("Screen 0 row 0"), - Truly([=](bool underline) { return underline == false; }), - Truly([=](int column) { return column == 0; }))) - .Times(1); - EXPECT_CALL(*display1, displayRow(Truly([=](int row) { return row == 0; }), StrEq("Screen 1 row 0"), - Truly([=](bool underline) { return underline == false; }), - Truly([=](int column) { return column == 0; }))) - .Times(1); - - // First update of a display with a screen should cause a clearScreen() call - EXPECT_CALL(*display0, clearScreen()).Times(1); - EXPECT_CALL(*display1, clearScreen()).Times(1); + // When calling displayManager->update(screenManager), both displays should have displayScreen called + EXPECT_CALL(*display0, displayScreen(Truly([=](Screen *screen) { return screen->getId() == 0; }))).Times(1); + EXPECT_CALL(*display1, displayScreen(Truly([=](Screen *screen) { return screen->getId() == 1; }))).Times(1); // Call update displayManager->update(screenManager); diff --git a/test/mocks/MockDisplay.h b/test/mocks/MockDisplay.h index 930fd4c..53a16f9 100644 --- a/test/mocks/MockDisplay.h +++ b/test/mocks/MockDisplay.h @@ -19,6 +19,7 @@ #define MOCKDISPLAY_H #include "DisplayInterface.h" +#include "Screen.h" #include /// @brief Mock physical display class @@ -28,9 +29,7 @@ class MockDisplay : public DisplayInterface { MOCK_METHOD(void, clearScreen, (), (override)); - MOCK_METHOD(void, displayRow, (uint8_t row, const char *text, bool underlined, uint8_t column), (override)); - - MOCK_METHOD(void, clearRow, (uint8_t row), (override)); + MOCK_METHOD(void, displayScreen, (Screen *screen), (override)); MOCK_METHOD(void, displayStartupInfo, (const char *version), (override)); diff --git a/test/mocks/MockSPIDisplay.h b/test/mocks/MockSPIDisplay.h index c18bfe4..08b4ef3 100644 --- a/test/mocks/MockSPIDisplay.h +++ b/test/mocks/MockSPIDisplay.h @@ -34,9 +34,7 @@ class MockSPIDisplay : public DisplayInterface { MOCK_METHOD(void, clearScreen, (), (override)); - MOCK_METHOD(void, displayRow, (uint8_t row, const char *text, bool underlined, uint8_t column), (override)); - - MOCK_METHOD(void, clearRow, (uint8_t row), (override)); + MOCK_METHOD(void, displayScreen, (Screen *screen), (override)); MOCK_METHOD(void, displayStartupInfo, (const char *version), (override)); diff --git a/test/unit/test_Display/test_DisplayInterface.cpp b/test/unit/test_Display/test_DisplayInterface.cpp index 9fb7c02..5887a87 100644 --- a/test/unit/test_Display/test_DisplayInterface.cpp +++ b/test/unit/test_Display/test_DisplayInterface.cpp @@ -15,6 +15,7 @@ * along with this code. If not, see . */ +#include "Screen.h" #include "Version.h" #include "test/mocks/MockDisplay.h" #include @@ -55,26 +56,15 @@ TEST_F(DisplayInterfaceTests, TestBasicMethods) { /// @brief Test DisplayInterface methods that should interact with a physical display TEST_F(DisplayInterfaceTests, TestParameterMethods) { - // Test methods requiring SCREEN() parameters - int expectRow = 2; - const char *expectRowText = "This is text for row 2"; - bool expectUnderline = true; - int expectColumn = 1; + // Create a screen with nothing in it + Screen *screen = new Screen(0); - // Setup the expected displayRow call to validate parameters are received and called only once - EXPECT_CALL(*display, displayRow(Truly([=](int row) { return row == expectRow; }), StrEq(expectRowText), - Truly([=](bool underline) { return underline == expectUnderline; }), - Truly([=](int column) { return column == expectColumn; }))) - .Times(1); + // Setup displayScreen expectation + EXPECT_CALL(*display, displayScreen(Truly([=](Screen *displayScreen) { + return displayScreen->getId() == 0; + }))).Times(1); - // Call the method that should trigger displayRow - display->displayRow(2, "This is text for row 2", true, 1); - - // Setup the expected clearRow call - EXPECT_CALL(*display, clearRow(Truly([=](int row) { return row == expectRow; }))).Times(1); - - // Call it - display->clearRow(2); + display->displayScreen(screen); // Make sure the screen ID is updated display->setScreenId(4); @@ -84,6 +74,9 @@ TEST_F(DisplayInterfaceTests, TestParameterMethods) { // Verify all expectations were made testing::Mock::VerifyAndClearExpectations(display); + + // Clean up + delete screen; } /// @brief Test to ensure the startup info sets the EX-Display version correctly