Hi again everybody, Can someone help me with this code?

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

Hi again my friends,
Could someone please help me out with this code?

I am using an Atmega32 & I have a switch connected to INT_0 or PD2 as my ext interrupt and also have both of Port A and C connected to LEDs (had also disable Jtag so no worries for Ports c :) )

However whe i run this code no change on LEDs they just keep shifting like they suppose (according to the code)

Could some one clever out there spot the cause?

Again many thanks

the code is below

[

/*
 * Created: 7/02/2012 8:55:49 p.m.
 *  Author: Kase_57
 */ 


#include 
#include 
#include 
#include 
#include 

volatile bool flag;

int main(void)

{

	flag = false;
	
	cli();
	
	DDRA = 0xFF;	
	PORTA = 0x00; 
	DDRC = 0xFF;  
	PORTC = 0x00; 
	DDRD = 0x00;
	//PORTD = 0xFF;
	
	MCUCSR |= 1<<JTD; //disabling Jtag just incase of Port C
	MCUCSR |= 1<<JTD;
	
	
	sei();
	
	GIFR = 0x00;
	MCUCR = (0< ISC01) | (0<<ISC00) ;
	GICR |= 1<< INT0; 
	
	
	TCCR1B |= 1<<CS10 | 1<<CS11;
	
	int Led_num[2];
	
	
    while(1)
    {
	  
	  do 
	  {
   
	 if(TCNT1 > 2232)
        {
	     TCNT1 = 0;
		 PORTA = 1<< Led_num[0];  
		 Led_num[0] ++;
		                            
		 if (Led_num[0] >6)       
		   {                      
		    Led_num[0] = 0;
			PORTC = 1<< Led_num[1];
			Led_num[1]++;
			
			if (Led_num[1] >6)
			  { 
			   Led_num[1] = 0;
			   }
		   } 
		 }  flag = false; 
		 
	   }  while (flag);
	 
	}   
}

ISR (INT0_vect)

{
   
 flag = true;
 
 }

]

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

does anyone know how to write the code (when making a comment) so it's visible
like is in compiler?
I think it's a lot more visible too.

Cheers.

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

Quote:

does anyone know how to write the code (when making a comment) so it's visible
like is in compiler?

Assuming you want to keep indentation and stuff in code when you post it at AVRFreaks, look for tip # 1 here: https://www.avrfreaks.net/index.p...

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 int Led_num[2];

You never initialize this array before using it.

    if(TCNT1 > 2232)
        {
        TCNT1 = 0;

Why are you not using CTC mode for this?

Regards,
Steve A.

The Board helps those that help themselves.

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

reformatting so the indentation makes sense:

while(1)
{
    do
    {
        if(TCNT1 > 2232)
        {
            TCNT1 = 0;
            PORTA = 1<< Led_num[0]; 
            Led_num[0] ++;
                                 
            if (Led_num[0] >6)       
            {                     
                Led_num[0] = 0;
                PORTC = 1<< Led_num[1];
                Led_num[1]++;
         
                if (Led_num[1] >6)
                {
                    Led_num[1] = 0;
                }
            }
        }  
        flag = false;
    }  
    while (flag);
}   

Regardless of what your LED's do, at the end of each loop you clear flag and exit the loop. Pushing the button will set the flag, but nothing else changes. When you get to the end of the loop, you clear flag and exit the loop. This behavior will never change with the way you wrote this code.
What do you want it to do when you press the button?

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

@ dksmall

Thanks for your reply, i only want that code (within the LEDs) to only be executed when the flag is true ie (when the button is pressed on PD2)

you have said " at the end of each loop you clear flag and exit the loop."

But i don't see anything wrong with this code and i am ONLY clearing flag at the end of the Do ... while loop ( just after LEDs code is finished being executed) which actually makes sense to me since this will happen again only is the button is being pressed again (thus interrupted) isn't it?

@ Koshchi

" if(TCNT1 > 2232)
{
TCNT1 = 0;
Why are you not using CTC mode for this? "

I know this code is probably not very effective but i will surely make it, after this interrupt issue which was my main concern

Could someone tell me why the button press on PD2 isn't interrupting the main routine ? Could it be that i need to assign somewhere flag == true rather than =true ? i know this is also confusing but the first one means "is" and the 2nd one of just one = means "equal to"

the button press is hooked up to the ground and the other end to PD2.

Many Thanks all.

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

Actually I think although the while(1) loop carries on forever, it doesn't meant to be executing the code within Do....while loop until flag is true isn't it? and the flag will also be only if the button was being pressed (And after the Do .. while loop finishes, it reset the flag back to 0, to wait until the button is being pressed again)
unless i am missing something ?

Cheers.

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

I have also tried to see if the problem within the code was (about interrupts or just within main itself) by adding this code

 
          while (PIND & (1<<PD2))
            { 
               flag = true;
	      }

Right in the begining of whithin while(1) loop and it works perfectly ie (the Do .. while loop is only being executed when the button is pressed)

However without ofcourse adding this code it doesn't work when i get an ISR to do set the flag on :/
which clearly shows that the error lies whithin my ISR
Could anyone help me out to spot where the error is?
This is my first time to use ISR unfortunately so i don't have many clue about it :/

Again Many thanks everybody for your contribution

Cheers

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

The first time through the do loop, the loop gets executed, it doesn't look to the terminating "until" condition until all the code inside the do loop gets executed. So everytime the while loop loops, the do loop also runs.

while(1)
{
  while(flag)
  {
     do LED stuff
  }
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Actually I think although the while(1) loop carries on forever, it doesn't meant to be executing the code within Do....while loop until flag is true isn't it?
The whole point of a do-while loop (as opposed to a while or a for loop) is that it will always be run at least once each time it is run.

Regards,
Steve A.

The Board helps those that help themselves.

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

Whoau... I haven't even test this yet but I am going to give it a try!

then why would

Do
{
//code
} while (condition is met)

execute the code inside it without having checked 1st what are the conditions to run that "while" loop?

will this condition matters anymore if the statement is executed without reaching the while condition anyway ?

I am not arguing tho! in facts i am going to give it a try and have a look! but it doesn't make sense to me at all that the code inside the do .. while loop would be execute before the compiler checks what is the conditions (within while loop) first.
(I don't use it much anyway, and that's probably why it is causing the confusion)

Thank you everyone.
I really appreciate your suggestion.

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

Quote:

then why would

Do
{
//code
} while (condition is met)

execute the code inside it without having checked 1st what are the conditions to run that "while" loop?


Because that is how the C language defines it. The do-while tests the condition after the body has been executed, not before.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

...and you also have

Quote:
while (condition is met)
{
//code
}

...so you can choose:
- 0 or more iterations, condition checked before code (while... version)
- 1 or more iterations, condition checked after code (do... version)

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

Thanks everybody for all your contributions
it helped alot :)

Cheers

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

Now add this to your list of essential web links: http://publications.gbdirect.co....

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]