Help! error-expected unqualified-id before 'else'

Hi,

I’m working with a Mega, and trying to move through a TLC5940 tutorial I found on the AVR freaks forum. I’ve run into a bit of a brick wall though. Every time I compile now I get this error
“error-expected unqualified-id before ‘else’”.

I think it is related to this section

#if (12 * TLC5940_N > 255)
#define dcData_t uint16_t
#else
#define dcData_t uint8_t
#endif

#if (24 * TLC5940_N > 255)
#define gsData_t uint16_t
#else
#define gsData_t uint8_t
#endif

#define dcDataSize ((dcData_t)12 * TLC5940_N)
#define gsDataSize ((gsData_t)24 * TLC5940_N)

Here is the complete code

#include <stdint.h>
// Definition of interrupt names
#include <avr/interrupt.h>
// ISR interrupt service routine
#include <avr/io.h>

// TLC5940 pin definitions
// PWM PIN 11
#define GSCLK_DDR DDRB
#define GSCLK_PORT PORTB
#define GSCLK_PIN PB5
// MOSI PIN 51
#define SIN_DDR DDRB
#define SIN_PORT PORTB
#define SIN_PIN PB2
// SCK PIN 52
#define SCLK_DDR DDRB
#define SCLK_PORT PORTB
#define SCLK_PIN PB1
// SS PIN 53
#define BLANK_DDR DDRB
#define BLANK_PORT PORTB
#define BLANK_PIN PB0

#define DCPRG_DDR DDRC
#define DCPRG_PORT PORTC
#define DCPRG_PIN PC4

#define VPRG_DDR DDRC
#define VPRG_PORT PORTC
#define VPRG_PIN PC7

#define XLAT_DDR DDRC
#define XLAT_PORT PORTC
#define XLAT_PIN PC1

#define TLC5940_N 1

// Macros
#define setOutput(ddr, pin) ((ddr) |= (1 << (pin)))
#define setLow(port, pin) ((port) &= ~(1 << (pin)))
#define setHigh(port, pin) ((port) |= (1 << (pin)))
#define pulse(port, pin) do { \
                              setHigh((port), (pin)); \
                              setLow((port), (pin)); \
                        } while (0)
                        
#define outputState(port, pin) ((port) & (1 << (pin)))

#if (12 * TLC5940_N > 255)
#define dcData_t uint16_t
#else
#define dcData_t uint8_t
#endif

#if (24 * TLC5940_N > 255)
#define gsData_t uint16_t
#else
#define gsData_t uint8_t
#endif

#define dcDataSize ((dcData_t)12 * TLC5940_N)
#define gsDataSize ((gsData_t)24 * TLC5940_N)

// Array for Dot Correction Data
uint8_t dcData[12 * TLC5940_N] = {
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
  0b11111111,
};

// Array for GreyScale Data
uint8_t gsData[24 * TLC5940_N] = {
  0b00000000,
  0b00000000,
  0b00000000,
  0b00000000,
  0b00000000,
  0b00000001,
  0b00000000,
  0b00100000,
  0b00000100,
  0b00000000,
  0b10000000,
  0b00010000,
  0b00000010,
  0b00000000,
  0b01000000,
  0b00001000,
  0b00000001,
  0b00000000,
  0b00100000,
  0b00000100,
  0b00000000,
  0b10000000,
  0b00001111,
  0b11111111,
};

