-
-
Notifications
You must be signed in to change notification settings - Fork 19.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Serial1 on Teensy 3.6 not receiving #21277
Comments
Can you copy the build error here? |
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! |
The patch didn't immediately work for me since the new Serial1Class was missing from my bugfix version from last week.
I can confirm though that USB serial (-1) is still working and not broken. |
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)
|
With this patch, there are no longer any compilation errors for both SERIAL_PORT -1 and 1 and acts just like the release version. |
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? |
Did what you suggested and here's the interesting thing, this doesn't work (Serial1 doesn't get transmitted to USBserial):
This DOES work, TX and RX works on both sides...I have absolutely no idea why the bootscreen would affect anything.
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. |
Interesting. |
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. |
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. |
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. |
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. |
I'm confused here. Per this diagram: 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. But as you noted, I'll just use my fix for the time being. |
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). |
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.
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. |
Please check the pins in that file: You'll to follow the LCD you have declared and then check, for example, this repository for your device (open the file called For example, for a ST7920, you'll open u8g_dev_st7920_128x64.c and the code for
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. |
I'm not sure that this is due to the definition of pins in marlinui_DOGM.h
I debugged my way down the rabbithole to find that this specific function is changing the pinmode in u8g_com_arduino_common.c:
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. |
If you search for 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). |
If you don't know what pin and index were used by default, you can change code above to dump it:
Obviously, you'll need to put the serial initialization before the UI initilialization to see the messages on the serial console. |
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.
This told me that specifically
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 In marlin_DOGM.cpp, the first initialization of the u8g dev is at
Normally not an issue if only initialized once but now Technically the code that I put above is another suitable workaround. As is commenting out |
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 If the constructor does not call cbegin, then it should do that instead of calling |
Yup that's right. But at no point is |
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. 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. |
So I was looking into the constructor init and found this comment that mentions this exact issue:
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:
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? |
There are 2 solutions here:
// 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. |
That didn't work for me but this did: |
Can you submit a PR ? |
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. |
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. |
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
Expected behavior:
Actual behavior:
Additional Information
I also tested the following code from: https://www.pjrc.com/teensy/td_uart.html
Everything compiled on PlatformIO.
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.
The text was updated successfully, but these errors were encountered: