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.28k stars 19.24k forks source link

[2.0] PIO toolchain: Is there a better way to handle lib_deps, lib_ldf_mode? #10736

Closed AletheianAlex closed 5 years ago

AletheianAlex commented 6 years ago

Note: I have found a workaround for my particular issue, but I wasted a weekend of coding figuring out the mechanism behind it.

While trying to work on the STM32 code, lib_ldf_mode=1 includes libs that I know are broken with this core (which are scanned from cpp files and included but not found since I explicitly excluded them), but lib_ldf_mode=0 breaks the build’s dependency graph and can only find liquidcrystal_i2c and 30aa480 from [common] lib_deps (missing out on eeprom, wire, add, servo, spi, etc). Because guessing which libs to explicitly exclude is frustrating and can be broken easily in future versions if ldf automagically decides to include other libs, and the build brakes regardless if it requires an excluded lib.

My solution was tracking down all the unnecessary cpp files that reference a header which includes another header that includes <U8glib.h>, moving them in a naughty folder and zipping it... that solved it, and now the LCD is functioning with just ULTRA_LCD and pin defines like it did in v1.x. But that process was difficult to say the least.

Example:

Trying to work with character based LCD should exclude the u8g libs which seem to not be working properly on this core. Defining ULTRA_LCD should not pull in or require u8g?? (I tried tracking down all the includes down by hand to make sure), but it seems to scan the u8g related *.cpp files, follow them back to their headers, and then throw an error if they are not included... in my case a u8g cpp to a u8g header, which had #include <U8glib.h>. Installing U8glib breaks the build (as well as the Hal and arm versions that I tried... u8g2 doesn’t but nothing is built on that), and if I put it on the lib_ignore list, the build fails with “fatal error: U8glib.h: No such file or directory”, so I assume there is a PIO parsing problem with conditional includes. I searched their support, but the mods tend to shut down threads prematurely...

The default LDF finder mode (used in the marlin ini) is ‘chain’ (i.e. mode 1), which does not evaluate preprocessor conditionals. I tried chain+ and deep+ don’t seem to help though.

Would moving the bare necessity common deps to a .json file be possible with the git/zip way of handling things? Or something similar? Because unless PIO makes more granular options and a more robust search that scans for unused .cpp files, we may have similar issues, and in no way can I get a successful build with without extensive modification, getting all libs to work with all cores, or providing more modular separation in the headers for PIO's sake

Happy ending though: LCD is “working”

Roxy-3D commented 6 years ago

I'm fighting a similar problem. In order to put the FreeRTOS library under Marlin we need to make some changes from the default configuration files. And we also need to delete a few lines of buggy code from one of the files. We can not just import the library into Arduino and use it.

Any library experts that have advice to offer will be welcome!

xC0000005 commented 6 years ago

The MalyanM200 V1 section has a list of libs to ignore that's a mile long for exactly this reason, and I don't have a good solution, so I'm watching this to see what people come up with.

AletheianAlex commented 6 years ago

I’m going to look into their library.json setup to see if the essential libs can be controlled with that, so that I can just turn off LDF completely since just getting a f103CB running with a plain LCD was painful enough. Or maybe LDF can have its scope limited to keep it away from certain subfolders... I have no idea, I’m still working through the PIO docs.

Roxy-3D commented 6 years ago

I'm wondering about my case where I need to change the default library settings... Can I just override the #defines in the library's Configuration.h files? Can I force some symbols to be #undef and override some #define values? Can I add that as a compiler flag for everything that gets compiled to build Marlin?

That would not fix the 2 or 3 lines of code I need deleted... But that would get me most of the way to where I need to be.

AletheianAlex commented 6 years ago

Just a thought, but this was an example they gave that I was tinkering with earlier for the source filter, using lib_ldf_mode = chain+ and preprocessor conditionals to exclude using chain+

[env:myenv]
lib_ldf_mode = chain+
build_flags = -D MY_PROJECT_VERSION=13
#ifdef PROJECT_VERSION
  // include common file for the project
  #include "my_common.h"
#endif

mylib.h

#if PROJECT_VERSION < 10
  // this include will be ignored because does not satisfy condition above
  #include "my_old.h"
#endif

So maaaaaybe defining some kind of -D MODIFIED as a build flag and then set #if MODIFIED == true in the modified lib or something like that

thinkyhead commented 6 years ago

My solution was tracking down all the unnecessary cpp files that reference a header which includes another header that includes <U8glib.h>

Headers and dependencies will need to be cleaned up. Marlin shouldn't include any headers it isn't actually using. And as a rule, .h files should never include other headers unless the .h file itself needs symbols defined in those headers. Headers that define code (inlines, for example) may, and should, include MarlinConfigPre.h so they can use conditionals to exclude the #includes and definitions that won't actually be used.

Defining ULTRA_LCD should not pull in or require u8g?

It shouldn't. Only defining DOGLCD should.

So, for example, headers should make sure to do this…

#include "../../inc/MarlinConfigPre.h"

#if ENABLED(DOGLCD)
  #include <U8glib.h>
#endif

…and all the "u8g_com_*.cpp files absolutely must do this:

#include "../../inc/MarlinConfigPre.h"

#if ENABLED(DOGLCD)

// The entire cpp file's contents go here

#endif // DOGLCD
AletheianAlex commented 6 years ago

Gotcha. My issue was:

In file included from Marlin/src/lcd/u8g_fontutf8.cpp:16:0:
Marlin/src/lcd/u8g_fontutf8.h:12:10: fatal error: U8glib.h: No such file or directory
#include <U8glib.h>
^~~~~~~~~~

Here are the top portions of those files:

/**
 * @file    fontutf8.c
 * @brief   font api for u8g lib
 * @author  Yunhui Fu (yhfudev@gmail.com)
 * @version 1.0
 * @date    2015-02-19
 * @copyright GPL/BSD
 */

#include "../inc/MarlinConfig.h"

#if ENABLED(ULTRA_LCD)

#include <string.h>
#include "fontutils.h"
#include "u8g_fontutf8.h"

////////////////////////////////////////////////////////////
/**
 * @file    fontutf8.h
 * @brief   font api for u8g lib
 * @author  Yunhui Fu (yhfudev@gmail.com)
 * @version 1.0
 * @date    2015-02-19
 * @copyright GPL/BSD
 */
#ifndef _UXG_FONTUTF8_H
#define _UXG_FONTUTF8_H 1

#include <U8glib.h>
#include "fontutils.h"