void TLC5940_Init(void) {
  setOutput(GSCLK_DDR, GSCLK_PIN);
  setOutput(SCLK_DDR, SCLK_PIN);
  setOutput(DCPRG_DDR, DCPRG_PIN);
  setOutput(VPRG_DDR, VPRG_PIN);
  setOutput(XLAT_DDR, XLAT_PIN);
  setOutput(BLANK_DDR, BLANK_PIN);
  setOutput(SIN_DDR, SIN_PIN);

  setLow(GSCLK_PORT, GSCLK_PIN);
  setLow(SCLK_PORT, SCLK_PIN);
  setLow(DCPRG_PORT, DCPRG_PIN);
  setHigh(VPRG_PORT, VPRG_PIN);
  setLow(XLAT_PORT, XLAT_PIN);
  setHigh(BLANK_PORT, BLANK_PIN);
  
  // Enable SPI, Master, set clock rate fck/2
  SPCR = (1 << SPE) | (1 << MSTR);
  SPSR = (1 << SPI2X);
  
  // Dont need to call sei(); because Arduino already does this
  
  // SET TIMERS
  // Clear TIMER1 Reg back to default
  TCCR1A = B00000000;
  TCCR1B = B00000000;
  // Enable timer 1 Compare Output channel A in toggle mode
  TCCR1A |= (1 << COM1A0);
  // Configure timer 1 for CTC mode
  TCCR1B |= (1 << WGM12);
  // Set up timer to fCPU (no Prescale) = 16Mhz/1 = 16Mhz 
  TCCR1B |= (1 << CS10);
  // Set CTC compare value to pulse PIN at 8Mhz
  // (1 / Target Frequency) / (1 / Timer Clock Frequency) - 1
  // (1/16000000)/(1/16000000)-1 = 0 = 16Mhz
  // Full period of PIN 11 pulse requires 2 clock ticks (HIGH, LOW) 
  // So = f/2 = 16Mhz/2 = 8Mhz
  OCR1A = 0;
  
  // Clear TIMER3 Reg back to default
  TCCR3A = B00000000;
  TCCR3B = B00000000;
  // Configure timer 3 for CTC mode
  TCCR3B |= (1 << WGM32);
  // Set up timer to fCPU (no prescale) = 16Mhz/1 = 16Mhz 
  TCCR3B |= (1 << CS30);
  // Set CTC compare value to 4096 full clock periods so 4096 @ 8Mhz
  // Hence (GSCLK value @ 16Mhz * 2 to bring it down to 8Mhz) -1
  OCR3A = (4096*2) - 1;
  // Enable Timer/Counter3 Compare Match A interrupt
  TIMSK3 |= (1 << OCIE3A);
}

void TLC5940_ClockInDC(void) {
  setHigh(DCPRG_PORT, DCPRG_PIN);
  setHigh(VPRG_PORT, VPRG_PIN);
  
  for (dcData_t i = 0; i < dcDataSize; i++) {
    // Start transmission
    SPDR = dcData[i];
    // Wait for transmission complete
    while (!(SPSR & (1 << SPIF)));
  }
  pulse(XLAT_PORT, XLAT_PIN);
}

ISR(TIMER3_COMPA_vect) {
  static uint8_t xlatNeedsPulse = 0;
  setHigh(BLANK_PORT, BLANK_PIN);
  if (outputState(VPRG_PORT, VPRG_PIN)) {
    setLow(VPRG_PORT, VPRG_PIN);
    if (xlatNeedsPulse) {
      pulse(XLAT_PORT, XLAT_PIN);
      xlatNeedsPulse = 0;
    }
    pulse(SCLK_PORT, SCLK_PIN);
  } 
  else if (xlatNeedsPulse) {
    pulse(XLAT_PORT, XLAT_PIN);
    xlatNeedsPulse = 0;
  }
  setLow(BLANK_PORT, BLANK_PIN);
  // Below this we have 4096 cycles to shift in the data for the next cycle
  for (gsData_t i = 0; i < gsDataSize; i++) {
    SPDR = gsData[i];
    while (!(SPSR & (1 << SPIF)));
  }
  xlatNeedsPulse = 1;
}

void setup(){
  TLC5940_Init();
  //*** for now keep this out and just hold DCPROG LOW using the default EEPROM value
  TLC5940_ClockInDC();

}

void loop() {
  
}

Thanks for any help :slight_smile:

I don’t know if this will help at all, but here is the version before. It compiles and works just fine. No SPI in it though, and the code is a little different.

#include <stdint.h>
// Definition of interrupt names
#include <avr/interrupt.h>
// ISR interrupt service routine
#include <avr/io.h>

// TLC5940 pin definitions
// PWM PIN 11
#define GSCLK_DDR DDRB
#define GSCLK_PORT PORTB
#define GSCLK_PIN PB5
// MOSI PIN 51
#define SIN_DDR DDRB
#define SIN_PORT PORTB
#define SIN_PIN PB2
// SCK PIN 52
#define SCLK_DDR DDRB
#define SCLK_PORT PORTB
#define SCLK_PIN PB1
// SS PIN 53
#define BLANK_DDR DDRB
#define BLANK_PORT PORTB
#define BLANK_PIN PB0

#define DCPRG_DDR DDRC
#define DCPRG_PORT PORTC
#define DCPRG_PIN PC4

#define VPRG_DDR DDRC
#define VPRG_PORT PORTC
#define VPRG_PIN PC7

#define XLAT_DDR DDRC
#define XLAT_PORT PORTC
#define XLAT_PIN PC1

#define TLC5940_N 1

// Macros
#define setOutput(ddr, pin) ((ddr) |= (1 << (pin)))
#define setLow(port, pin) ((port) &= ~(1 << (pin)))
#define setHigh(port, pin) ((port) |= (1 << (pin)))
#define pulse(port, pin) do { \
                              setHigh((port), (pin)); \
                              setLow((port), (pin)); \
                        } while (0)
                        
