Bizzare Codewarrior problem

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Bizzare Codewarrior problem

5,532 Views
Seegoon
Contributor I
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
Labels (1)
0 Kudos
9 Replies

665 Views
Lundin
Senior Contributor IV
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.
0 Kudos

665 Views
Seegoon
Contributor I

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

 

0 Kudos

665 Views
CompilerGuru
NXP Employee
NXP Employee
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
0 Kudos

665 Views
Seegoon
Contributor I

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

0 Kudos

665 Views
rocco
Senior Contributor II
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". :smileywink:
0 Kudos

665 Views
Seegoon
Contributor I

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 

0 Kudos

665 Views
Lundin
Senior Contributor IV
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...
0 Kudos

665 Views
kwik
Contributor I
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 ?
0 Kudos

665 Views
Seegoon
Contributor I

Sorry  I forgot to mention the processor.

I'm using the MC9S12c32CFU16 part.

Cheers

Rob

0 Kudos