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

Compile error with enabled NEOPIXEL #7999

Closed SCiunczyk closed 6 years ago

SCiunczyk commented 7 years ago

The PR #7919 is closed so I think my comment was missed ;-)

@thinkyhead I'm afraid I can't compile this version now :-( (with NEOPIXEL).

first problem: the NEO_WHITE should be as LED_WHITE defined of four components (there is no default parameters in neopixel_set_led_color)

Marlin\src\feature\leds\leds.cpp:39:42: error: too few arguments to function 'bool neopixel_set_led_color(uint8_t, uint8_t,
 uint8_t, uint8_t, uint8_t)'
neopixel_set_led_color(NEO_WHITE, p);

second problem:

Marlin\src\gcode\temperature\M104_M109.cpp: In static member function 'static void GcodeSuite::M109()':
Marlin\src\gcode\temperature\M104_M109.cpp:233:51: error: 'set_neopixel_color' was not declared in this scope
set_neopixel_color(pixels.Color(NEO_WHITE));

IMHO this is redundant because function set_led_color calls set_neopixel_color inside. And there is no reason to treat neopixel different from others and include calls to neopixel directly inside firmware code.

Maybe just define function set_led_white() in leds.h/c and inside this function manage all types of WHITEness.

I like the idea to have only one place to manage all types of leds.

Because I have this corrected on my branch I can make new PR. It's OK?

fiveangle commented 7 years ago

I would make the PR

Tannoo commented 7 years ago

Make a PR... I tried compiling with a fresh copy of bugfix-2.0.x this morning:

platformio run --environment megaatmega2560
Compiling .pioenvs\megaatmega2560\src\src\feature\leds\neopixel.o
Marlin\src\feature\leds\leds.cpp: In function 'void set_led_color(uint8_t, uint8_t, uint8_t, uint8_t, uint8_t)':
Marlin\src\feature\leds\leds.cpp:39:42: error: too few arguments to function 'bool neopixel_set_led_color(uint8_t, uint8_t, uint8_t, ui
nt8_t, uint8_t)'
neopixel_set_led_color(NEO_WHITE, p);
^
In file included from Marlin\src\feature\leds\leds.h:32:0,
from Marlin\src\feature\leds\leds.cpp:31:

Marlin\src\feature\leds\neopixel.h:34:6: note: declared here
bool neopixel_set_led_color(const uint8_t r, const uint8_t g, const uint8_t b, const uint8_t w, const uint8_t p);
^
Compiling .pioenvs\megaatmega2560\src\src\feature\leds\pca9632.o
*** [.pioenvs\megaatmega2560\src\src\feature\leds\leds.o] Error 1

 [SUMMARY]
Environment megaatmega1280              [SKIP]
Environment teensy20                    [SKIP]
Environment rambo                       [SKIP]
Environment anet10                      [SKIP]
Environment sanguino_atmega644p         [SKIP]
Environment DUE                         [SKIP]
Environment teensy35                    [SKIP]
Environment Re-ARM                      [SKIP]
Environment Re-ARM_debug_and_upload     [SKIP]
 [ERROR] Took 16.69 seconds
Environment megaatmega2560              [ERROR]
 [ERROR] Took 16.69 seconds
SCiunczyk commented 7 years ago

There is no PR still - I can send you my corrected files if you want. Only 3 files.

Tannoo commented 7 years ago

Yes... please. I would like it to work and likely in-sync with what might be to come.

SCiunczyk commented 7 years ago

Voila

files.zip

Tannoo commented 7 years ago

Still no go.

How do you get away with neopixel_set_led_color(NEO_WHITE, p);, when the function neopixel_set_led_color(const uint8_t r, const uint8_t g, const uint8_t b, const uint8_t w, const uint8_t p); calls for more than two arguments?

btw... still the same error.

SCiunczyk commented 7 years ago

NEO_WHITE pragma is equal to four arguments in function call

#if ENABLED(NEOPIXEL_LED) 
  #if NEOPIXEL_TYPE == NEO_RGB || NEOPIXEL_TYPE == NEO_RBG || NEOPIXEL_TYPE == NEO_GRB || NEOPIXEL_TYPE == NEO_GBR || NEOPIXEL_TYPE == NEO_BRG || NEOPIXEL_TYPE == NEO_BGR
    #define NEO_WHITE 255, 255, 255, 0
  #else
    #define NEO_WHITE 0, 0, 0, 255
  #endif
#endif
Tannoo commented 7 years ago

I saw that.... currently having some unknown issue with my repo..... getting it straightened out and will try again.

Tannoo commented 7 years ago

Okay... got that straightened out... now back to another old issue to fix. -- Rather, I'll just submit a new issue as I don't know how to fix it.

SCiunczyk commented 7 years ago

@fiveangle any news with PR? Maybe I can help somehow ?

fiveangle commented 7 years ago

@SCiunczyk - I was quickly responding to your question of whether or not you should make a PR. I appoligize for the ambiguity. I meant: "I would make the PR [if I were you]." :)

SCiunczyk commented 7 years ago

Hi hi hi. This is fault of my weak English. OK I will make it immediately.

SCiunczyk commented 7 years ago

Done!

github-actions[bot] commented 3 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.