I'm trying to move some of my code from my main program into some new tabs or a library because it's getting to be a real pain scrolling all over my program to find functions, and I'm getting the above error.
I got the error after creating a header file with the following:
extern byte leds; // Number of LEDs in total. For now, this will always be led::modules*24
namespace led {
byte modules; // Number of LED modules connected. With current LED modules, there is a maximum of 6. Defined in config.txt.
int * scale; // Scaling factor for LED brightness. Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.
// Valid range in config.txt is [0..1] but this is converted to range [0..4095] when stored so as to make the math simpler when calcualting LED brightnesses.
float * currentValue; // Current LED brightness. [0..1]
float * targetValue; // Target LED brightness. [0..1]
float * speed; // How fast LED should move towards target value. 1.0 = Starting from 0, LED would reach full brightness in one second. Set speed to -1 to change brightness instantly.
byte * data; // LED data is stored as a string of 12 bit LED values. There are 3 bytes for every two LEDs. We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s. Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
void init(); // This function allocates the memory and sets up the LEDs.
void set(int, float, float); // led::set(int index, float brightness, float speed = -1)
float get(int, boolean);
//void toggle(); // Used to blink an LED.
} // 15.5 bytes per LED. At 6 LED modules max, that's 2232 bytes max. Could halve that if I used fixed point for LED values.
extern int LEDFrameTime = 1000 / 48; // The number of milliseconds between each update of the LEDs. 1000 / 24 = 24fps. (Changed to a global so LED framerate can be set in config file.)
I'm about ready to just shove everything back into one source file again because I thought I could just toss my functions in a few tabs and be done with it, but it seems like the whole tabs without extensions thing is pretty broken for anything but the simplest code. It doesn't even support functions with default parameters apparently from what I've been reading.
That worked for byte, (it would be nice if it gave some sort of comprehensible error like "warning type 'byte' is undefined") but then I get the exact same error for 'led' where I declare the namespace.
And yes, that's the whole header file, which I've now got that include at the top of as you suggested.
The compilation error seemed reasonable to me, although I agree the message could have been clearer. You used a type which was not defined in the header file. The error would have occurred when you tried to compile a file that included this header and did not already include a file that defined 'byte'.
It seems to me that you are confused about what a 'header file' is. A header file conventionally contains a set of external declarations. Typically it would be used in combination with an implementation file ('C' or C++) that defines the variables, functions and methods declared in the header. Your led namespace seems to include external declarations as well as variable definitions. This means that each file including that header would end up with its own copy of the definitions, which is probably not what you wanted.
Define your variables, functions and methods in one place and one place only. If you need to refer to variables, types, functions, methods etc from elsewhere then use a header file to publish the declarations that need to be shared. You can then include this in the implementation file, and in the other code that needs to use them.
For larger (or small) projects you could tick the File.Preferences.Use External Editor in the Arduino IDE and use something like Notepad++ with Arduino language references added http://arduino.cc/forum/index.php/topic,112662.0.html
It allows split views, code folding and a host of other useful options.
PaulS:
Why are you defining a namespace, anyway? You certainly don't need to.
Why not? It looks nice and clean and more professional than a random collection of global variables with an "led" prefix stuck in front of them. And it's very clear that all those variables are part of the same code. And I'm going to have a similar set of variables and functions for animating servos.
When you move files into .h and .cpp files you have to use full declarations and includes so that all functions are properly declared with prototypes and all types that you use are properly defined. This is how you program in standard C++ (and C). When you use a .pde/.ino file, the IDE runs a filter over the code help newbies so they don't have to be as precise in terms of the syntax.
I'm aware of how header files are used. Yes, there's some mistakes in my code because I just copied and pasted the code from my main program into the header file. I forgot I needed to use extern for the variables. It's been a long time since I've coded in pure C. But that's neither here nor there, because the error I'm getting is BEFORE I even get to those improperly declared variables.
#include "Arduino.h"
extern byte leds; // Number of LEDs in total. For now, this will always be led::modules*24
namespace led {
extern byte modules; // Number of LED modules connected. With current LED modules, there is a maximum of 6. Defined in config.txt.
extern int * scale; // Scaling factor for LED brightness. Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.
// Valid range in config.txt is [0..1] but this is converted to range [0..4095] when stored so as to make the math simpler when calcualting LED brightnesses.
extern float * currentValue; // Current LED brightness. [0..1]
extern float * targetValue; // Target LED brightness. [0..1]
extern float * speed; // How fast LED should move towards target value. 1.0 = Starting from 0, LED would reach full brightness in one second. Set speed to -1 to change brightness instantly.
extern byte * data; // LED data is stored as a string of 12 bit LED values. There are 3 bytes for every two LEDs. We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s. Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
void init(); // This function allocates the memory and sets up the LEDs.
void set(int, float, float); // led::set(int index, float brightness, float speed = -1)
float get(int, boolean);
//void toggle(); // Used to blink an LED.
} // 15.5 bytes per LED. At 6 LED modules max, that's 2232 bytes max. Could halve that if I used fixed point for LED values.
extern int LEDFrameTime; // = 1000 / 48; // The number of milliseconds between each update of the LEDs. 1000 / 24 = 24fps. (Changed to a global so LED framerate can be set in config file.)
Okay, I tracked the error down to using include "leds.h" in the "leds.c" file, which I thought was okay because I'd seen it done in the wave lib C file but recalling something I'd read about protecting your header files from just that with compiler directives I found that's just what they'd done in the wave lib. Ugh. Why would you ever do something like that which you then have to work around with ugly compiler directives?
That freaking error STILL hasn't gone away though. It's moved on to yet ANOTHER location. I seriously want to murder the person that came up with that error because there seems to be nothing in common to all these situations. I got the error when I didn't have a type defined, I got it when I had a header included twice and now I'm getting it on a function and I have no idea why.
#include "Arduino.h"
extern byte leds; // Number of LEDs in total. For now, this will always be led::modules*24
namespace led {
extern byte modules; // Number of LED modules connected. With current LED modules, there is a maximum of 6. Defined in config.txt.
extern int * scale; // Scaling factor for LED brightness. Used when say, you want to be able to adjust the brightness of the powercell without changing all the code that assumes the max desired brightness is 1.0.
// Valid range in config.txt is [0..1] but this is converted to range [0..4095] when stored so as to make the math simpler when calcualting LED brightnesses.
extern float * currentValue; // Current LED brightness. [0..1]
extern float * targetValue; // Target LED brightness. [0..1]
extern float * speed; // How fast LED should move towards target value. 1.0 = Starting from 0, LED would reach full brightness in one second. Set speed to -1 to change brightness instantly.
extern byte * data; // LED data is stored as a string of 12 bit LED values. There are 3 bytes for every two LEDs. We store it this way because the data does not need to be altered when we are shifting it out to the TLC5947s. Modifying the data with bit shifts would be very expensive and there is a lot of data to move.
void init(); // This function allocates the memory and sets up the LEDs.
void set(int, float, float); // led::set(int index, float brightness, float speed = -1)
float get(int, boolean);
//void toggle(); // Used to blink an LED.
} // 15.5 bytes per LED. At 6 LED modules max, that's 2232 bytes max. Could halve that if I used fixed point for LED values.
extern int LEDFrameTime; // = 1000 / 48; // The number of milliseconds between each update of the LEDs. 1000 / 24 = 24fps. (Changed to a global so LED framerate can be set in config file.)
/*
This function sets up all the LED modules.
*/
void led::init() {
led::modules = 2; // Eventually this will be defined in config.txt. Should check to make sure it is in range.
leds = led::modules * 24;
led::scale = (int *) malloc(leds * sizeof(int));
led::currentValue = (float *) malloc(leds * sizeof(float));
led::targetValue = (float *) malloc(leds * sizeof(float));
led::speed = (float *) malloc(leds * sizeof(float));
led::data = (byte *) malloc(led::modules*36 * sizeof(byte));
for (int x = 0; x < leds; x++) {
led::scale[x] = 4095 * 1.0; // Set LED brightness scale factor to default, then adjust only those which are defined in config.txt. Scale is what currentValue is multiplied by to get integer LED brightness. In config file it will use a flat from 0..1
led::currentValue[x] = 0;
led::targetValue[x] = 0;
led::speed[x] = -1;
}
// Adjust by values in config.txt:
// TEMP:
for (int x = 0; x < leds; x++) {
led::scale[x] = 4095 * 0.5; // Default to 50% brightness for most LEDs (actually 0.5*0.5 or 25% current, because we use logarithmic brightness)
led::speed[x] = 16.0; // Default incandescent simulation = 1/16th of a second to change from 0 to fullbright or vice versa.
}
// Bargraph LEDs 1-15:
for (int x = 1; x <= 15; x++) {
led::scale[x-1] = 4095 * 1.0; // LED rray is 0 indexed.
}
// Vent LED:
// Spare strobe LEDs:
// Cyclotron LEDs:
//for (int x = 0; x < (led::modules * 36); x++) { led::data[x] = 0; } // No need to reset this.
}
/*
This function sets or animates the brightness of an LED.
Parameters:
index - The LED whose brightness you wish to change. Valid range: [1..led::modules*24]
value - The desired brightness level. Valid range: [0..1]
speed - An optional parameter which specifies how much to increase or decrease the brightness by each second. A value of 1.0 will take an led which is off to full bright in one second, or an led which is already at 50% to full bright in half a second.
*/
void led::set(int index, float value, float speed = -1) {
// int targetBrightness, currentBrightness, newBrightness;
//if (index < 1) || (index > leds) { } // Check to make sure the LED index is within the allowed range. If not, log the error and halt.
led::targetValue[index-1] = value;
led::speed[index-1] = speed;
if (speed < 0) { led::currentValue[index-1] = value; } // Set LED immediately to desired brightness.
}
/*
This function returns an LEDs value.
By default, it returns the current value, but if you set target to true, it will return the target value instead.
The current value of an LED changes over time at a speed defined by the LEDs speed. If you set an LED but the speed is not -1, then currentValue will change over time to match the targetValue you set.
The targetValue on the other hand is set immediately regardless of speed. But it may not represent the LED's current level of brightness.
*/
float led::get(int index, boolean target = false) {
//if (index < 1) || (index > (LEDMODULES*24)) { } // Check to make sure the LED index is within the allowed range. If not, log the error and halt.
if (target) {
return led::targetValue[index-1];
} else {
return led::currentValue[index-1];
}
}
/*
This function updates the LED values and shifts all the LED data out to the TLC5947s if it is time to update them again.
*/
void updateLEDs() {
static unsigned long nextLEDFrameTime = 0;
// Move LEDs towards target values.
for (int x = 0; x < leds; x++) {
if (led::speed[x] > 0) {
// Move LED towards desired brightness over time.
if (led::currentValue[x] < led::targetValue[x]) {
led::currentValue[x] += led::speed[x] * timeDeltaSec; // Increase LED brightness.
if (led::currentValue[x] > led::targetValue[x]) { led::currentValue[x] = led::targetValue[x]; } // If we've gone past the target value, set value to target value.
} else {
led::currentValue[x] -= led::speed[x] * timeDeltaSec; // Decrease LED brightness.
if (led::currentValue[x] < led::targetValue[x]) { led::currentValue[x] = led::targetValue[x]; } // If we've gone past the target value, set value to target value.
} // if-else
} // if
} // for
if (time >= nextLEDFrameTime) { // If it is time for an LED update...
fillLEDarray();
updateLEDdrivers(); // Send data to LED drivers.
nextLEDFrameTime = time + LEDFrameTime; // Set the time of the next update.
}
}
/*
This function fills the packed array of 12-bit LED data using the current floating point data.
*/
void fillLEDarray() {
float * value = led::currentValue;// Current brightness of LED.
int * scale = led::scale; // [0..4095]
unsigned int tmp;
byte * dest = led::data;
byte * lastByte = &led::data[led::modules*36]; // Lastbyte must be one greater than actual last byte so loop exits at correct time.
do {
tmp = *value * *value * *scale; // Calculate LED brightness, adjusting by scale factor. (We also multiply the LED value by itself for a logarithmic light level falloff which looks linear to the eye.)
*dest = tmp; // Place bits 7..0 of first LED in data buffer.
dest++; // Advance to next byte of data buffer.
*dest = tmp >> 8; // Place bits 11..8 of first LED in bits 3..0 in data buffer.
value++; // Advance to next LED.
scale++;
tmp = *value * *value * *scale; // Calculate brightness of next LED.
*dest |= (tmp & 0b1111) << 4; // Zero out all bits except 3..0 in brightness, shift them into bits 7..4, and place them in dest, leaving bits 3..0 alone.
// Could do the above shift by 4 by ANDing tmp with 0b1111, SWAPping the high and ow nybbles of *dest, ORing tmp with *dest, and SWAPping the nybbles back. Would avoid conditional loop.
dest++;
*dest = tmp>>4; // Place bits 11..4 of second LED in third byte of data buffer.
/// Another conditional loop could be avoided here with SWAP.
dest++; // Advance to next byte of data buffer.
value++; // Advance to next LED.
scale++;
} while (dest != lastByte);
}
/*
This function shifts all the LED data out to the TLC5947's.
*/
#define NOP __asm__ __volatile__ ("nop\n\t");
#define WAIT NOP NOP NOP NOP NOP NOP NOP NOP NOP NOP NOP // 11 NOPs
void updateLEDdrivers() {
byte tmp;
byte * thisLED = &led::data[led::modules*36]; // The first operation is to decrement this pointer, so the first time we write thisLED it will be array index LEDMODULES*36-1
byte * lastLED = &led::data[0];
//cli(); // Halt interrupts.
SPI_busy = 1;
do {
SPDR = *--thisLED; WAIT;
} while (thisLED != lastLED); // thisLED is decremented one last time before we hit the end of the loop, so after byte 0 transfers, the loop exits.
// Clear transfer flag.
SPSR = SPSR & ~_BV(SPIF);
tmp = SPDR;
//while (!(SPSR & _BV(SPIF)));
// Move data from shift register into latch register:
LATPORT &= ~(1 << LATPIN); // Set LATCH pin low. It is important to do this first, just before transitioning high! I don't know why, but the LED modules won't work right otherwise!
LATPORT |= (1 << LATPIN); // Set LATCH pin high. Transitioning from low to high moves all data from the shift register into the latch registers.
ENAPORT |= (1 << ENAPIN); // Set BLANK pin high. Turn all outputs off, and reset grayscale timer. Blanking reduces flicker, but it does not seem to matter if it is before, after, or in the middle of latching.
ENAPORT &= ~(1 << ENAPIN); // Set BLANK pin low. Turn all outputs on.
SPI_busy = 0;
//sei(); // Reenable interrupts.
}
It's saying it expects all that garbage in the subject line before the ":" token, on this function:
void led::init() {
}
So I got an error about led not being defined so I included my namespace defintion in my main program. And I got an error about byte type so I included Arduino.h. And then I got an error about void init(); needing to be decalred inside the led namepace because I had it commented out.
All well and good but why the hell do I have to redeclare all these functions in my namespace in my cpp file when they're in the header which is included in my main program and the cpp file is only being used because that header is being included? Shouldn't it be taking the definitions from there?
Isn't C contained within C++? Why not just compile everything as if it might potentially contain C++ commands?
The compiler driver cares what extension the file is (i.e. the IDE is just calling gcc foo.c and gcc is invoking the C compiler instead of the C++ compiler because of the extension).
Most of C is contained as a subset within C++, but there are differences and subtle nuances. And obviously if you are using C++ features such as namespaces and '::', you need to make sure the C++ compiler is invoked by naming the file foo.cpp or foo.cc. In the Arduino world, it is best to code in C++, or at least the C subset within C++ and not use C unless you know what you are doing.
scswift:
All well and good but why the hell do I have to redeclare all these functions in my namespace in my cpp file when they're in the header which is included in my main program and the cpp file is only being used because that header is being included? Shouldn't it be taking the definitions from there?
Do you include the .h file in your cpp file? Each file is compiled separately. So if you don't include the foo.h file in foo.cpp, foo.cpp won't see those definitions. The whole point of .h files is to provide a set of common definitions between the callers and callees, and both sides need to include the file.
As I said there are subtle differences. For example, C and C++ have slightly different rules for what 'const' is. Newer C's have designated initializers and C++ did not (yet) include this feature. In C you need to use 'void' to indicate functions that take no arguments, while in C++ this is optional.