Merge pull request #20 from DCC-EX:multi-tft_espi

Updated multi display logic

DisplayManager.cpp

@@ -48,9 +48,10 @@ void DisplayManager::startDisplays() {
     return;
   }
   for (auto *display = _firstDisplay; display; display = display->getNext()) {
+    // If display's CS pin is defined, set the pin mode and select the display
+    _setSPIDisplayCSPin(display);
     _selectSPIDisplay(display);
     display->begin();
-    _deselectSPIDisplay(display);
   }
 }
 
@@ -64,8 +65,6 @@ void DisplayManager::displayStartupInfo(const char *version) {
     // Select this SPI display if necessary
     _selectSPIDisplay(display);
     display->displayStartupInfo(version);
-    // Deselect this SPI display if necessary
-    _deselectSPIDisplay(display);
   }
 }
 
@@ -102,8 +101,6 @@ void DisplayManager::update(ScreenManager *screenManager) {
       // Now we've redrawn, clear the flag
       display->setNeedsRedraw(false);
     }
-    // Deselect this SPI display if necessary
-    _deselectSPIDisplay(display);
   }
 }
 
@@ -134,20 +131,29 @@ DisplayManager::~DisplayManager() {
   _logger = nullptr;
 }
 
-void DisplayManager::_selectSPIDisplay(DisplayInterface *display) {
+void DisplayManager::_setSPIDisplayCSPin(DisplayInterface *display) {
   // If not set, don't do anything
   if (display->getCSPin() == -1) {
     return;
   }
-  // Otherwise set CS pin low to select this screen
-  digitalWrite(display->getCSPin(), LOW);
+  // Set the pin to output mode
+  pinMode(display->getCSPin(), OUTPUT);
 }
 
-void DisplayManager::_deselectSPIDisplay(DisplayInterface *display) {
+void DisplayManager::_selectSPIDisplay(DisplayInterface *display) {
   // If not set, don't do anything
-  if (display->getCSPin() == -1) {
+  int csPin = display->getCSPin();
+  if (csPin == -1) {
     return;
   }
-  // Otherwise set CS pin high to deselect this screen
-  digitalWrite(display->getCSPin(), HIGH);
+  // First every other display needs to be disabled
+  for (DisplayInterface *other = _firstDisplay; other; other = other->getNext()) {
+    if (other != display) {
+      if (other->getCSPin() != -1) {
+        digitalWrite(other->getCSPin(), HIGH);
+      }
+    }
+  }
+  // Otherwise set CS pin low to select this screen
+  digitalWrite(csPin, LOW);
 }

DisplayManager.h

@@ -67,13 +67,13 @@ class DisplayManager {
   Logger *_logger;
   uint8_t _nextDisplayId;
 
-  /// @brief Manually selects SPI displays that require manual selecting via the CS pin - sets pin low
+  /// @brief Set the chip select pin state for any display that has it set
   /// @param display Pointer to the DisplayInterface instance
-  void _selectSPIDisplay(DisplayInterface *display);
+  void _setSPIDisplayCSPin(DisplayInterface *display);
 
-  /// @brief Manually deselects SPI displays that require manual selecting via the CS pin - sets pin high
+  /// @brief Manually selects SPI displays that require manual selecting via the CS pin - sets pin low
   /// @param display Pointer to the DisplayInterface instance
-  void _deselectSPIDisplay(DisplayInterface *display);
+  void _selectSPIDisplay(DisplayInterface *display);
 };
 
 #endif // DISPLAYMANAGER_H

TFT_eSPIDisplay.cpp

@@ -56,9 +56,11 @@ void TFT_eSPIDisplay::begin() {
   _maxColumn = _tft->width() / _fontWidth;
   LOG(LogLevel::DEBUG,
       "TFT_eSPIDisplay[%d] settings: "
-      "_textSize=%d|_rotation=%d|_textColour=0x%04X|_backgroundColour=0x%04X|_fontHeight=%d|_fontWidth=%d|_maxRow=%d|_"
+      "_textSize=%d|_rotation=%d|_textColour=0x%04X|_backgroundColour=0x%04X|_csPin=%d|_fontHeight=%d|_fontWidth=%d|_"
+      "maxRow=%d|_"
       "maxColumn=%d",
-      _displayId, _textSize, _rotation, _textColour, _backgroundColour, _fontHeight, _fontWidth, _maxRow, _maxColumn);
+      _displayId, _textSize, _rotation, _textColour, _backgroundColour, _csPin, _fontHeight, _fontWidth, _maxRow,
+      _maxColumn);
   _tft->fillScreen(_backgroundColour);
 }
 

Version.h

@@ -2,8 +2,10 @@
 #define VERSION_H
 
 // Numeric version here: major.minor.patch
-#define VERSION "0.0.16"
+#define VERSION "0.0.17"
 
+// 0.0.17 includes:
+//  - Improved experimental support for multiple TFT_eSPI instances
 // 0.0.16 includes:
 //  - Add configuration of input device via macro USER_INPUT()
 // 0.0.15 includes:

platformio.ini

@@ -78,10 +78,10 @@ build_flags =
 	-DTFT_MISO=23
 	-DTFT_MOSI=19
 	-DTFT_SCLK=18
-	-DTFT_CS=15
+	-DTFT_CS=25
 	-DTFT_DC=2
 	-DTFT_RST=4
-	-DTOUCH_CS=22
+	-DTOUCH_CS=32
 	-DLOAD_GLCD=1
 	-DLOAD_FONT2=1
 	-DLOAD_FONT4=1
@@ -113,7 +113,7 @@ build_flags =
 	; -DTFT_CS=-1
 	-DTFT_DC=2
 	-DTFT_RST=4
-	-DTOUCH_CS=22
+	-DTOUCH_CS=32
 	-DLOAD_GLCD=1
 	-DLOAD_FONT2=1
 	-DLOAD_FONT4=1

test/integration/test_CompilerCreation/test_CompilerCreation.cpp

@@ -68,6 +68,16 @@ TEST_F(CompilerCreationTests, CreateDevices) {
   EXPECT_CALL(*display1, begin()).Times(1);
   EXPECT_CALL(*display2, begin()).Times(1);
 
+  // MockSPIDisplay instances should cause mockPinMode and mockDigitalWrite calls
+  // display1
+  EXPECT_CALL(MockArduino::getInstance(), mockPinMode(22, OUTPUT)).Times(1);
+  EXPECT_CALL(MockArduino::getInstance(), mockDigitalWrite(22, LOW)).Times(1);
+  EXPECT_CALL(MockArduino::getInstance(), mockDigitalWrite(22, HIGH)).Times(1);
+  // display2
+  EXPECT_CALL(MockArduino::getInstance(), mockPinMode(23, OUTPUT)).Times(1);
+  EXPECT_CALL(MockArduino::getInstance(), mockDigitalWrite(23, LOW)).Times(1);
+  EXPECT_CALL(MockArduino::getInstance(), mockDigitalWrite(23, HIGH)).Times(1);
+
   displayManager->startDisplays();
 
   // Now create the input

test/integration/test_Controller/test_ControllerDisplayUpdate.cpp

@@ -62,6 +62,8 @@ 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"),

test/integration/test_Controller/test_ControllerInputAction.cpp

@@ -86,6 +86,10 @@ 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);
+
   // Expect a single controller->update() should now ensure display is set to screen 0 (first)
   controller->update();
   EXPECT_EQ(display0->getScreenId(), 0);

test/integration/test_Display/test_DisplayScreens.cpp

@@ -53,6 +53,9 @@ 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);
+
   // Call controller->update() and display should still have the first screen ID
   displayManager->update(screenManager);
   EXPECT_EQ(display0->getScreenId(), 0);
@@ -89,6 +92,10 @@ TEST_F(DisplayScreenTests, UpdateDisplays) {
                                     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);
+
   // Call update
   displayManager->update(screenManager);