Closed jtq closed 4 years ago
(Creating this issue from the duscussion started on https://github.com/FastLED/FastLED/pull/1050#pullrequestreview-450988727 now Issues are available on this repo - thanks @ngyl88 !)
Regarding commit 92a62102ad7ea4a01ad5039cab083627a52e759f, sadly this doesn't resolve the issue, as timer0_millis
does still need definition in order to not cause different issues:
In file included from D:\Users\USER\Documents\source\Arduino\SmoothSweep\SmoothSweep.ino:6:0:
D:\Users\USER\Documents\Arduino\libraries\FastLED/FastLED.h:14:21: note: #pragma message: FastLED version 3.003.003
# pragma message "FastLED version 3.003.003"
^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from D:\Users\USER\Documents\Arduino\libraries\FastLED/FastLED.h:65:0,
from D:\Users\USER\Documents\source\Arduino\SmoothSweep\SmoothSweep.ino:6:
D:\Users\USER\Documents\Arduino\libraries\FastLED/fastspi.h:135:23: note: #pragma message: No hardware SPI pins defined. All SPI access will default to bitbanged output
# pragma message "No hardware SPI pins defined. All SPI access will default to bitbanged output"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Users\USER\AppData\Local\Temp\cc2b5eIW.ltrans0.ltrans.o: In function `showPixels':
D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: undefined reference to `timer0_millis'
D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: undefined reference to `timer0_millis'
D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: undefined reference to `timer0_millis'
D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: undefined reference to `timer0_millis'
D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: undefined reference to `timer0_millis'
C:\Users\USER\AppData\Local\Temp\cc2b5eIW.ltrans0.ltrans.o:D:\Users\USER\Documents\Arduino\libraries\FastLED/platforms/avr/clockless_trinket.h:144: more undefined references to `timer0_millis' follow
collect2.exe: error: ld returned 1 exit status
exit status 1
Error compiling for board Arduino Nano Every.
My C++ is rusty as hell, but I believe the basic issue is that while it's fine to declare a variable in a .h header file, you shouldn't define a variable in one. The led_sysdefs_avr.h
header file is included in a lot of different places so it's evaluated a number of times, and while multiple declarations of the same variable aren't a problem in C/C++, multiple definitions of the same variable name trigger a compilation error.
From my vague memory and more recent reading up, the declaration should be in the header file:
// special defs for mega environments
#if defined(__AVR_ATmega4809__)
extern volatile unsigned long timer0_millis;
#endif
but the definition (volatile unsigned long timer0_millis = 0;
) should be in a .c/.cpp file where it's only evaluated once.
The trouble is I have no idea whatsoever in a codebase like FastLED where an appropriate place would be. 😂
HI @jtq , I agree with you, we shouldn't include definition in header files. Regarding commit 92a62102ad7ea4a01ad5039cab083627a52e759f, I'm removing because the declaration in the extern block should already have been included (hopefully my understanding of C still valid).
File led_sysdefs.h
in the root folder
#elif defined(__AVR__) || defined(__AVR_ATmega4809__)
// AVR platforms
#include "platforms/avr/led_sysdefs_avr.h"
File platforms/avr/led_sysdefs_avr.h
extern "C" {
# if defined(CORE_TEENSY) || defined(TEENSYDUINO)
extern volatile unsigned long timer0_millis_count;
# define MS_COUNTER timer0_millis_count
# elif defined(ATTINY_CORE)
extern volatile unsigned long millis_timer_millis;
# define MS_COUNTER millis_timer_millis
# else
extern volatile unsigned long timer0_millis;
# define MS_COUNTER timer0_millis
# endif
};
I think it might be relevant to ATmega4809 is not expecting this variable, as stated in this comment? https://github.com/FastLED/FastLED/issues/716#issuecomment-647146670
timer0_millis
and replace it with timer_millis
for AVR_ATmega4809 ? If this the case, what should we define fr MS_COUNTER?millis_timer_millis
like ATTINY_CORE?Hi @jtq , looks like I have figured out.
AVR is using timer0_millis
, while ATmega4809 is using timer_millis
.
Could you help to try with the latest commit?
Success! As of commit d0149ad
I can compile and use FastLED on the Nano Every without needing to prefix my code with volatile unsigned long timer0_millis = 0;
.
Many thanks for this fix, @ngyl88 !
@jtq You're welcomed. Thanks for the help with the testing.
I'm pretty new to arduino (and haven't done much C for about 25 years!), but I checked out this branch, installed it as a .zip library through the Arduino UI and tried to compile some of the provided example sketches (Blink, etc) for the Nano Every, and every one I tested displayed the following errors:
Commenting out the
#if... #endif
block and inserting thevolatile unsigned long timer0_millis = 0;
line at the top of a sketch just under the#include <FastLED.h>
line fixes the problem, allowing sketches to compile for Nano Every and run correctly on the device.I would have registered an issue on the source repo (https://github.com/ngyl88-arduino/FastLED), but it appears to have issues disabled so this was the best place I could find to raise it.