[BUG] Serial1 on Teensy 3.6 not receiving

[kgy2002]

Bug Description

I designed a custom board that currently uses a Teensy 3.6 with an ESP3D connected to Serial1. On release 2.0.7.2, everything compiles and uploads without errors but it appears the Serial1 RX on the Teensy specifically doesn't work between the two devices. On power up and KILL button, the ESP3D terminal receives startup messages and the KILL message. However, M105 and M122 returns nothing when commanded. There are no issues with USB serial (SERIAL_PORT -1), with an unchanged baudrate of 500k.

bugfix-2.0.x pulled March 6th works correctly with USB serial, however setting SERIAL_PORT 1 throws a bunch of compilation errors. My guess is that the recent serial refactoring (Refactor serial class with templates #20783) doesn't work well with teensy's HardwareSerial.h and that the HAL probably needs to be updated. Again, no issues with USB serial though.

Configuration Files

config.zip

Attached configs and pin file for custom board. pins.h and boards.h updated to include the pin file.

Steps to Reproduce

1. Compile and upload firmware with SERIAL_PORT 1.
2. Send G-code command that returns anything at all like M105.
3. See if anything is returned in terminal.
4. Press KILL_BUTTON to verify Teensy transmits status back.

Expected behavior:

3. M105 command should return temperature data.

Actual behavior:

3. M105 returns nothing.

Additional Information

I also tested the following code from: https://www.pjrc.com/teensy/td_uart.html
Everything compiled on PlatformIO.

#define HWSERIAL Serial1

void setup() {
	Serial.begin(500000);
	HWSERIAL.begin(500000);
}

void loop() {
  int incomingByte;
        
	if (Serial.available() > 0) {
		incomingByte = Serial.read();
		Serial.print("USB received: ");
		Serial.println(incomingByte, DEC);
                HWSERIAL.print("USB received:");
		HWSERIAL.println(incomingByte, DEC);
	}
	if (HWSERIAL.available() > 0) {
		incomingByte = HWSERIAL.read();
		Serial.print("UART received: ");
		Serial.println(incomingByte, DEC);
		HWSERIAL.print("UART received:");
		HWSERIAL.println(incomingByte, DEC);
	}
}

Everything works fine between the USB serial and the ESP3D serial terminal. Any characters sent on either device is clearly received by the other so this is definitely not a connectivity issue.

The ESP3D is connected on the Teensy 3.6's default Serial1 pins, pin 0 as RX1 and pin 1 as TX1.
My best guess is that the RX1 pin mode is set incorrectly somehow.
I can only confirm this bug for the last release version since the latest bugfix is completely uncompilable for me with SERIAL_PORT set to 1.

[X-Ryl669]

I can only confirm this bug for the last release version since the latest bugfix is completely uncompilable for me with SERIAL_PORT set to 1.

Can you copy the build error here?

[X-Ryl669]

Can you apply the following patch on your bugfix version :

diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.cpp b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
index 547681de5f..ee5bfeacaf 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.cpp
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
@@ -32,6 +32,12 @@
 #include <Wire.h>

 DefaultSerial MSerial(false);
+#define _IMPLEMENT_SERIAL(X) DefaultSerial##X MSerial##X(false)
+#define IMPLEMENT_SERIAL(X)  _IMPLEMENT_SERIAL(X)
+#if WITHIN(SERIAL_PORT, 1, 3)
+  IMPLEMENT_SERIAL(SERIAL_PORT);
+#endif
+
 USBSerialType USBSerial(false, SerialUSB);

 uint16_t HAL_adc_result, HAL_adc_select;
diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.h b/Marlin/src/HAL/TEENSY35_36/HAL.h
index f77b71d91c..645a5a3256 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.h
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.h
@@ -54,18 +54,28 @@
 #endif

 #include "../../core/serial_hook.h"
-typedef Serial1Class<decltype(Serial)> DefaultSerial;
-extern DefaultSerial MSerial;
+
+#define _DECLARE_SERIAL(X) \
+  typedef Serial1Class<decltype(Serial##X)> DefaultSerial##X; \
+  extern DefaultSerial##X MSerial##X;
+#define DECLARE_SERIAL(X) _DECLARE_SERIAL(X)
+
+
 typedef ForwardSerial1Type<decltype(SerialUSB)> USBSerialType;
 extern USBSerialType USBSerial;

 #define _MSERIAL(X) MSerial##X
 #define MSERIAL(X) _MSERIAL(X)
-#define MSerial0 MSerial
+#define Serial0 MSerial

 #if SERIAL_PORT == -1
   #define MYSERIAL1 USBSerial
-#elif WITHIN(SERIAL_PORT, 0, 3)
+#elif SERIAL_PORT == 0
+  typedef Serial1Class<decltype(Serial)> DefaultSerial;
+  extern DefaultSerial MSerial;
+  #define MYSERIAL1 MSerial
+#elif WITHIN(SERIAL_PORT, 1, 3)
+  DECLARE_SERIAL(SERIAL_PORT)
   #define MYSERIAL1 MSERIAL(SERIAL_PORT)
 #endif

Thanks!

[kgy2002]

The patch didn't immediately work for me since the new Serial1Class was missing from my bugfix version from last week.
Just updated my bugfix files to today's version, which seems to largely include this patch.
This is the compilation error now with SERIAL_PORT 1 (significantly better than last weeks version):

Building in release mode
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\HAL.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\HAL_SPI.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\Servo.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\eeprom.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\timers.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\TEENSY35_36\watchdog.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\shared\Delay.cpp.o
Compiling .pio\build\teensy36\src\src\HAL\shared\HAL_MinSerial.cpp.o
In file included from Marlin\src\HAL\TEENSY35_36\HAL.h:56:0,
                 from Marlin\src\HAL\TEENSY35_36\HAL.cpp:29:
Marlin\src\HAL\TEENSY35_36\../../core/serial_hook.h: In instantiation of 'BaseSerial<SerialT>::BaseSerial(bool) [with SerialT = HardwareSerial]':
Marlin\src\HAL\TEENSY35_36\HAL.cpp:37:3:   required from here
Marlin\src\HAL\TEENSY35_36\../../core/serial_hook.h:77:42: error: no matching function for call to 'HardwareSerial::HardwareSerial()'
   BaseSerial(const bool e) : BaseClassT(e) {}
                                          ^
In file included from C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/WProgram.h:46:0,
                 from C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/Arduino.h:6,
                 from Marlin\src\HAL\TEENSY35_36\../shared/Marduino.h:36,
                 from Marlin\src\HAL\TEENSY35_36\HAL.h:30,
                 from Marlin\src\HAL\TEENSY35_36\HAL.cpp:29:
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:256:12: note: candidate: constexpr HardwareSerial::HardwareSerial(void (*)())
  constexpr HardwareSerial(void (* const se)()) : _serialEvent(se) {}
            ^
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:256:12: note:   candidate expects 1 argument, 0 provided
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:253:7: note: candidate: constexpr HardwareSerial::HardwareSerial(const HardwareSerial&)
 class HardwareSerial : public Stream
       ^
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:253:7: note:   candidate expects 1 argument, 0 provided
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:253:7: note: candidate: constexpr HardwareSerial::HardwareSerial(HardwareSerial&&)
C:\Users\kgy2002\.platformio\packages\framework-arduinoteensy\cores\teensy3/HardwareSerial.h:253:7: note:   candidate expects 1 argument, 0 provided
cc1plus.exe: warning: unrecognized command line option '-Wno-register'
*** [.pio\build\teensy36\src\src\HAL\TEENSY35_36\HAL.cpp.o] Error 1

I can confirm though that USB serial (-1) is still working and not broken.

[X-Ryl669]

Can you apply this patch and try again?:

diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.cpp b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
index 8640bdfe00..5d808cd19b 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.cpp
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
@@ -31,7 +31,7 @@

 #include <Wire.h>

-#define _IMPLEMENT_SERIAL(X) DefaultSerial##X MSerial##X(false)
+#define _IMPLEMENT_SERIAL(X) DefaultSerial##X MSerial##X(false, Serial##X)
 #define IMPLEMENT_SERIAL(X)  _IMPLEMENT_SERIAL(X)
 #if WITHIN(SERIAL_PORT, 0, 3)
   IMPLEMENT_SERIAL(SERIAL_PORT);
diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.h b/Marlin/src/HAL/TEENSY35_36/HAL.h
index e769454b3f..5b120d852d 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.h
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.h
@@ -57,7 +57,7 @@

 #define Serial0 Serial
 #define _DECLARE_SERIAL(X) \
-  typedef Serial1Class<decltype(Serial##X)> DefaultSerial##X; \
+  typedef ForwardSerial1Class<decltype(Serial##X)> DefaultSerial##X; \
   extern DefaultSerial##X MSerial##X
 #define DECLARE_SERIAL(X) _DECLARE_SERIAL(X)

[kgy2002]

With this patch, there are no longer any compilation errors for both SERIAL_PORT -1 and 1 and acts just like the release version.
USB serial works like always but Serial1 RX at the Teensy still doesn't work for me, exactly like release and as I described above.

[X-Ryl669]

Can you put your test code from first post in MarlinCore.cpp (in the top of the setup function) as to ensure it's working, then move it down slowly until it breaks, and report?
I've no idea why it does not work, our code fallback to your code (to the Arduino's code), so I suspect that something else is misconfiguring your UART port in the boot.

[kgy2002]

Did what you suggested and here's the interesting thing, this doesn't work (Serial1 doesn't get transmitted to USBserial):

SETUP_RUN(ui.init());
    #if HAS_WIRED_LCD && ENABLED(SHOW_BOOTSCREEN)

      //TEST Serial Ports
      Serial.begin(500000);
      Serial1.begin(500000);

      SETUP_RUN(ui.show_bootscreen());
    #endif
    SETUP_RUN(ui.reset_status());     // Load welcome message early. (Retained if no errors exist.)
#endif

This DOES work, TX and RX works on both sides...I have absolutely no idea why the bootscreen would affect anything.

SETUP_RUN(ui.init());
    #if HAS_WIRED_LCD && ENABLED(SHOW_BOOTSCREEN)
      SETUP_RUN(ui.show_bootscreen());

      //TEST Serial Ports
      Serial.begin(500000);
      Serial1.begin(500000);

    #endif
    SETUP_RUN(ui.reset_status());     // Load welcome message early. (Retained if no errors exist.)
#endif

After seeing this, I disabled SHOW_BOOTSCREEN and moved the serial.begins back to the start of setup to see what would happen but it didn't appear to fix the problem.
However, when I also disabled my LCD display define in the config, everything works properly.
With the default MarlinCore, and no graphical display defined, Serial1 works properly so something with regards to the display is misconfiguring the serial port.

[X-Ryl669]

Interesting.

[kgy2002]

Additional info. With SHOW_BOOTSCREEN disabled and REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER enabled, putting the serial.begins anywhere, even at the end of the setup, doesn't work. Only works with SHOW_BOOTSCREEN enabled like in the previous pasted code with the serial.begins from there, down to the last entry of setup.

[X-Ryl669]

I think you've messed up pin configuration. You'll need to double check the board's electronic schema to ensure you're not mixing pin for 2 features (like LCD display). You might even need to check the technical user manual of the CPU to check what they usually call "pin muxing". That is, sometime, when you enable a CPU peripheral, it takes over a GPIO in another bank and that might be the reason for the serial port not working.

If I were you, I'd start by the pin you've used for the serial's RX and check what are the other possible use on this pin and look for any conflict with the LCD. I know it's a Teensy, but for STM32F1, you can't mux all function on each pins, so extra care must be taken for choosing which GPIO is used. I wonder it's the same for Atmel's CPU.

[kgy2002]

I haven't done anything to change the peripheral type on the pin on my side. As I was noting, it just appears that the UI initialization seems to either cause a hang on the serial or changes the pin config as long as it comes after serial init. Either way, the workaround for me that makes everything work is just to move all of the UI initialization code (from above) before the serial initialization. Display and Serial1 then works without issues with everything enabled.

[X-Ryl669]

I can't change this order for Marlin in general since that'll likely break other user configurations. Serial has to be one of the first things to initialize since some code requires a valid serial link to work.

In your configuration, you've redirected LCD_PIN_D4 to pin 14, but it's also mux-able on TX1. I don't know which pin Marlin is using by default for the serial port, but if it's pin 14, then yes, initializing the display after the serial port will reconfigure the pin to the display peripheral and it's not more available for TX anymore. The fact of moving the serial after the display configuration only fix one problem by hiding another one.

So for now, I think you've identified the reason for the bug (and a potential fix), but I don't think we can do anything on Marlin to fix it. Maybe you should rewire your LCD device on other (compatible) pins that aren't muxed with serial TX/RX pins.

[kgy2002]

I'm confused here. Per this diagram:

And the list of alternate pins for UART listed on the PJRC website: https://www.pjrc.com/teensy/td_uart.html
Pin 14 (PTD1) on the Teensy 3.6 doesn't have UART RX or TX as a selectable peripheral:

Second, the issue is with RX1 functionality. TX1 has generally always worked because I can see startup and KILL_PIN transmits which should happen both before and after the UI initialization normally. So in my case, the pins that share RX1 functionality is 0, 21, and 27 for which I have connected are UART RX, a stepper DIR, and analog in respectively. The TX1 alternate pins are also not shared with any of my LCD pins either based on the pinout diagram.
I find it odd that here would be any issues with pin 0 or 1 since the standard mega also uses both for UART, just RX0 and TX0 instead.

But as you noted, I'll just use my fix for the time being.
If that doesn't work or something else breaks, then I'll try moving display pins around because all the other pins are hardwired.

[X-Ryl669]

You are right, I wasn't checking the right CPU here. I don't know Teensy enough to help you put a watchpoint on the peripheral's pinmux register (and then, locate when it's changed in the code).

[kgy2002]

So I did a bit of debugging basically constantly checking the pinmux setting for pin0 everywhere throughout setup and was able to isolate the issue down to the u8glib library. show_bootscreen eventually calls u8g.firstPage which then calls the cbegin function in the u8glib causing pin0 to be reset to GPIO function from UART. Following cbegin down to u8g_Begin then to u8g_InitLL seems to indicate that the low level init is what's causing the issues.
Commenting out the u8g_InitLL function out of u8g_Begin "fixes" the serial issue but then obviously breaks the display so this isn't an actual usable fix but at least narrows my issue down to this function:

uint8_t u8g_Begin(u8g_t *u8g)
{
  /* call and init low level driver and com device */
  /*if ( u8g_InitLL(u8g, u8g->dev) == 0 )
    return 0;*/
  /* fetch width and height from the low level */
  u8g_UpdateDimension(u8g);
  return 1;
}

I'm personally unable to follow what happens with the low level init after that function but I figure it would good to let you know. As you speculated though, setting serial.begin after the u8g init does fix/reset the pin mux setting back to UART.

[X-Ryl669]

Please check the pins in that file: Marlin/src/lcd/dogm/marlinui_DOGM.h

You'll to follow the LCD you have declared and then check, for example, this repository for your device (open the file called u8g_dev_<your lcd>.cpp. In that file, you'll have to look into what u8g is doing for the MSG_INIT case.

For example, for a ST7920, you'll open u8g_dev_st7920_128x64.c and the code for MSG_INIT is doing this:

u8g_InitCom(u8g, dev);
u8g_WriteEscSeqP(u8g, dev, u8g_dev_st7920_128x64_init_seq);

In this example, it's using the declared SPI pins (CS/MOSI/SCLK) (you've to dig into u8g com module here). If the CS (chip select) pin is wrongly declared, you've found your culprit.
Then walk up the calling chain to the marlinui_DOGM.h and check the pin declaration for the CS. If it's not defined, maybe 0 is used instead (I don't know), so define it correctly and it should work.

[kgy2002]

I'm not sure that this is due to the definition of pins in marlinui_DOGM.h
For my display, only these two lines are defined and all of the LCD PINS are already defined in my PINS file:

    #else
      #define U8G_CLASS U8GLIB_ST7920_128X64_RRD                // Adjust stripes with PAGE_HEIGHT in ultralcd_st7920_u8glib_rrd.h
    #endif
    #define U8G_PARAM LCD_PINS_D4, LCD_PINS_ENABLE, LCD_PINS_RS // AVR version ignores these pin settings
                                                                // HAL version uses these pin settings

I debugged my way down the rabbithole to find that this specific function is changing the pinmode in u8g_com_arduino_common.c:

/* this procedure does not set the RW pin */
void u8g_com_arduino_assign_pin_output_high(u8g_t *u8g)
{
  uint8_t i;
  /* skip the RW pin, which is the last pin in the list */
  for( i = 0; i < U8G_PIN_LIST_LEN-1; i++ )
  {
    if ( u8g->pin_list[i] != U8G_PIN_NONE )
    {
      pinMode(u8g->pin_list[i], OUTPUT);
      digitalWrite(u8g->pin_list[i], HIGH);
    }
  }
}

Commenting out this function in U8G_COM_MSG_INIT in u8g_com_arduino_st7920_spi.c in the library U8glib-HAL prevents the pinmode from changing but then also breaks the display again because the display SPI signals are probably not set as outputs. The pin_list array must have zero value elements because its not obvious to me where the unused elements are written as U8G_PIN_NONE to prevent pin 0 from being set to output.

[X-Ryl669]

If you search for pin_list in Marlin's codebase, there are plenty of matches for AVR, DUE, LPC but none for Teensy. So maybe you should copy the file from LPC version (for example: Marlin/src/HAL/LPC1768/u8g/u8g_com_HAL_LPC1768_st7920_sw_spi.cpp to Marlin/src/HAL/TEENSY35_36/u8g/u8g_com_HAL_Teensy35_36_st7920_sw_spi.cpp) and start fixing in there. This implies creating a new device for u8g to use and registering it.

Another solution would be apply this patch:

diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.cpp b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
index 5d808cd19b..bf3c3d6cfe 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.cpp
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.cpp
@@ -29,6 +29,10 @@
 #include "HAL.h"
 #include "../shared/Delay.h"

+#if HAS_MARLINUI_U8GLIB
+  #include <U8glib.h>
+#endif
+
 #include <Wire.h>

 #define _IMPLEMENT_SERIAL(X) DefaultSerial##X MSerial##X(false, Serial##X)
@@ -65,6 +69,16 @@ static const uint8_t pin2sc1a[] = {
   void sei() { interrupts(); }
 */

+void HAL_init() {
+  #if HAS_MARLINUI_U8GLIB
+    // TODO Replace by your pins setup, you need to find out what are the index used in your device, likely CS / MOSI and SCLK / and maybe RS
+    u8g->pin_list[index] = pin;
+    u8g->pin_list[index] = pin;
+    u8g->pin_list[index] = pin;
+  #endif
+}
+
+
 void HAL_adc_init() {
   analog_init();
   while (ADC0_SC3 & ADC_SC3_CAL) {}; // Wait for calibration to finish
diff --git a/Marlin/src/HAL/TEENSY35_36/HAL.h b/Marlin/src/HAL/TEENSY35_36/HAL.h
index 5b120d852d..c2dfc57854 100644
--- a/Marlin/src/HAL/TEENSY35_36/HAL.h
+++ b/Marlin/src/HAL/TEENSY35_36/HAL.h
@@ -91,7 +91,7 @@ typedef int8_t pin_t;
 #undef sq
 #define sq(x) ((x)*(x))

-inline void HAL_init() {}
+void HAL_init();

 // Clear reset reason
 void HAL_clear_reset_source();

But tweak the index and value your setup (if possible using the LCD_XX macros for the value).

[X-Ryl669]

If you don't know what pin and index were used by default, you can change code above to dump it:

void HAL_init() { 
SERIAL_ECHOPAIR("U8G has ", U8G_PIN_LIST_LEN, "pins declared");
for (int i = 0; i < U8G_PIN_LIST_LEN; i++)
   SERIAL_ECHOPAIR("Pin ", i, " points to pin: ", u8g->pin_list[i]);
delay(1000);
}

Obviously, you'll need to put the serial initialization before the UI initilialization to see the messages on the serial console.

[kgy2002]

The above code doesn't compile for me, I think the include is wrong, but either way I don't think it will fix the root cause.
I took a little time to figure out what exactly is happening and I think I figured it out. I added these 2 lines to the function in my previous post and everything works perfectly:

/* this procedure does not set the RW pin */
void u8g_com_arduino_assign_pin_output_high(u8g_t *u8g)
{
  uint8_t i;
  /* skip the RW pin, which is the last pin in the list */
  for( i = 0; i < U8G_PIN_LIST_LEN-1; i++ )
  {
    if ( u8g->pin_list[i] != U8G_PIN_NONE )
    {
      if ( i == 3 )                            // THIS!!
      u8g->pin_list[i] = U8G_PIN_NONE;                 // AND THIS!!!
      pinMode(u8g->pin_list[i], OUTPUT);
      digitalWrite(u8g->pin_list[i], HIGH);
    }
  }
}

This told me that specifically u8g->pin_list[U8G_PI_A0_STATE] was the cause of the issue. Initially I had no idea where that pin state was being written to 0 but it was pretty obvious that any calls to u8g_SetAddress(u8g, dev, 0); used all over the place would set the pin to 0 from 255 (U8G_PIN_NONE).

    case U8G_COM_MSG_ADDRESS:                     /* define cmd (arg_val = 0) or data mode (arg_val = 1) */
      u8g->pin_list[U8G_PI_A0_STATE] = arg_val;
      break;

Not an issue usually if the set to 0 after initialization since the pinMode is supposed to only be written once. So I dug a little deeper and discovered that the pins are initialized twice due to 2 uses of u8g_Begin().

In marlin_DOGM.cpp, the first initialization of the u8g dev is at U8G_CLASS u8g(U8G_PARAM); early in the setup. This calls u8g_InitSPI which sets all of the pin list values to 255 usingu8g_init_data(u8g);before setting the SPI pins to the defined LCD pins from my file. When u8g_Begin() is called in the function, this should be the case: u8g->pin_list[U8G_PI_A0_STATE] = U8G_PIN_NONE. However you can see what happens towards the end of this function here:

case U8G_COM_MSG_INIT:
      u8g_com_arduino_assign_pin_output_high(u8g);
      u8g_com_arduino_digital_write(u8g, U8G_PI_CS, LOW);
      // u8g_com_arduino_digital_write(u8g, U8G_PI_SCK, LOW);
      u8g_com_arduino_digital_write(u8g, U8G_PI_SCK, HIGH);
      u8g_com_arduino_digital_write(u8g, U8G_PI_MOSI, LOW);
      u8g_com_arduino_init_shift_out(u8g->pin_list[U8G_PI_MOSI], u8g->pin_list[U8G_PI_SCK]);
      u8g->pin_list[U8G_PI_A0_STATE] = 0;       /* inital RS state: command mode */ <--As well as subsequent SetAddress functions 
      break;

Normally not an issue if only initialized once but now u8g->pin_list[U8G_PI_A0_STATE] = 0. After the first initialization further down in the setup is u8g.firstPage(); . This function eventually runs u8g_Begin() again but without the u8g_init_data(u8g); therefore leaving the pin at 0 instead of 255 and when the pinMode run again, this changes pin 0 to OUTPUT from its UART mode.

Technically the code that I put above is another suitable workaround. As is commenting out cbegin() from void firstPage(void) or u8g_Begin(&u8g); from void cbegin(void) but I feel like this is a setup issue for the display. Let me know if you have another suitable fix that doesn't change the library files. Thanks.

[X-Ryl669]

This function eventually runs u8g_Begin() again but without the u8g_init_data(u8g);

This is the source of the issue. A dual initialization is a wrong (and useless) code here. If I understand u8g correctly, in u8g.firstPage(), it's calling cbegin. In cbegin, there is void cbegin(void) { if ( is_begin == 0 ) { is_begin = 1; u8g_Begin(&u8g); } }
So it has a flag that's toggled when it's started and it's not initialized again if it's true.

If the constructor does not call cbegin, then it should do that instead of calling u8g_Begin directly.

[kgy2002]

Yup that's right. But at no point is is_begin ==1 before cbegin basically triggering a second call of u8g_Begin.

[X-Ryl669]

What I mean is: if the U8G_CLASS's constructor is in Marlin's code, we can change it to call cbegin, instead of u8g_Begin.
If it's in u8g arduino library, then it's a bug with u8g and it should be reported there.

From what I've read, it's the latter, but I might be wrong, there're so many case in Marlin for constructing u8g.

The only thing you could do in Marlin is to work around the issue, either by toggling the is_begin value in the init code manually, or by resetting the pin in the pin list like I suggested above. But both cases are temporary fix, if the issue is in u8g.

[kgy2002]

So I was looking into the constructor init and found this comment that mentions this exact issue:

uint8_t u8g_InitSPI(u8g_t *u8g, u8g_dev_t *dev, uint8_t sck, uint8_t mosi, uint8_t cs, uint8_t a0, uint8_t reset)
{

  /* fill data structure with some suitable values */
  u8g_init_data(u8g);
  u8g->dev = dev;

  /* assign user pins */
  u8g->pin_list[U8G_PI_SCK] = sck;
  u8g->pin_list[U8G_PI_MOSI] = mosi;
  u8g->pin_list[U8G_PI_CS] = cs;
  u8g->pin_list[U8G_PI_A0] = a0;
  u8g->pin_list[U8G_PI_RESET] = reset;

  /* On the Arduino Environment this will lead to two calls to u8g_Begin(), the following line will be called first (by U8glib constructors) */
  /* if - in future releases - this is removed, then still call u8g_UpdateDimension() */
  /* if Arduino call u8g_UpdateDimension else u8g_Begin */
  /* issue 146 */
  return u8g_Begin(u8g);
}

Replaced the u8g_Begin with this and everything works as well since the firstPage call later on uses the u8g_init_data values minus the user assigned pins:

  //return u8g_Begin(u8g);
  u8g_UpdateDimension(u8g);
  return 1;

So I guess u8glib is the issue here but since it doesn't appear to be in development anymore, are we only stuck with temp fixes?

[X-Ryl669]

There are 2 solutions here:

1. Make a patch for the u8glib and we add a new step to patch the library whenever required in Marlin
2. Workaround the issue by adding:

// There is a bug in u8glib on Arduino that's setting this pin to 0 upon initialization and using this pin to update the pin direction. 
// Pin 0 is valid and used for serial port on Teensy. Upon first call to u8g.firstPage(), u8g will break the pin configuration when it's set to 0
// Since A0 is not used on Teensy by u8g, the call to firstPage() will again reset U8G's pin A0 to 0, but it has no impact.
// On the other architectures, where A0 is used, this will still lead to correct behavior    
u8g->pin_list[U8G_PI_A0] = U8G_PIN_NONE;

again in Marlin in the initialization code, just before calling firstPage.

[kgy2002]

That didn't work for me but this did:
u8g.getU8g()->pin_list[U8G_PI_A0_STATE] = U8G_PIN_NONE;
I think because u8g is a private in the class. Also should be U8G_PI_A0_STATE for pin index 3.

[X-Ryl669]

Can you submit a PR ?

[github-actions]

This issue has had no activity in the last 60 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 10 days.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.