Marlin icon indicating copy to clipboard operation
Marlin copied to clipboard

[BUG] DWIN_CREALITY_LCD breaks MultiSerial

Open blazewicz opened this issue 3 years ago • 21 comments

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

Compilation with multiple serial ports while having DWIN_CREALITY_LCD enabled results in a cascade of errors similar to this one:

Marlin/src/HAL/STM32F1/../../inc/../HAL/./STM32F1/../../core/serial_hook.h:225:68: error: request for member 'write' in '((MultiSerial<BaseSerial<MarlinSerial>, int, 0>*)this)->MultiSerial<BaseSerial<MarlinSerial>, int, 0>::serial1', which is of non-class type 'int'

It happens because SERIAL_CATCHALL is set here to an integer:

https://github.com/MarlinFirmware/Marlin/blob/de38cae00cf45ef864e2df180a9d37dc509817a4/Marlin/src/inc/Conditionals_LCD.h#L1079

And here it is expected to be a compatible Serial class instance:

https://github.com/MarlinFirmware/Marlin/blob/de38cae00cf45ef864e2df180a9d37dc509817a4/Marlin/src/core/serial.h#L93

I guess a simple fix like one below would help (it does compile).

--- a/Marlin/src/core/serial.h
+++ b/Marlin/src/core/serial.h
@@ -90,7 +90,7 @@ extern uint8_t marlin_debug_flags;
   #define SERIAL_ASSERT(P)    if(multiSerial.portMask!=(P)){ debugger(); }
   // If we have a catchall, use that directly
   #ifdef SERIAL_CATCHALL
-    #define _SERIAL_LEAF_2 SERIAL_CATCHALL
+    #define _SERIAL_LEAF_2 MSERIAL(SERIAL_CATCHALL)
   #elif HAS_ETHERNET
     typedef ConditionalSerial<decltype(MYSERIAL2)> SerialLeafT2;  // We need to create an instance here
     extern SerialLeafT2 msSerial2

Yet I'm not sure of the function on SERIAL_CATCHALL and how it should be defined properly.

@thinkyhead it seems you added it in https://github.com/MarlinFirmware/Marlin/pull/17719, I can't find any other reference nor documentation.

I think it got broken in https://github.com/MarlinFirmware/Marlin/pull/21306, but I'm not sure if it worked before.

Bug Timeline

No response

Expected behavior

No response

Actual behavior

No response

Steps to Reproduce

No response

Version of Marlin Firmware

bugfix-2.0.x

Printer model

No response

Electronics

No response

Add-ons

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

blazewicz avatar Jul 05 '21 12:07 blazewicz

Please attach your configuration files.

ellensp avatar Jul 05 '21 12:07 ellensp

Any configuration with DWIN LCD and MultiSerial fails. It can be easily reproduced using Configurations/config/examples/Creality/Ender-3 V2 with SERIAL_PORT_2 enabled:

--- a/Marlin/Configuration.h
+++ b/Marlin/Configuration.h
@@ -123,7 +125,7 @@
  * Currently Ethernet (-2) is only supported on Teensy 4.1 boards.
  * :[-2, -1, 0, 1, 2, 3, 4, 5, 6, 7]
  */
-//#define SERIAL_PORT_2 -1
+#define SERIAL_PORT_2 2

blazewicz avatar Jul 05 '21 13:07 blazewicz

Yes I can reproduce the original error.. But your diff to fix it, I cant replicate a compile at all.

ellensp avatar Jul 05 '21 13:07 ellensp

With STM32F103RET6_creality env it fails because it can't find MSerial0 symbol, probably because Serial port 0 is not defined for this platform and SERIAL_CATCHAL is declared to be 0.

Marlin/src/HAL/STM32/../../inc/../HAL/./STM32/HAL.h:50:21: error: 'MSerial0' was not declared in this scope; did you mean 'MSerial10'?

It would compile if you changed:

--- a/Marlin/src/inc/Conditionals_LCD.h
+++ b/Marlin/src/inc/Conditionals_LCD.h
@@ -1076,7 +1076,7 @@
 #endif

 #if ENABLED(DWIN_CREALITY_LCD)
-  #define SERIAL_CATCHALL 0
+  #define SERIAL_CATCHALL 2
   #ifndef LCD_SERIAL_PORT
     #define LCD_SERIAL_PORT 3 // Creality 4.x board
   #endif

It is defined for STM32F103RET6_creality_maple's platform, but I don't know which port it represents.

I would suggest a fix, but I don't know what SERIAL_CATCHALL is.

blazewicz avatar Jul 05 '21 13:07 blazewicz

Bump... with people using the DWIN_CREALITY_LCD on more and more hardware, this is becoming more of an issue

ellensp avatar Jul 24 '21 10:07 ellensp

Bump

env STM32F103RC_btt_maple

"request for member 'write' in '((MultiSerial<ForwardSerial<USBSerial>, int, 0>*)this)->MultiSerial<ForwardSerial<USBSerial>, int, 0>::serial1', which is of non-class type 'int'",
	"startLineNumber": 225,
	"startColumn": 68,
	"endLineNumber": 225,
	"endColumn": 68
}

if i disable serial port 2 it works but i want the second serial for my raspberry pi

Gaiadan avatar Jul 24 '21 13:07 Gaiadan

Let me try to follow the bouncing ball and try to make sense of this.

This concept comes from #21336 by @X-Ryl669 and the value of SERIAL_CATCHALL gets assigned to _SERIAL_LEAF_2 which would otherwise be msSerial2 or MYSERIAL2. The value of _SERIAL_LEAF_2 is used in one of two ways:

