search

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.24k stars 19.23k forks source link

LCM1602 fails to compile with argument mismatch #10764

Closed comps closed 6 years ago

comps commented 6 years ago

Description

Running default bugfix-2.0.x config b9079aa, with only these changed:

diff --git a/Marlin/Configuration.h b/Marlin/Configuration.h
index e508b38db..ec759c024 100644
--- a/Marlin/Configuration.h
+++ b/Marlin/Configuration.h
@@ -1425,7 +1425,7 @@
  * Disable all menus and only display the Status Screen, or
  * just remove some extraneous menu items to recover space.
  */
-//#define NO_LCD_MENUS
+#define NO_LCD_MENUS
 //#define SLIM_LCD_MENUS

 //
@@ -1558,7 +1558,9 @@
 //
 // Generic 16x2, 16x4, 20x2, or 20x4 character-based LCD.
 //
-//#define ULTRA_LCD
+#define ULTRA_LCD
+#define LCD_WIDTH 20
+#define LCD_HEIGHT 4

 //=============================================================================
 //======================== LCD / Controller Selection =========================
@@ -1590,7 +1592,7 @@
 //
 // Generic LCM1602 LCD adapter
 //
-//#define LCM1602
+#define LCM1602

 //
 // PANELOLU2 LCD with status LEDs,

the compilation via platformio (pio run) fails on

Compiling .pioenvs/megaatmega2560/src/src/lcd/lcdprint_hd44780.cpp.o
In file included from Marlin/src/lcd/lcdprint_hd44780.cpp:28:0:
Marlin/src/lcd/ultralcd_common_HD44780.h:177:19: fatal error: LCD.h: No such file or directory

Looking around, it seems that I'm missing the LiquidCrystal library, but there are a few:

$ pio lib search LiquidCrystal
Found 8 libraries:

LiquidCrystal
=============
#ID: 136
LiquidCrystal Library is faster and extensable, compatible with the original LiquidCrystal library

Keywords: lcd, hd44780
Compatible frameworks: Arduino
Compatible platforms: Atmel AVR, Espressif 8266
Authors: F Malpartida

Adafruit LiquidCrystal
======================
#ID: 855
Fork of LiquidCrystal HD44780-compatible LCD driver library, now with support for ATtiny85.

Keywords: display
Compatible frameworks: Arduino
Compatible platforms: Atmel AVR, Atmel SAM, Espressif 8266, Intel ARC32, Microchip PIC32, Nordic nRF51, Teensy, TI MSP430
Authors: Adafruit

LiquidCrystal
=============
#ID: 887
Allows communication with alphanumerical liquid crystal displays (LCDs).

Keywords: display
Compatible frameworks: Arduino
Compatible platforms: Atmel AVR, Atmel SAM, Espressif 8266, Intel ARC32, Microchip PIC32, Nordic nRF51, Teensy, TI MSP430
Authors: Arduino, Adafruit

<snip>

LiquidCrystal_I2C
=================
#ID: 1574
A library for DFRobot I2C LCD displays with port parameters

Keywords: lcd, liquidcrystal, i2c
Compatible frameworks: Arduino
Compatible platforms: Atmel AVR
Authors: Tony Kambourakis

LiquidCrystal_I2C is already installed by Marlin's dependencies.

Trying out #136 makes lcdprint_hd44780.cpp compile, but it stops on