#define outputState(port, pin) ((port) & (1 << (pin)))

uint8_t dcData[96 * TLC5940_N] = {
  // MSB            LSB
  1, 1, 1, 1, 1, 1, // Channel 15
  1, 1, 1, 1, 1, 1, // Channel 14
  1, 1, 1, 1, 1, 1, // Channel 13
  1, 1, 1, 1, 1, 1, // Channel 12
  1, 1, 1, 1, 1, 1, // Channel 11
  1, 1, 1, 1, 1, 1, // Channel 10
  1, 1, 1, 1, 1, 1, // Channel 9
  1, 1, 1, 1, 1, 1, // Channel 8
  1, 1, 1, 1, 1, 1, // Channel 7
  1, 1, 1, 1, 1, 1, // Channel 6
  1, 1, 1, 1, 1, 1, // Channel 5
  1, 1, 1, 1, 1, 1, // Channel 4
  1, 1, 1, 1, 1, 1, // Channel 3
  1, 1, 1, 1, 1, 1, // Channel 2
  1, 1, 1, 1, 1, 1, // Channel 1
  1, 1, 1, 1, 1, 1, // Channel 0
};

uint8_t gsData[192 * TLC5940_N] = {
  // MSB                              LSB
  0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, // Channel 15
  0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, // Channel 14
  0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, // Channel 13
  0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, // Channel 12
  0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 0, // Channel 11
  0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, // Channel 10
  0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, // Channel 9
  0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, // Channel 8
  0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, // Channel 7
  0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, // Channel 6
  0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, // Channel 5
  0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, // Channel 4
  0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Channel 3
  0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 0, // Channel 2
  0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, // Channel 1
  0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, // Channel 0
};

void TLC5940_Init(void) {
  Serial.begin(38400);
  setOutput(GSCLK_DDR, GSCLK_PIN);
  setOutput(SCLK_DDR, SCLK_PIN);
  setOutput(DCPRG_DDR, DCPRG_PIN);
  setOutput(VPRG_DDR, VPRG_PIN);
  setOutput(XLAT_DDR, XLAT_PIN);
  setOutput(BLANK_DDR, BLANK_PIN);
  setOutput(SIN_DDR, SIN_PIN);

  setLow(GSCLK_PORT, GSCLK_PIN);
  setLow(SCLK_PORT, SCLK_PIN);
  setLow(DCPRG_PORT, DCPRG_PIN);
  setHigh(VPRG_PORT, VPRG_PIN);
  setLow(XLAT_PORT, XLAT_PIN);
  setHigh(BLANK_PORT, BLANK_PIN);
  
  // Dont need to call sei(); because Arduino already does this
  
  // SET TIMERS
  // Clear TIMER1 Reg back to default
  TCCR1A = B00000000;
  TCCR1B = B00000000;
  // Enable timer 1 Compare Output channel A in toggle mode
  TCCR1A |= (1 << COM1A0);
  // Configure timer 1 for CTC mode
  TCCR1B |= (1 << WGM12);
  // Set up timer to fCPU (no Prescale) = 16Mhz/1 = 16Mhz
  TCCR1B |= (1 << CS10);
  // Set CTC compare value to pulse PIN at 8Mhz
  // (1 / Target Frequency) / (1 / Timer Clock Frequency) - 1
  // (1/16000000)/(1/16000000)-1 = 0 = 16Mhz
  // Full period of PIN 11 pulse requires 2 clock ticks (HIGH, LOW) 
  // So = f/2 = 16Mhz/2 = 8Mhz
  OCR1A = 0;
  
  // Clear TIMER3 Reg back to default  
  TCCR3A = B00000000;
  TCCR3B = B00000000;
  // Configure timer 3 for CTC mode
  TCCR3B |= (1 << WGM32);
  // Set up timer to fCPU (no prescale) = 16Mhz/1 = 16Mhz 
  TCCR3B |= (1 << CS30);
  // Set CTC compare value to 4096 full clock periods so 4096 @ 8Mhz
  // Hence (GSCLK value @ 16Mhz * 2 to bring it down to 8Mhz) -1
  OCR3A = (4096*2) - 1;
  // Enable Timer/Counter3 Compare Match A interrupt
  TIMSK3 |= (1 << OCIE3A);
}

void TLC5940_ClockInDC(void) {
  setHigh(DCPRG_PORT, DCPRG_PIN);
  setHigh(VPRG_PORT, VPRG_PIN);
  uint8_t Counter = 0;
  for (;;) {
    if (Counter > TLC5940_N * 96 - 1) {
      pulse(XLAT_PORT, XLAT_PIN);
      break;
    } 
    else {
      if (dcData[Counter])
        setHigh(SIN_PORT, SIN_PIN);
      else
        setLow(SIN_PORT, SIN_PIN);
      pulse(SCLK_PORT, SCLK_PIN);
      Counter++;
    }
  }
}

