AVR Freaks

I had that same error just the other day. There was a missing closing brace in the function before the ISR so the compiler thought I was trying to put the ISR inside the function. Silly compiler.

I don't know exactly what is going on.  A GCC guru will need to tell me how to "save temps" to see preprocessor output.

I certainly don't like the ; at the end of constant #define, but I cannot tell exactly where that might be tripping you up.

The Linux "indent" command can be useful in such cases. I just copied all that text to an "indent.c" and then ran:

$ indent -kr indent.c
indent: indent.c:95: Warning:old style assignment ambiguity in "=&".  Assuming "= &"

indent: indent.c:97: Warning:old style assignment ambiguity in "=&".  Assuming "= &"

indent: indent.c:98: Warning:old style assignment ambiguity in "=&".  Assuming "= &"

indent: indent.c:297: Error:Unexpected end of file

The warnings look spurious as they are referring to things like:

    95	    btnTest = &btnScan; 

which seems like totally valid C to me. However the real eye-opener there is the last message. That basically says "I got to the end of the file before the }'s had got back to the left margin". You can see this in the modified file:

#include <avr/io.h>
#include "stdio.h"
#include "stdlib.h"
#include "string.h"
#include "avr/io.h"
#include "avr/interrupt.h"
#include "switchhandler.h"

//prototypes
void DoSwitch1(void);
void DoSwitch2(void);
int btnScan(int);
void btnUps(int);
void btnHolds(int);
//void SaveToEE(void);
void SendToShiftyTypes(void);
void init_devices(void);
void pgm_init(void);
void timer_init(void);


// switch handler function pointers
extern int (*btnTest) (int index);	// pointer to function to check buttons
extern void (*btnDown) (int index);	// pointer to function to execute
extern void (*btnUp) (int index);	// pointer to function to execute
extern void (*btnHold) (int index);	// pointer to function to execute

// IO
#define	pinSW1		(PINA&0x10)
#define	pinSW2		(PINA&0x20)

// Shifty Types
#define	SS_LO		PORTA&=~0x40;
#define	SS_HI		PORTA|=0x40;

#define		BUTTONTIMEOUT	6

#define SW1_LEDS_OFF	0xF1;
#define SW2_LEDS_OFF	0x8F;

#define SW1_LED1_ON		0x02;
#define SW1_LED2_ON		0x04;
#define SW1_LED3_ON		0x08;

#define SW2_LED1_ON		0x10;
#define SW2_LED2_ON		0x20;
#define SW2_LED3_ON		0x40;

uint8_t LEDs;

// LED Setters
#define SW1_LED1	{LEDs&=SW1_LEDS_OFF;LEDs|=SW1_LED1_ON;}
#define SW1_LED2	{LEDs&=SW1_LEDS_OFF;LEDs|=SW1_LED2_ON;}
#define SW1_LED3	{LEDs&=SW1_LEDS_OFF;LEDs|=SW1_LED3_ON;}

#define SW2_LED1	{LEDs&=SW2_LEDS_OFF;LEDs|=SW2_LED1_ON;}
#define SW2_LED2	{LEDs&=SW2_LEDS_OFF;LEDs|=SW2_LED2_ON;}
#define SW2_LED3	{LEDs&=SW2_LEDS_OFF;LEDs|=SW2_LED3_ON;}

// output macros
#define CH1_OFF			0xFC;
#define CH2_OFF			0xF3;

#define CH1_STATE1		0x00;
#define CH1_STATE2		0x01;
#define CH1_STATE3		0x02;

#define CH2_STATE1		0x00;
#define CH2_STATE2		0x04;
#define CH2_STATE3		0x08;

// output setters
#define CH1_STATE1_ON	{PORTA&=CH1_OFF;PORTA|=CH1_STATE1;}
#define CH1_STATE2_ON	{PORTA&=CH1_OFF;PORTA|=CH1_STATE2;}
#define CH1_STATE3_ON	{PORTA&=CH1_OFF;PORTA|=CH1_STATE3;}

#define CH2_STATE1_ON	{PORTA&=CH2_OFF;PORTA|=CH2_STATE1;}
#define CH2_STATE2_ON	{PORTA&=CH2_OFF;PORTA|=CH2_STATE2;}
#define CH2_STATE3_ON	{PORTA&=CH2_OFF;PORTA|=CH2_STATE3;}


// flags for depressed switches
uint8_t flgSW1, flgSW2;
uint8_t stateSW1, stateSW2;

int main(void)
{
    //stop errant interrupts until set up
    cli();

    init_devices();
    timer_init();
    pgm_init();

    btnTest = &btnScan;
    //      btnDown=&btnDwns;
    btnUp = &btnUps;
    btnHold = &btnHolds;
    SetSwitchHoldParams(0, 167);	//setup length button hold time for ~1 sec

    sei();

    while (1) {
	if (flgSW1) {
	    flgSW1 = 0;
	    DoSwitch1();
	}
	if (flgSW2) {
	    flgSW2 = 0;
	    DoSwitch2();
	}
    }


    void DoSwitch1(void) {
	// inc sw state
	stateSW1++;
	if (stateSW1 >= 3)
	    stateSW1 = 0;
	// update LEDs and outputs
	switch (stateSW1) {
	case 0:
	    SW1_LED1;
	    CH1_STATE1_ON break;
	case 1:
	    SW1_LED2;
	    CH1_STATE2_ON break;
	case 2:
	    SW1_LED3;
	    CH1_STATE3_ON break;
	}
	// send to LEDs
	SendToShiftyTypes();
    }


    void DoSwitch2(void) {
	// inc sw state
	stateSW2++;
	if (stateSW2 >= 3)
	    stateSW2 = 0;
	// update LEDs and outputs
	switch (stateSW2) {
	case 0:
	    SW2_LED1;
	    CH2_STATE1_ON break;
	case 1:
	    SW2_LED2;
	    CH2_STATE2_ON break;
	case 2:
	    SW2_LED3;
	    CH2_STATE3_ON break;
	}
	// send to LEDs
	SendToShiftyTypes();
    }


    int btnScan(int index) {
	switch (index) {
	case 0:
	    return (pinSW1);
	    break;
	case 1:
	    return (pinSW2);
	    break;
	}

	return (1);
    }

/*
void btnDwns(int index)
{
	switch (index) {
	case 0:  					// Mode
//		tmrModeChg=0;			//reset timeout timer
//		flgProcessModeChange=1;	//set flag
		break;
	}

}
*/

    void btnUps(int index) {
	switch (index) {
	case 0:		// Mode 
	    flgSW1 = 1;
	    break;
	case 1:		// Mode
	    flgSW2 = 1;
	    break;
	}
    }


    void btnHolds(int index) {
    }


/*
void SaveToEE(void)
{
	cli();
	eeprom_write_block((void *)&ThisMode,&EE_ThisMode,1);
	eeprom_write_block((void *)&OutMode,&EE_OutMode,1);
	sei();
	tmrModeChg=MODECHGTIMEOUT;
	FlashLEDs(0xff);
	FlashLEDs(0xff);
	FlashLEDs(0xff);
	UpdateLEDs();
}
*/

    void SendToShiftyTypes(void) {
	uint8_t dummy;

	cli();
	SS_LO;

	SPDR = LEDs;
	do {
	} while (!(SPSR & (1 << SPIF)));

	dummy = SPDR;		//dummy read to clear flag

	SS_HI;
	sei();
    }


//call this routine to initialize some stuff
    void init_devices(void) {
	//stop errant interrupts until set up
	cli();

	// set up pins
	DDRA = 0x4F;		// PA 0-3,6 outputs, 4-5 inputs
	DDRB = 0x03;		// PB 0-1 outs

	// init dac pins
	SS_HI;

	//enable interrupts
	sei();
    }


    void pgm_init(void) {
	//cli();
	//      eeprom_read_block((void *)&ThisMode,&EE_ThisMode,1);
	//ThisMode=eeprom_read_byte(&EE_ThisMode);
	//      eeprom_read_block((void *)&OutMode,&EE_OutMode,1);
	//OutMode=eeprom_read_byte(&EE_OutMode);
	//sei();
    }


    void timer_init(void) {
	// for 8.0000MHz clock
	// timer 0 is the 1mS ticker (verified)
	TCCR0A = (1 << WGM01);	// ctc mode
	TCCR0B = (1 << CS02);	// divide by 256
	OCR0A = 0x1E;		// or 30 decimal
	TIMSK = 1 << OCIE0A;	// output compare A match irq
    }


//      1mS timer tick for
//      button timeout, etc
    ISR(TIM0_COMPA_vect) {
	static uint8_t tmrButton;

	tmrButton++;

	if (tmrButton >= BUTTONTIMEOUT) {
	    tmrButton = 0;
	    SwitchHandler();
	}
    }

That all starts because main() is not correctly terminated. As a reality check here is the original:

int main(void)
{
	//stop errant interrupts until set up
	cli();

	init_devices();
	timer_init();
	pgm_init();

	btnTest=&btnScan;
	//	btnDown=&btnDwns;
	btnUp=&btnUps;
	btnHold=&btnHolds;
	SetSwitchHoldParams(0,167);			//setup length button hold time for ~1 sec

	sei();

    while(1)
    {
        if(flgSW1) {
			flgSW1=0;
			DoSwitch1();
		}
        if(flgSW2) {
	        flgSW2=0;
	        DoSwitch2();
    }
}

The last } in the left column is misleading - it's not closing main(), it is closing the while(1). That in turn starts because what appears to be closing the while(1) is actually closing the if(flagSW2).

I think this just highlights the need to be rigorous about indentation - the whole point of doing it is to spot things like this.

As I say, if you are lucky enough to be a Linux user you either already have indent or it's a simple "sudo apt-get install indent" away.

Not on an Atmel Studio machine ATM, but IITC it has a "match brace" function. So, starting from, or rather a bit down, placing the cursor at the opening brace of main() and going to the matching brace you would have "fallen down" far beyond what you expected.

Some of my old heuristics for coding in e.g. C:

* If  an error seems totally unrelated to the line it is flagged at, look on the preceing line(s) for a mistake, omission etc.

* For errors that are hard to explain, investigate (as 'clawson' did) if it is related to unbalanced brackets. Several ways to do that:

- Reformatting as 'clawson' did

- Matching bracket hunt as I suggest above

- Temporary removal/commenting of hunks of code - use a divide and conquer strategy

Every hard problem can, by forming a wise strategy and working in a structured way, be reduced to one or several smaller and not so hard problems.

Use a versioning system. Build and commit to it very frequently. Write a few lines of code, build, test, commit. Whenever an error or bug occurs you can easily compare the current and previous version to help you think about what went wrong.

And of course:

* Write well-structured code from the start. The purpose is to help you during the development and maintenance process, so it should be done at all times - NOT as a final fixup before baselining or releasing.

* Write small functions, with one purpose and no dependencies on other code apart from the call/return interface.

* Do not fear using spaces in code (see how much easier the code in 'clawson's formatted version of main() is to read than the original, e.g. the assignment statements.

* Use descriptive variable names, and don't fear writing them out rthr thn usng shrtr abbreviated names. Any modern IDE will help you finish/expand the full name when you start typing it. The underlying fact here is that code is read much more often than it is written. Your goal when writing code is primarily that it should be easy to read, not easy to write.

These are lessons learned from writing software for 35 years. (I still learn every day, but that is one of the blessings of working with writing software. Never a dull moment!)