Compiling .pioenvs/megaatmega2560/src/src/lcd/ultralcd.cpp.o
In file included from Marlin/src/lcd/ultralcd.cpp:110:0:
Marlin/src/lcd/ultralcd_impl_HD44780.h:65:55: error: no matching function for call to 'LiquidCrystal_I2C::LiquidCrystal_I2C(int, int, int, int, int, int, int, int, int, t_backlighPol)'
LCD_CLASS lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);
^
Marlin/src/lcd/ultralcd_impl_HD44780.h:65:55: note: candidates are:
In file included from Marlin/src/lcd/ultralcd_common_HD44780.h:178:0,
from Marlin/src/lcd/ultralcd_impl_HD44780.h:31,
from Marlin/src/lcd/ultralcd.cpp:110:
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:57:3: note: LiquidCrystal_I2C::LiquidCrystal_I2C(uint8_t, uint8_t, uint8_t)
LiquidCrystal_I2C(uint8_t lcd_Addr,uint8_t lcd_cols,uint8_t lcd_rows);
^
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:57:3: note:   candidate expects 3 arguments, 10 provided
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:55:7: note: constexpr LiquidCrystal_I2C::LiquidCrystal_I2C(const LiquidCrystal_I2C&)
class LiquidCrystal_I2C : public Print {
^
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:55:7: note:   candidate expects 1 argument, 10 provided
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:55:7: note: constexpr LiquidCrystal_I2C::LiquidCrystal_I2C(LiquidCrystal_I2C&&)
.piolibdeps/LiquidCrystal_I2C_ID576/LiquidCrystal_I2C.h:55:7: note:   candidate expects 1 argument, 10 provided

The other libraries don't seem to define LCD.h.

Just to make sure,

$ pio lib install LiquidCrystal_I2C
Library Storage: /home/user/gitit/Marlin/.piolibdeps
LibraryManager: Installing id=576
LiquidCrystal_I2C @ 1.1.2 is already installed

Steps to Reproduce

  1. apply the diff to Configuration.h
  2. pio run

Expected behavior: compilation succeeds Actual behavior: compilation aborts

Additional Information

Since I'm running on a Sanguino-based board with very limited pin count, I cannot afford an encoder or SD, but have SDA/SCL free for a simple i2c status screen, which is what I'm trying to get working.

Also, my platformio setup is working just fine without any LCDs.

comps commented 6 years ago

Looking at the examples for LiquidCrystal_I2C, this makes more sense:

diff --git a/Marlin/src/lcd/ultralcd_impl_HD44780.h b/Marlin/src/lcd/ultralcd_impl_HD44780.h
index 7511d97c6..f1cab7021 100644
--- a/Marlin/src/lcd/ultralcd_impl_HD44780.h
+++ b/Marlin/src/lcd/ultralcd_impl_HD44780.h
@@ -62,7 +62,7 @@
   #endif

 #elif ENABLED(LCM1602)
-  LCD_CLASS lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);
+  LCD_CLASS lcd(0x27, LCD_WIDTH, LCD_HEIGHT);

 #else
   // Standard directly connected LCD implementations

and if compiles just fine.

I'll order the LCD and test this when it arrives. If it indeed works, I'll clean the code up a bit (ie. move the address to Conditionals_LCD.h) and maybe wait for #10584 to land and send this as PR.

AletheianAlex commented 6 years ago

I am having a similar issue getting i2c LCD with 8574t expander running on stm32, with the same and similar Configuration.h settings. But I got a compile by forcing include of LiquidCrystal@1.3.4 in lib_deps , and forcing exclude of LiquidCrystal_I2C in lib_ignore. It 'builds'... now to get it to work

thinkyhead commented 6 years ago

Yes, I noticed that while looking at other issues today. I don't know how it ended up that way. The code appears to be trying to use a PCF8575 interface. We'd love to have a fix.

comps commented 6 years ago

My patch above doesn't seem to work; the firmware seems to boot fine (shows M503 output), but then fails to respond to anything (manual M503 or M43 or anything else). Connecting the display doesn't change this (displays all squares).

comps commented 6 years ago

I can however confirm that

diff --git a/platformio.ini b/platformio.ini
index 9ef403065..bdb3ae7f2 100644
--- a/platformio.ini
+++ b/platformio.ini
@@ -217,7 +217,10 @@ platform     = atmelavr
 framework    = arduino
 board        = sanguino_atmega1284p
 build_flags  = ${common.build_flags}
-lib_deps     = ${common.lib_deps}
+lib_deps     =
+       ${common.lib_deps}
+       LiquidCrystal@1.3.4
+lib_ignore   = LiquidCrystal_I2C
 src_filter   = ${common.default_src_filter}
 monitor_baud = 250000

makes the display (and firmware) work without any code changes.