ISR(TIMER3_COMPA_vect) {
  uint8_t firstCycleFlag = 0;
  static uint8_t xlatNeedsPulse = 0;
  setHigh(BLANK_PORT, BLANK_PIN);
  if (outputState(VPRG_PORT, VPRG_PIN)) {
    setLow(VPRG_PORT, VPRG_PIN);
    firstCycleFlag = 1;
  }
  if (xlatNeedsPulse) {
    pulse(XLAT_PORT, XLAT_PIN);
    xlatNeedsPulse = 0;
  }
  if (firstCycleFlag)
    pulse(SCLK_PORT, SCLK_PIN);
  setLow(BLANK_PORT, BLANK_PIN);
  // Below this we have 4096 cycles to shift in the data for the next cycle
  uint8_t Data_Counter = 0;
  for (;;) {
    if (!(Data_Counter > TLC5940_N * 192 - 1)) {
      if (gsData[Data_Counter])
        setHigh(SIN_PORT, SIN_PIN);
      else
        setLow(SIN_PORT, SIN_PIN);
      pulse(SCLK_PORT, SCLK_PIN);
      Data_Counter++;
    } 
    else {
      xlatNeedsPulse = 1;
      break;
    }
  }
}

void setup(){
  TLC5940_Init();
  //*** for now keep this out and just hold DCPROG LOW using the default EEPROM value
  TLC5940_ClockInDC();
}

void loop() {
}

I am new to the Arduino way, so maybe some guru can tell me if I am wrong (or maybe point me to a bug report that might have been posted and/or resolved)

When I look at the cpp file generated by Arduino-0018, I see the following in the part where Arduino has collected the function prototypes from the sketch.

.
.
.
void TLC5940_Init(void);
void TLC5940_ClockInDC(void);
else if (xlatNeedsPulse);
void setup();
void loop();
.
.
.

This is (obviouly, I think) the source of the error message.

It seems to me that when Arduino collects prototypes for this particular sketch it can't handle

.
.
.
  else if (xlatNeedsPulse) {
.
.
.

When I change your if...else if... part of the code to the following, it compiles OK.

  if (outputState(VPRG_PORT, VPRG_PIN)) {
    setLow(VPRG_PORT, VPRG_PIN);
    if (xlatNeedsPulse) {
      pulse(XLAT_PORT, XLAT_PIN);
      xlatNeedsPulse = 0;
    }
    pulse(SCLK_PORT, SCLK_PIN);
  }
  else {
    if (xlatNeedsPulse) {
      pulse(XLAT_PORT, XLAT_PIN);
      xlatNeedsPulse = 0;
    }
  }

Bottom line: Your code itself may be valid standard C (and C++) code but it seems that Arduino sometimes requires things to be a little different. I know that the else if construct works in other sketches including many Arduino examples, but maybe Arduino got confused by some of the macros. See Footnote.

Regards,

Dave

The if..else if.. is used by everyone, everywhere, including many Arduino examples, so I think restricting my (and your) use of it is totally unacceptable.

However, here's a suggestion for programming style that (maybe) avoids having to change the way you implement conditionals:

Many (most?) C and C++ programmers avoid macro function definitions since they (sometimes) make debugging so difficult. (And there are lots of other reasons.)

In this case, Arduino got confused (I think) by your pulse macro.

Instead of a macro for pulse try using a function:

Go back to your original code (with else if the way that you wrote it) but replace your #define pulse... macro by something like the following:

inline void pulse(uint8_t port, uint8_t pin)
{
    setHigh(port, pin);
    setLow(port, pin);
}

I mean, although I think your macro definition is legal C and C++ code, I really, really (really) prefer a function. (And Arduino agrees with me, I think.)

Note that I haven't tested the sketch since I don't have the same hardware as you, but it does compile with either of the changes that I showed.

Final question for gurus: I think that for ATmega devices the data types for port and pin numbers can be uint8_t as I show here, but for other avr processors it would be different. This is the kind of detail that Arduino (and the use of macros instead of functions) tries to keep us from having to know, right?

Thanks so much for the great info. I replaced the macro with the Inline function and the program compiles and loads now... however, I can't seem to get the SPI to load any data to the TLC5940 :frowning: Gonna keep working on it.

Thanks again for the great help.

I got the SPI working. Turned I had to drop the nested macro and the inline function. I ended up calling the setHigh and setLow macros directly.

Here's the thread where I got it working

http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1278307902 :slight_smile: