Bizzare Codewarrior problem

[Seegoon]

Hi to all.

I have written some simple code that is giving me some weird results.

I am using Codewarrior ver 5.5.1272.

The code has 2 main parts. A 1ms timer and a main function.

Here is the main function:

Code:

void main(void){  /*** Processor Expert internal initialization. DON'T REMOVE THIS CODE!!! ***/  PE_low_level_init();  /*** End of Processor Expert internal initialization.     ***/    DDRB_BIT6 = 1;      // set portB bit 6 to output (light outout)  DDRB_BIT7 = 1;      // set portB bit 7 to output (status_led)  DDRA_BIT6 = 0;      //set portA bit 6 to input  gate_open_flag = 0; light_on_delay = 20;  while(1)    {          if(light_trigger==0)             //  trigger turns on light      {        counter_1sec = 0;        Light = !Light;        while(light_trigger==0 && ( counter_1sec < 4));    //wait for button to be released or 4 sec to pass                  //TIE_C0I = 0;             if(counter_1sec < 4)                   // button not held down , light on - TIMED          {            timeout_flag = 1;     //enable timeout in 1ms timer module            status_led_mode = 0;          }          else                        // button held down , light on permanently          {              //flashes status led (x) times            status_led_mode = 1;   //set led to flash-- delay 2 sec-- flash            timeout_flag = 0;                }                     while(light_trigger==0);      }//---------------------------------------           if(gate_open_flag && Light == 0)  //gate turns on light for delay time      {         counter_1sec = 0;        Light = 1;       }            if( counter_1sec > light_on_delay && gate_open_flag)      {        gate_open_flag = 0;        Light = 0;      }      //---------------------------------------              }

 
What happens:

The variable counter_1sec is incremented in the 1ms timer every 1000ms. Light_trigger is mapped to portA bit 6.

When light_trigger is held low one of 2 things happen:

If it is held low for less than 4 seconds the IF part of the if..else statement executes. If it is held low for longer than 4 seconds the ELSE statement executes. Everything works fine up till here.The   //TIE_C0I = 0;   instruction turns off the 1ms interrupt. I put this in there to help me step through the code with the debugger (P&E USB multilink )without continuously jumping to the timer interrupt.

The main problem happens on the line:   " status_led_mode = 1; " in the ELSE statement.

Before this line the value of the status_led_mode variable is 0.

After this line executes the value is 5 , not 1 as you would expect. I have stepped through this code many times with the same result.

Now if I change the instruction to "  status_led_mode = 2;  " or any number  other than 1 it works correctly.

Any idea what may be going on here.

I have noticed that the assembler that is generated is different if I use the value 1 as opposed to any other number . 

It looks like the compiler is performing some sort of optimization somewhere.

I have tried creating the project from scratch with Processor expert (all default settings) with the same result.

Any help appreciated as I'm really stumped.Cheers

Rob

[Lundin]

And what is "status_led_mode" then? An integer variable? Some port pin?

As a sidenote, it is good practice to set all unused pins to outputs in DDR registers, and to set PUCR to a value that makes sense to the application. PORTA and PORTB have no pull-ups activated out of reset, so if those pins are set as inputs and left unconnectet on the circuit board, the result will be random.

[Seegoon]

Status_led_mode is a variable. An insigned int. Sorry it's declared higher up in the code which I have not shown.

I have been doing some experimenting and this is what I have found. Code has been stripped down to this.

Code:

 while(1)    {          if(light_trigger==0)             //  trigger turns on light      {    //    counter_1sec = 0;        Light = !Light;        while(light_trigger==0 ) ;        TIE_C0I = 0;             if(counter_1sec < 4)                   // button not held down , light on - TIMED          {            timeout_flag = 1;     //enable timeout in 1ms timer module            status_led_mode = 0;          }          else                        // button held down , light on permanently          {              //flashes status led (x) times            status_led_mode = 1;   //set led to flash-- delay 2 sec-- flash            timeout_flag = 0;                }                     while(light_trigger==0);      }

I have found this:

If I comment out this line  counter_1sec = 0;  then the code works like it should.

Status_led_mode  always gets a value of 1 in the ELSE statement

If I leave it in , the value of Status_led_mode  becomes one more than counter_1sec( which is an unsigned int). ie If I hold the light_trigger input low for 15s the value in status_led_mode becomes 16.

Still confused :0(

P.S Thanks for the sidenote.

Cheers

Rob

 

[CompilerGuru]

is light_trigger and all other variables which are accessed by both the interupt handler and the
main code volatile?
I did see in the past really strage effects of a missing volatile, effects which are not obvious to humans like me, just to compilers :smileyhappy:
In the end, I think we would need complete, compilable code.

Daniel

[Seegoon]

Hi there.

Using volatile has solved my problem:0)  I did try that before but it did not seem to work. Maybe there was another problem that I was not aware of. I was under the impression that you should use the 'volatile' keyword on registers that may be changed at random by external events unknown to the compiler.

Does this mean that I should declare EVERY variable as volitile just in cast the compiler decides to optimize it in some way?

Thanks again for the help.

Cheers

Rob

[rocco]

Hi, Seegoon:

---

I was under the impression that you should use the 'volatile' keyword on registers that may be changed at random by external events unknown to the compiler.

---

But any variable that is modified in an ISR will "be changed at random by external events unknown to the compiler".

[Seegoon]

Ha , makes sense.

Thanks for the help guys.This is something very easy to miss and could cause endless problems for the unwary. I'm lucky this was a small/simple piece of code!!

Cheers

Rob 

[Lundin]

The problem is rather that Codewarrior has optimization enabled by default and lacks a single switch for turning all optimization off. Every other compiler I know of, for every platform, has the optimizer disabled by default and has also got a compiler switch "disable/enable optimizer".

I remember when I once thought I found a bug in the optimizer. Metrowerks couldn't find the cause of the problem (which I later found, it was one of the optimizing switches that made the code behave wrongly).
But they couldn't find the cause and suggested that I should use volatile, because then the optimizer containing the possible bug wouldn't be used...

[kwik]

Another thing ; This has probably nothing to do with your problem ,but I wouldnt write this ;

while(light_trigger==0);

I think its cleaner to write

while(light_trigger==0){}

But its perhaps just a matter of taste ?

[Seegoon]

Sorry  I forgot to mention the processor.

I'm using the MC9S12c32CFU16 part.

Cheers

Rob