[BUG] 2.1.2 Release build fails using Anycubic Chiron example configs

SwiftNick commented 1 year ago

Did you test the latest `bugfix-2.1.x` code?

Yes, and the problem still exists.

Bug Description

Using the example config files for the AnyCubic Chiron, compilation fails due to missing LCD_SERIAL_PORT definition.

The previous definition in configuration.h has been removed 2.1.1 config file There is some refactoring around the DGUS_LCD code which seems to relate to this.

In configuration.h, restoring LCD_SERIAL_PORT 3 or adding HAS_DGUS_LCD 1 to the code block resolves the issue.

#define ANYCUBIC_LCD_CHIRON
#if EITHER(ANYCUBIC_LCD_I3MEGA, ANYCUBIC_LCD_CHIRON)
  #define HAS_DGUS_LCD 1

@thinkyhead not sure which you would prefer as I don't know the extent of your refactoring.

Bug Timeline

Introduced in 2.1.2 release

Expected behavior

Code should build without error

Actual behavior

Compilation fails with sanity check error

Steps to Reproduce

No response

Version of Marlin Firmware

2.1.2

Printer model

Anycubic Chiron

Electronics

stock build

Add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

[X] A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

No response

Nuck-TH commented 1 year ago

Chiron screen with "white" interface is not DGUS, so that define should not be automatically activated.

SwiftNick commented 1 year ago

So the LCD_SERIAL_PORT 3 needs to be reinstated or the ANYCUBIC_LCD_CHIRON needs to be included in the automatic port definitions in https://github.com/MarlinFirmware/Marlin/blob/a7eacbcc49dc74c6e49bae7d82f726c801eaf3fd/Marlin/src/inc/Conditionals_adv.h#L1079

#ifndef LCD_SERIAL_PORT
  #if HAS_DWIN_E3V2 || IS_DWIN_MARLINUI || HAS_DGUS_LCD
    #if MB(BTT_SKR_MINI_E3_V1_0, BTT_SKR_MINI_E3_V1_2, BTT_SKR_MINI_E3_V2_0, BTT_SKR_MINI_E3_V3_0, BTT_SKR_E3_TURBO, BTT_OCTOPUS_V1_1)
      #define LCD_SERIAL_PORT 1
    #elif MB(CREALITY_V24S1_301, CREALITY_V24S1_301F4, CREALITY_V423, MKS_ROBIN)
      #define LCD_SERIAL_PORT 2 // Creality Ender3S1, MKS Robin
    #else
      #define LCD_SERIAL_PORT 3 // Other boards

thinkyhead commented 1 year ago

I should set it in the example configuration, since the Chiron's LCD can be hooked up to any serial port on any board. A default of 3 is possibly the only option when the Trigorilla board is selected, so maybe it makes sense to assign it in the Trigorilla pins file when the Anycubic serial LCD is selected, in prparation, as there is a major refactor of LCD pin assignments on the horizon.

github-actions[bot] commented 1 year ago

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.

SwiftNick commented 1 year ago

Bump just retested using bugfix-2.1.x & configurations bugfix-2.1.x the LCD_SERIAL_PORT definition is still missing.

thisiskeithb commented 1 year ago

I should set it in the example configuration, since the Chiron's LCD can be hooked up to any serial port on any board

Transferring to the Configurations repo…

ellensp commented 1 year ago

to me the thing to do is this, in code...

diff --git a/Marlin/src/inc/Conditionals_LCD.h b/Marlin/src/inc/Conditionals_LCD.h
index 60bc653560..cfb72b6745 100644
--- a/Marlin/src/inc/Conditionals_LCD.h
+++ b/Marlin/src/inc/Conditionals_LCD.h
@@ -477,7 +477,7 @@
 #endif

 // Aliases for LCD features
-#if !DGUS_UI_IS(NONE) || ENABLED(ANYCUBIC_LCD_VYPER)
+#if !DGUS_UI_IS(NONE) || ENABLED(ANYCUBIC_LCD_VYPER) || ENABLED(ANYCUBIC_LCD_CHIRON)
   #define HAS_DGUS_LCD 1
   #if DGUS_UI_IS(ORIGIN, FYSETC, HIPRECY, MKS)
     #define HAS_DGUS_LCD_CLASSIC 1

Then #define LCD_SERIAL_PORT 3 is set

SwiftNick commented 1 year ago

I should set it in the example configuration, since the Chiron's LCD can be hooked up to any serial port on any board

Transferring to the Configurations repo…

I haven't submitted a PR as Scott mention the LCD code was being refactored so I wasn't sure of the best place to fix this.

thisiskeithb commented 1 year ago

Fixed in https://github.com/MarlinFirmware/Configurations/pull/927

thisiskeithb commented 1 year ago

My PR was not merged since this was patched upstream in Marlin: https://github.com/MarlinFirmware/Marlin/commit/a2c0cc0e7b0fcbeddf95995d916ff0a23c6f1c37

I'm sure there will be a follow-up since CI is failing.

SwiftNick commented 9 months ago

2.1.2.2 release also fails to build with the same issue missing LCD_SERIAL_PORT definition.

Bugfix 2.1.x branch builds fine.

ellensp commented 9 months ago

"Marlin 2.1.2.2 is a maintenance release with several patches to improve existing features, with minor adjustments to configuration."

It is not a fully updated marlin, you need to wait for 2.1.3

MarlinFirmware / Configurations