// Hook Meatpack if it's enabled on the second leaf
#if ENABLED(MEATPACK_ON_SERIAL_PORT_2)
  typedef MeatpackSerial<decltype(_SERIAL_LEAF_2)> SerialLeafT2;
  extern SerialLeafT2 mpSerial2;
  #define SERIAL_LEAF_2 mpSerial2
#else
  #define SERIAL_LEAF_2 _SERIAL_LEAF_2
#endif

Later, SERIAL_LEAF_2 is used to define the MultiSerial instance.

Should SERIAL_CATCHALL actually be an integer, because it doesn't fit the pattern of other things, or should it be something like MYSERIAL0 or msSerial0 ?

Or, is there some other condition, and the SERIAL_CATCHALL value should be based on the value of SERIAL_PORT or SERIAL_PORT_2, or … ?

It's a lot to review and try to make sense of, but the fix is probably something simple.

thinkyhead avatar Jul 30 '21 03:07 thinkyhead

What is the expected port to use in those printer ? If I understand the intend correctly from #17719, SERIAL_CATCHALL should be a serial port that's overwriting all serial configuration when defined.

I think it can be removed and the proper serial port declared and used instead. So maybe you can just remove the define line from the Conditional file, and define the other serial port you want to use instead, like you did with #define SERIAL_PORT_2 2

@thinkyhead Can you explain what it was used for initially ?

X-Ryl669 avatar Jul 30 '21 05:07 X-Ryl669

Bumping this, as is still an issue.

GhostlyCrowd avatar Sep 09 '21 15:09 GhostlyCrowd

SERIAL_CATCHALL

#if EITHER(HAS_DWIN_E3V2, IS_DWIN_MARLINUI) //#define SERIAL_CATCHALL 0 #ifndef LCD_SERIAL_PORT #if MB(BTT_SKR_MINI_E3_V1_0, BTT_SKR_MINI_E3_V1_2, BTT_SKR_MINI_E3_V2_0, BTT_SKR_E3_TURBO) #define LCD_SERIAL_PORT 1 #else #define LCD_SERIAL_PORT 3 // Creality 4.x board #endif #endif #endif

As you suggested, fixed to compile issue for me, cannot test until i get everything assembled.

GhostlyCrowd avatar Sep 09 '21 15:09 GhostlyCrowd

Is there a fix for this yet ? I cant compile without multiSerial errors when enabling a second Serial Port

SISLANGER avatar Oct 09 '21 01:10 SISLANGER

Just a quick update it seems like PlatformIO wasn't clearing / updating when re-compiling even when using the clean all function. However after removing and re-installing PlatformIO disabling the SERIAL_CATCHALL 0 has done the trick. Main build 02000902)

SISLANGER avatar Oct 10 '21 19:10 SISLANGER

Solved: https://github.com/bigtreetech/BIGTREETECH-SKR-mini-E3/issues/601#issuecomment-1029852372

tiagofreire-pt avatar Mar 11 '22 23:03 tiagofreire-pt

Solved: bigtreetech/BIGTREETECH-SKR-mini-E3#601 (comment)

How does this even remotely solve this? That uses fairly specific options for a specific board and stepper driver setup. Where I am using the TMC2208 drivers, those instructions say to use otherwise. I am also using the BTT SKR 1.4 (Non Turbo), so some of those options also are incorrect for my board. On top of that, I'm using an Ender 3, not a v2.

Disabling the second serial port, and setting the first one to -1 seems to allow me to at least compile the firmware, but it refuses to use the LCD on my printer.

EDIT: Well, I found MY issue out. It was that I had the wrong LCD selected.

Xlaits avatar Jul 10 '22 21:07 Xlaits

Issue still exists

rarens avatar Oct 12 '22 02:10 rarens

Issue still exists as of 3/26/2023 with bugfix build.

samiresa avatar Mar 27 '23 05:03 samiresa

Marlin\src\HAL\shared../../core/serial.h:87:28: error: 'MYSERIAL2' was not declared in this scope

Cuplikan layar 2023-08-14 091144

i am using mks tinybee and since the board is using serial -1 for wifi i always come across this problem

yuuahmad avatar Aug 14 '23 02:08 yuuahmad

Marlin\src\HAL\shared../../core/serial.h:87:28: error: 'MYSERIAL2' was not declared in this scope

Cuplikan layar 2023-08-14 091144

i am using mks tinybee and since the board is using serial -1 for wifi i always come across this problem

In Configuration_adv.h #define ESP3D_WIFISUPPORT and it builds just fine

denis1482 avatar Sep 11 '23 14:09 denis1482

I'm still having the error compiling using bugfix 2.1.x as of 12/9/2023.

I'm using #define MOTHERBOARD BOARD_BTT_SKR_V2_0_REV_B (which is the setting required by BigTree_SKR_2_F429) I've tried compiling with BIGTREE_SKR2_F429 and BIGTREE_SKR2_F429_USB

My error is the same as @yuuahmad:

Marlin\src\HAL\STM32\../../inc/../core/serial.h:87:28: error: 'MYSERIAL2' was not declared in this scope; did you mean 'MYSERIAL1'?

Neither the changes suggested by @denis1482 nor @Xlaits fixes it for me.

My fix was to fall back to 2.1.2.1.

VectorHasting avatar Dec 31 '23 08:12 VectorHasting

This issue is specifically about having #define SERIAL_PORT and #define SERIAL_PORT_2 with any of the UI for Ender-3 v2 OEM display.

If you do not have two serial ports enabled in marlin, you have a different issue ie your #define SERIAL_PORT or LCD_SERIAL_PORT is wrong for your motherboard

ellensp avatar Feb 10 '24 06:02 ellensp

Issue still exists as of 5/12/2024

(Please help us)

BIG-BRO-L avatar May 13 '24 01:05 BIG-BRO-L