#ifdef __cplusplus
extern "C" {
#endif

I have a src_filter in place in my *.ini build env at the moment to skip over u8g files in that folder, which seems to work for a temp fix.

thinkyhead commented 6 years ago

I've patched all the u8g-related files in the PR linked above and merged the change. See if the current bugfix-2.0.x has solved your issue.

AletheianAlex commented 6 years ago

Not working, but thanks for the quick action. src_filter = -<src/lcd/u8g*> excludes them and builds... Just trying to get basic LCDs working, and it is a chore trying to sort out the PIO errors from the ststm32 platform errors, so I'm trying to do some troubleshooting before posting any reports.

I think it is still including from fontutf8.cpp which wasn't touched in the merge:

/**
 * @file    fontutf8.c
 * @brief   font api for u8g lib
 * @author  Yunhui Fu (yhfudev@gmail.com)
 * @version 1.0
 * @date    2015-02-19
 * @copyright GPL/BSD
 */

#include "../inc/MarlinConfig.h"

#if ENABLED(ULTRA_LCD)

#include <string.h>
#include "fontutils.h"
#include "u8g_fontutf8.h"

And #include "u8g_fontutf8.h" starts

/**
 * @file    fontutf8.h
 * @brief   font api for u8g lib
 * @author  Yunhui Fu (yhfudev@gmail.com)
 * @version 1.0
 * @date    2015-02-19
 * @copyright GPL/BSD
 */
#ifndef _UXG_FONTUTF8_H
#define _UXG_FONTUTF8_H 1

#include <U8glib.h>
#include "fontutils.h"

#ifdef __cplusplus
extern "C" {
#endif

Which has U8glib.h included. src_filter = -<src/lcd/u8g*> excludes them with no ill effect, so I am not sure why these files apply to a character display.

I consequently forgot to mention that I also get the error in ultralcd.cpp:

Marlin/src/lcd/ultralcd.cpp:5474:3: error: 'va_end' was not declared in this scope
va_end(args);
^~~~~~

Marlin/src/lcd/ultralcd.cpp: In function 'void lcd_status_printf_P(uint8_t, const char*, ...)':
Marlin/src/lcd/ultralcd.cpp:5472:3: error: 'va_start' was not declared in this scope
va_start(args, fmt);
^~~~~~~~
^~~~~~~~~~~~~~~~~~

I got around it for now by commenting out the contents of the void function like this, just to get it to build:

void lcd_status_printf_P(const uint8_t level, const char * const fmt, ...) {
  if (level < lcd_status_message_level) return;
  lcd_status_message_level = level;
  //va_list args;
  //va_start(args, fmt);
  //vsnprintf_P(lcd_status_message, MAX_MESSAGE_LENGTH, fmt, args);
  //va_end(args);
  //lcd_finishstatus(level > 0);
}

And then a plain ‘ol #define ULTRA_LCD will work, and the LCD functions.

That is all for a 16x2 LCD with no controller, defined with ULTRA_LCD in configuration.h

BUT I have an extension of the display issue (and therefore may not belong in this thread), which may be related to this subject of ldf, dependencies and pulling in extra files... or not, so I'll add it here in case it is helpful:

if I add a panel by enabling #define REPRAP_DISCOUNT_SMART_CONTROLLER in addition to the above errors, I get the following:

In file included from Marlin/src/lcd/ultralcd.cpp:27:0:
Marlin/src/lcd/ultralcd.cpp: In function 'void lcd_update()':
Marlin/src/lcd/ultralcd.h:228:36: error: 'EN_C' was not declared in this scope
#define LCD_CLICKED (buttons & EN_C)
^
Marlin/src/lcd/ultralcd.cpp:5145:26: note: in expansion of macro 'LCD_CLICKED'
if (UBL_CONDITION && LCD_CLICKED) {
^~~~~~~~~~~

If I comment out the BUTTON_EXISTS conditional in ultra_lcd.h to force the define, the error goes away:

    //#if BUTTON_EXISTS(ENC)
      #define BLEN_C 2
      #define EN_C (_BV(BLEN_C))
    //#endif

So I am not sure what about BUTTON_EXISTS is blocking the define.

And also a more tricky one that I have to change an external lib to get it to build:

Marlin/src/lcd/ultralcd.cpp: At global scope:

Marlin/src/lcd/ultralcd.cpp:4882:34: error: conflicting declaration 'typedef void int8'
DEFINE_MENU_EDIT_TYPE(uint8_t, int8, i8tostr3, 1);
^
Marlin/src/lcd/ultralcd.cpp:4879:18: note: in definition of macro 'DEFINE_MENU_EDIT_TYPE'
typedef void _name
^~~~~
In file included from /Users/User/.platformio/packages/framework-arduinoststm32/STM32F1/cores/maple/boards.h:37:0,
from /Users/User/.platformio/packages/framework-arduinoststm32/STM32F1/cores/maple/wirish.h:54,
from /Users/User/.platformio/packages/framework-arduinoststm32/STM32F1/cores/maple/Arduino.h:30,
from Marlin/src/lcd/../inc/../HAL/HAL_STM32F1/HAL.h:44,
from Marlin/src/lcd/../inc/MarlinConfig.h:28,
from Marlin/src/lcd/ultralcd.h:26,
from Marlin/src/lcd/ultralcd.cpp:36:
/Users/User/.platformio/packages/framework-arduinoststm32/STM32F1/system/libmaple/include/libmaple/libmaple_types.h:47:21: note: previous declaration as 'typedef signed char int8'
typedef signed char int8;
^~~~

From libmaple_types.h:

#include <stdint.h>

#ifdef __cplusplus
extern "C" {
#endif

typedef unsigned char uint8;
typedef unsigned short uint16;
typedef uint32_t uint32;
typedef unsigned long long uint64;

typedef signed char int8;
typedef short int16;
typedef int int32;
typedef long long int64;

Etc, etc, etc,

I’m not sure how to handle that one... I may try an ifdef/define/endif around the DEFINE_MENU_EDIT_TYPE macro in the ultralcd as a temp fix. Unless it is a couple include or... who knows. I don't know if ldf is pulling in an extra file that is adding a def... I’ll keep working on it.

AletheianAlex commented 6 years ago

Yup, the ifndef worked. Got around the ultralcd.cpp “error: conflicting declaration 'typedef void int8’” by:

 DEFINE_MENU_EDIT_TYPE(int16_t, int3, itostr3, 1);
    #ifndef DEFINE_MENU_EDIT_TYPE
      DEFINE_MENU_EDIT_TYPE(uint8_t, int8, i8tostr3, 1);
    #endif
  DEFINE_MENU_EDIT_TYPE(float, float3, ftostr3, 1.0);
  DEFINE_MENU_EDIT_TYPE(float, float52, ftostr52, 100.0);
  DEFINE_MENU_EDIT_TYPE(float, float43, ftostr43sign, 1000.0);
  DEFINE_MENU_EDIT_TYPE(float, float5, ftostr5rj, 0.01);
  DEFINE_MENU_EDIT_TYPE(float, float51, ftostr51sign, 10.0);
  DEFINE_MENU_EDIT_TYPE(float, float52sign, ftostr52sign, 100.0);
  DEFINE_MENU_EDIT_TYPE(float, float62, ftostr62rj, 100.0);
  DEFINE_MENU_EDIT_TYPE(uint32_t, long5, ftostr5rj, 0.01);

Got around the ultralcd.h the “error: EN_C' was not declared” by going back to the v1.x format:

   #define BLEN_A 0
    #define BLEN_B 1
    #if BUTTON_EXISTS(ENC)
      #define BLEN_C 2
    #endif

    #define EN_A (_BV(BLEN_A))
    #define EN_B (_BV(BLEN_B))
    #define EN_C (_BV(BLEN_C))

The “not declared” errors will just have to be solved by commenting them out for now.

thinkyhead commented 6 years ago

I think it is still including <U8glib.h> from fontutf8.cpp which wasn't touched in the merge

Ah. Well make this change to u8g_fontutf8.cpp. I'll push the change soon.

- #if ENABLED(ULTRA_LCD)
+ #if ENABLED(DOGLCD)
AletheianAlex commented 6 years ago

That worked. Thanks

thinkyhead commented 6 years ago

I consequently forgot to mention that I also get the error in ultralcd.cpp:

Add this to ultralcd.cpp

#include <stdarg.h>
thinkyhead commented 6 years ago

And also a more tricky one that I have to change an external lib…

Just make this change in ultralcd.cpp

-   typedef void _name
+   typedef void _name ## _void
thinkyhead commented 6 years ago

Please ZIP up your Configuration.h and Configuration_adv.h files and drop them into your next reply so I can debug any remaining errors in a thoroughgoing manner.

AletheianAlex commented 6 years ago

include worked. That was actually the first thing I tried before __gnuc_va_list, but it caused a ton of errors at the time. Strange.

typedef void _name##_void patch worked as well. I didn't notice that entry was different from th other two in the file.

configuration.h and configuration_adv are unchanged: bone stock except for the custom #define MOTHERBOARD definition with corresponding boards.h/pins.h entry and board pins file. I am just working through the configuration.h display files and trying to get as many to build in PIO as I can. With these changes, character LCDs now are working without trickery.

thinkyhead commented 6 years ago

If it's just the default configs, great. I'll just check whether REPRAP_DISCOUNT_SMART_CONTROLLER still throws any errors. It shouldn't at this point, since I did a small patch to only define that particular LCD_CLICKED when defined(EN_C)

Anyway, let me know if you see any other oddities and we'll patch them up forthwith!

boelle commented 5 years ago

@thinkyhead has this one served its purpose?

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.