MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.25k stars 19.23k forks source link

[BUG] DWIN_CREALITY_LCD breaks MultiSerial #22299

Open blazewicz opened 3 years ago

blazewicz commented 3 years ago

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

ellensp commented 3 years ago

Please attach your configuration files.

blazewicz commented 3 years ago

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
ellensp commented 3 years ago

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

blazewicz commented 3 years ago

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.

ellensp commented 3 years ago

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

Gaiadan commented 3 years ago

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

thinkyhead commented 3 years ago

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.

X-Ryl669 commented 3 years ago

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 ?

GhostlyCrowd commented 3 years ago

Bumping this, as is still an issue.

GhostlyCrowd commented 3 years ago

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.

SISLANGER commented 3 years ago

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

SISLANGER commented 3 years ago

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)

tiagofreire-pt commented 2 years ago

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

Xlaits commented 2 years ago

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.

rarens commented 2 years ago

Issue still exists

samiresa commented 1 year ago

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

yuuahmad commented 1 year ago

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

denis1482 commented 1 year ago

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

VectorHasting commented 10 months ago

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.

ellensp commented 8 months ago

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

BIG-BRO-L commented 5 months ago

Issue still exists as of 5/12/2024

(Please help us)