Given this, something like

diff --git a/Marlin/src/lcd/ultralcd_impl_HD44780.h b/Marlin/src/lcd/ultralcd_impl_HD44780.h
index 7511d97c6..a49827bfb 100644
--- a/Marlin/src/lcd/ultralcd_impl_HD44780.h
+++ b/Marlin/src/lcd/ultralcd_impl_HD44780.h
@@ -62,7 +62,7 @@
   #endif

 #elif ENABLED(LCM1602)
-  LCD_CLASS lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);
+  LiquidCrystal lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);

 #else
   // Standard directly connected LCD implementations

should do it (or an equivalent change in ultralcd_common_HD44780.h, but I ran out of time and couldn't test it.

AletheianAlex commented 6 years ago

I took a similar route with the LiquidCrystal@1.3.4 lib. I can't test your exact changes right now because I am rebuilding my toolchain and have to find time to finish that up.

comps commented 6 years ago

Okay, so the minimum change needed (aside from Configuration.h as seen above) is:

diff --git a/Marlin/src/lcd/ultralcd_common_HD44780.h b/Marlin/src/lcd/ultralcd_common_HD44780.h
index c0d3bca72..5534c77ac 100644
--- a/Marlin/src/lcd/ultralcd_common_HD44780.h
+++ b/Marlin/src/lcd/ultralcd_common_HD44780.h
@@ -175,8 +175,8 @@ extern volatile uint8_t buttons;  //an extended version of the last checked butt
 #elif ENABLED(LCM1602)
   #include <Wire.h>
   #include <LCD.h>
-  #include <LiquidCrystal_I2C.h>
-  #define LCD_CLASS LiquidCrystal_I2C
+  #include <LiquidCrystal.h>
+  #define LCD_CLASS LiquidCrystal

 #else
   // Standard directly connected LCD implementations

No platformio.ini changes, etc.

This boots fine on my AVR and, from previous tests, should work, though I won't have the display again until a few weeks from now, so I cannot 100% verify it.

The default is 16x2 per the defaults in Conditionals_LCD.h for ULTRA_LCD, but you can override it like I did above.

AletheianAlex commented 6 years ago

I think the grander solution would be to fix all LiquidCrystal_I2C to use the new LiquidCrystal @1.3.4 superclass because the LiquidCrystal_I2C @1.1.2 lib currently used is not working on 32-bit platforms. I suggested a few changes in https://github.com/MarlinFirmware/Marlin/issues/10786 but it seems that there are a few loops in how some of the displays/backpacks are defined, so the changes are on pause.

This works for me (on 32-bit) and I am currently running it. In Conditionals_LCD.h:

#if ENABLED(LCD_SAINSMART_I2C_1602) || ENABLED(LCD_SAINSMART_I2C_2004) || ENABLED(LCM1602)

  #define ULTRA_LCD
  #define LCD_I2C_TYPE_PCF8575

  #ifndef LCD_I2C_ADDRESS
    #define LCD_I2C_ADDRESS 0x27   // I2C Address of the port expander
  #endif

  #if ENABLED(LCD_SAINSMART_I2C_2004)
    #define LCD_WIDTH 20
    #define LCD_HEIGHT 4
  #endif

The LCD_CLASS LiquidCrystal_I2C can stay as-is with this defined in configuration.h :

#define LCM1602
#define LCD_WIDTH 20
#define LCD_HEIGHT 4
#define LCD_I2C_ADDRESS 0x3F

As long as this is set in the .ini env:

lib_deps = LiquidCrystal@1.3.4

and

lib_ignore = LiquidCrystal_I2C

But Your solution of excluding and using LiquidCrystal would be cleaner than the env additions as long as it is 'newliquidcrystal' superclass v1.3.4 , and assuming that lib works on AVR (it should but I can't check at the moment).

comps commented 6 years ago

If indeed LiquidCrystal_I2C doesn't work on 32bit, then it would make sense to replace all its uses by LiquidCrystal@1.3.4 (assuming it can cover everything LiquidCrystal_I2C can) and then just

diff --git a/platformio.ini b/platformio.ini
index 9ef403065..6b91de81f 100644
--- a/platformio.ini
+++ b/platformio.ini
@@ -27,7 +27,7 @@ default_src_filter = +<src/*> -<src/config>
 build_flags = -fmax-errors=5 -O2
 lib_deps =
   https://github.com/MarlinFirmware/U8glib-HAL/archive/dev.zip
-  LiquidCrystal_I2C@1.1.2
+  LiquidCrystal@1.3.4
   TMC2130Stepper
   https://github.com/teemuatlut/TMC2208Stepper/archive/v0.1.1.zip
   Adafruit NeoPixel@1.1.3

No ignore needed.

You need LiquidCrystal@1.3.4 anyway for ULTRA_LCD (and others) to compile because it provides LCD.h.

comps commented 6 years ago

I finally got my 16x2 red display and can say that while

diff --git a/Marlin/src/lcd/ultralcd_common_HD44780.h b/Marlin/src/lcd/ultralcd_common_HD44780.h
index c0d3bca72..5534c77ac 100644
--- a/Marlin/src/lcd/ultralcd_common_HD44780.h
+++ b/Marlin/src/lcd/ultralcd_common_HD44780.h
@@ -175,8 +175,8 @@ extern volatile uint8_t buttons;  //an extended version of the last checked butt
 #elif ENABLED(LCM1602)
   #include <Wire.h>
   #include <LCD.h>
-  #include <LiquidCrystal_I2C.h>
-  #define LCD_CLASS LiquidCrystal_I2C
+  #include <LiquidCrystal.h>
+  #define LCD_CLASS LiquidCrystal

 #else
   // Standard directly connected LCD implementations

compiles, it doesn't initialize the display correctly.

What fixes it for me is really just

diff --git a/platformio.ini b/platformio.ini
index 9ef403065..6b91de81f 100644
--- a/platformio.ini
+++ b/platformio.ini
@@ -27,7 +27,7 @@ default_src_filter = +<src/*> -<src/config>
 build_flags = -fmax-errors=5 -O2
 lib_deps =
   https://github.com/MarlinFirmware/U8glib-HAL/archive/dev.zip
-  LiquidCrystal_I2C@1.1.2
+  LiquidCrystal@1.3.4
   TMC2130Stepper
   https://github.com/teemuatlut/TMC2208Stepper/archive/v0.1.1.zip
   Adafruit NeoPixel@1.1.3

.. confirmed working without any other code changes, leaving LCD_CLASS defined as LiquidCrystal_I2C, using an i2c LCM1602.

It would be interesting to see if this works with other displays (full graphical, etc.) and if it does, maybe we can really just flip the library in the ini.

thinkyhead commented 6 years ago

Is there any noticeable difference in the size of the binary using LiquidCrystal_I2C@1.1.2 versus LiquidCrystal@1.3.4?

comps commented 6 years ago

Hard to say, it doesn't compile with LiquidCrystal_I2C@1.1.2 (see first post) with my configuration. Disabling ULTRA_LCD, both libraries seem to produce the same footprint (likely not being linked at all):

Program:   76870 bytes (58.6% Full)
(.text + .data + .bootloader)

Data:       3379 bytes (20.6% Full)
(.data + .bss + .noinit)

With ULTRA_LCD and LiquidCrystal@1.3.4:

Program:   86064 bytes (65.7% Full)
(.text + .data + .bootloader)

Data:       3651 bytes (22.3% Full)
(.data + .bss + .noinit)

With ULTRA_LCD and LiquidCrystal_I2C@1.1.2, patched with LCD_CLASS lcd(0x27, LCD_WIDTH, LCD_HEIGHT); as noted above (not working, but linkable):

Program:   85328 bytes (65.1% Full)
(.text + .data + .bootloader)

Data:       3637 bytes (22.2% Full)
(.data + .bss + .noinit)
thinkyhead commented 6 years ago

Well, let's try LiquidCrystal@1.3.4 and see how it holds up…

github-actions[bot] commented 4 years ago

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.