Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed build break (size overflow) HelixPico with Backlight or Underglow #3546

Merged
merged 4 commits into from
Aug 2, 2018

Conversation

mtei
Copy link
Contributor

@mtei mtei commented Aug 1, 2018

No description provided.

@drashna
Copy link
Member

drashna commented Aug 1, 2018

It may help to make sure that console and command are disabled in the rules.mk (for the main keyboard, actually).

And it may be a good idea to add this block to the config.h:

#if !defined(NO_DEBUG) && !defined(CONSOLE_ENABLE)
#define NO_DEBUG // if console isn't enabled, why are we debugging? 
#endif // !NO_DEBUG
#if !defined(NO_PRINT) && !defined(CONSOLE_ENABLE)
#define NO_PRINT // if console is disabled, there is nowhere to print to
#endif // !NO_PRINT
// LTO has issues with macros (action_get_macro) and "functions" (fn_actions), so just disable them
#define NO_ACTION_MACRO
#define NO_ACTION_FUNCTION

#define DISABLE_LEADER

I use this, along with EXTRAFLAGS += -flto in the rules.mk file to reduce the file size. And it really does help.

@mtei
Copy link
Contributor Author

mtei commented Aug 2, 2018

this block

#if !defined(NO_DEBUG) && !defined(CONSOLE_ENABLE)
#define NO_DEBUG // if console isn't enabled, why are we debugging? 
#endif // !NO_DEBUG
#if !defined(NO_PRINT) && !defined(CONSOLE_ENABLE)
#define NO_PRINT // if console is disabled, there is nowhere to print to
#endif // !NO_PRINT

more simply as follows?

#if !defined(CONSOLE_ENABLE)
  #define NO_DEBUG
  #define NO_PRINT
#endif

@drashna
Copy link
Member

drashna commented Aug 2, 2018

Sorry, yeah, it probably would.

I have it set that way, as some boards have those defined already, and if you redefine them with undefining them... it causes the compiler to error out. Bet if this in the keyboard itself, then that would be better

@mtei
Copy link
Contributor Author

mtei commented Aug 2, 2018

I tried '#define NO_DEBUG' and '#define NO_PRINT' in config.h, but it occured error.

because tmk_core/common.mk include this block.

ifeq ($(strip $(CONSOLE_ENABLE)), yes)
    TMK_COMMON_DEFS += -DCONSOLE_ENABLE
else
    TMK_COMMON_DEFS += -DNO_PRINT
    TMK_COMMON_DEFS += -DNO_DEBUG
endif

@drashna
Copy link
Member

drashna commented Aug 2, 2018

Ah, good to know!

However, the other stuff would still help (the no action function and no macros)

@drashna
Copy link
Member

drashna commented Aug 2, 2018

Thanks!

@drashna drashna merged commit b29799f into qmk:master Aug 2, 2018
@mtei
Copy link
Contributor Author

mtei commented Aug 3, 2018

Thank you !

@mtei mtei deleted the helixpico-fix branch August 3, 2018 00:55
alexey-danilov pushed a commit to alexey-danilov/qmk_firmware that referenced this pull request Aug 3, 2018
…or Underglow (qmk#3546)

* build break fix for HelixPico

* add customize variable 'Link_Time_Optimization' into rev2 and pico keymaps rules.mk

* "CFLAGS += -flto" change to "EXTRAFLAGS += -flto"

* add USE_Link_Time_Optimization macro
ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Sep 25, 2018
…or Underglow (qmk#3546)

* build break fix for HelixPico

* add customize variable 'Link_Time_Optimization' into rev2 and pico keymaps rules.mk

* "CFLAGS += -flto" change to "EXTRAFLAGS += -flto"

* add USE_Link_Time_Optimization macro
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
…or Underglow (qmk#3546)

* build break fix for HelixPico

* add customize variable 'Link_Time_Optimization' into rev2 and pico keymaps rules.mk

* "CFLAGS += -flto" change to "EXTRAFLAGS += -flto"

* add USE_Link_Time_Optimization macro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants