Strange Compiling

Go To Last Post
8 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello everyone, this is my first post but I am glad to join other enthusiasts. I am currently implementing an automatic speech recognition algorithm I wrote in Matlab and tranlasted into C. I have been building it in baby steps (evetually moving to UC3) but I just fixed a very strange event in my simple debugging code and was hoping other could tell me if this is normal.

The following runs in the background:

while(1){
		
		if(Addresses.write){
			// how long does this take??
			EEPROM_Write240();
		
		}
		if( (Addresses.two_sec_ADC) ){
			if(!Addresses.write){                /////   WHY IS IT INGORING THISSS!!!!!!!
				if(Addresses.eeprom == ADC_samples){
					lcd_clrscr();
					lcd_puts("Congrats! EEPROM\nat: ");
					itoa(Addresses.eeprom, buffer, 10);
					lcd_puts(buffer);
					_delay_ms(5000);

				}else{
					if(Addresses.timingError){

						lcd_clrscr();
						lcd_puts("Whoops!\nEEPROM:");
						itoa(Addresses.eeprom, buffer, 10);
						lcd_puts(buffer);
						_delay_ms(5000);
					
						lcd_clrscr();
						lcd_puts("There was a\nTIMING_ERR: ");
						itoa(Addresses.timingError, buffer, 10);
						lcd_puts(buffer);
						_delay_ms(5000);
					
						printAddresses();	// print last failed addresses

						lcd_clrscr();
						memset(buffer,' ',7);
						lcd_puts("Address Write:\n");
						itoa(Addresses.write, buffer, 10);
						lcd_puts(buffer);
						_delay_ms(5000);
					}else{
						lcd_clrscr();
						lcd_puts("Whoops!\nEEPROM:");
						itoa(Addresses.eeprom, buffer, 10);
						lcd_puts(buffer);
						_delay_ms(5000);
						lcd_clrscr();
						lcd_puts("Address Write:\n");
						itoa(Addresses.write, buffer, 10);
						lcd_puts(buffer);
						_delay_ms(5000);
					}	
					
				ReadEEPROM();			
				}	
			
			}
		}		
	}
	
}

The following runs in the foreground:

   ISR(ADC_vect){
	   cli();  // avoids stack overflow
	   ADC_counter++;
	   
	   if( (Addresses.ReadEnd > Addresses.microRead) && (ADC_counter <= ADC_samples) ){
		   // new conversion and not filled up ping/pong bank yet
		   *(Addresses.microRead) = ADCH;
		   Addresses.microRead++;
		   Addresses.size++;
	   }else{
		   //switch read write banks and signal that we need to write
		   if(Addresses.write){
			   Addresses.timingError++;  // if we have not finished writing, this is bad
			   saveAddresses();
		   } 

		   Addresses.write = 1;
		   
		   if(Addresses.ping){
			   Addresses.ping = false;
				Addresses.microRead = pong;
				Addresses.ReadEnd = pong + N;
				Addresses.microWrite = ping;
				Addresses.WriteEnd = Addresses.size;  // end of what we just read
		   }else{
			   Addresses.ping = true;
			   Addresses.microRead = ping;
			   Addresses.ReadEnd = ping + N;
			   Addresses.microWrite = pong;
			   Addresses.WriteEnd = Addresses.size;
		   }
		   
		  if(ADC_counter > ADC_samples){
			  
			  Addresses.two_sec_ADC = true;
			  CLEARBIT(ADCSRA,ADEN);  // TURN ADC off
			  TIMSK0 &= ~(1<<TOIE0);  // disable TIMER/COUNTER0 OVF
		   
		  }else{
		  
			   *(Addresses.microRead) = ADCH;
			   Addresses.microRead++;
			   Addresses.size = 1;
		  
		  }
	   }
	   
	   
	   
	   sei();  // re-enable interrupts
   }
   
   ISR(TIMER0_OVF_vect){
	   cli();
	   
	   TCNT0 = 152;		// reset counter for overflow with 104 counts of 256 8-bit counter
	   
	   sei();
   }

And Address.write is changed after writing to EEPROM:

void EEPROM_Write240(void){
	
	EEPROM_WriteBlock(Addresses.eeprom,Addresses.microWrite, Addresses.WriteEnd);

	// signal were done writing
	Addresses.write = 0;
	Addresses.writeComplete++;
}

If I change if( (Addresses.two_sec_ADC) ) to "elseif" then the code completely ignores the second conditional of the Address.write (confirmed by displaying contents on LCD while inside conditioanl). Is this becuase the compiler sees the first "if(Addresses.write)" and if I write "else if(!Addresses.write)" it thinks it is redundant and will remove??? Notice that interrupts can change this value after the first if statement. . . I fixed the issue by just having two "if" statements which is really strange to me.

Thanks!
Redplaya

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Haven't got time for a thorough look, but just from a glance, remove the cli() and sei() statements, and study up on volatile variables a bit.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, remove cli() and sei(), declare your shared variables volatile, and make sure you access shared multibyte variables atomically.

There are plenty of threads about this sort of thing, do a search.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks guys! Yea i should have saw the volatile deal . . . I took out the sei() and cli() for the overflow_vect just because it was so small but I generally always use them as good practice to prevent stack overflow.

I guess I was wondering why both of you were so assert on taking them out?

fyi - you guys already solved the problem so thanks!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The cli()/sei() should be removed because a jump to an ISR on the AVR architecture will automatically disable global interrupts. If you want to nest interrupts ( a bad idea in most cases anyway ) an explicit sei() is required. A return from an ISR is done with reti, which "returns and enables interrupt". So, in a general case, it is not "bad" to have the cli()/sei() pair, but it certainly redundant.

Martin Jay McKee

As with most things in engineering, the answer is an unabashed, "It depends."

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

mckeemj wrote:
in a general case, it is not "bad" to have the cli()/sei() pair, but it certainly redundant.

If the sei() is the last thing you do before returning from the ISR. And even when that is the case it could cause a stack overflow - exactly what the OP thought he was preventing when he put it in there.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

To clarify my last post - calling cli() in an ISR has no effect. Calling sei() does. It reenables interrupts. Hence, if any interrupts are queued up, they will now be handled. If the queued up interrupt's handler also calls sei(), you could very well end up in a situation where you run out of stack.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
evetually moving to UC3
Is this not AVR related then? Should it be in the 32 bits forum?

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly