Buttons don't work

Log in or register to post comments
Go To Last Post
12 posts / 0 new
Author
Message
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
#1
Posted by GertJ: Thu. Feb 24, 2005 - 09:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

I've connected 4 buttons to my avr mega 16 and they should be active low.
The code I use is:

        for(;;)	// Infinite loop
	{			
		
		AvrXDelay(&MyTimer3, 5);
		
		if( bit_is_clear( PORTC , PINC0 ) == 1 )	// check if low
		{
			AvrXDelay(&MyTimer3, 100);    // Dake care of disco stuff
			while( bit_is_clear( PORTC , PINC0 ) == 1 ) // Wait until released
			{
				AvrXDelay(&MyTimer3, 1);
			}

			AvrXSendMessage(&button,&right);
			AvrXWaitMessageAck(&right);
                 }
	
                 if( bit_is_clear( PORTC , PINC1 ) == 1 )	// check if low
		 {
			AvrXDelay(&MyTimer3, 100);    // Dake care of disco stuff
			while( bit_is_clear( PORTC , PINC1 ) == 1 ) // Wait until released
			{
				AvrXDelay(&MyTimer3, 1);
			}

			AvrXSendMessage(&button,&right);
			AvrXWaitMessageAck(&right);
                 }

                 if( bit_is_clear( PORTC , PINC2 ) == 1 )	// check if low
		{
			AvrXDelay(&MyTimer3, 100);    // Dake care of disco stuff
			while( bit_is_clear( PORTC , PINC2 ) == 1 ) // Wait until released
			{
				AvrXDelay(&MyTimer3, 1);
			}

			AvrXSendMessage(&button,&right);
			AvrXWaitMessageAck(&right);
                 }

                 if( bit_is_clear( PORTC , PINC3 ) == 1 )	// check if low
		{
			AvrXDelay(&MyTimer3, 100);    // Dake care of disco stuff
			while( bit_is_clear( PORTC , PINC3 ) == 1 ) // Wait until released
			{
				AvrXDelay(&MyTimer3, 1);
			}

			AvrXSendMessage(&button,&right);
			AvrXWaitMessageAck(&right);
                 }
         }

Now I've defined portc all inputs. And I've tested all buttons so that they work as they should but... How come only pinc0 works and no other pins? I've even tried this on porta and portd and it makes no difference.

Thank you very much.

/Gert

Tags:

AVR Microcontrollers, megaAVR and tinyAVR
Top
glitch
glitch's picture
Level: Raving Lunatic
Joined: Sat. Jan 12, 2002
Posts: 8710 View posts
Location: Canada
Posted by glitch: Thu. Feb 24, 2005 - 10:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

this appears to be incorrect

if( bit_is_clear( PORTC , PINC0 ) == 1 )   // check if low

you should be referencing PINC not PORTC

if( bit_is_clear( PINC , PINC0 ) == 1 )   // check if low

and again here

while( bit_is_clear( PORTC , PINC0 ) == 1 ) // Wait until released

should be

while( bit_is_clear( PINC , PINC0 ) == 1 ) // Wait until released

the way you've written it here, none of the routines will work correctly. they will all enter into the 'if' statement (assuming that the PORTC bit was last written to 0), and get stuck forever in the while loop, unless some ISR writes to the PORT register to change teh state of teh pull-up.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Top
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
Posted by GertJ: Thu. Feb 24, 2005 - 10:10 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello again,

Yeah, your right! I forgot to change that back.
It does look like that already and it only works on the pinc0

/Gert

Top
glitch
glitch's picture
Level: Raving Lunatic
Joined: Sat. Jan 12, 2002
Posts: 8710 View posts
Location: Canada
Posted by glitch: Thu. Feb 24, 2005 - 10:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

do you have the internal pull-ups enabled, or some external pull-up on the line, to pull the state high when the switch is open?

try writng 0xFF to PORTC at the top of this block, to see if it fixes the problem.

Also note that in all four cases you appear to take the exact same action. You do not have any changes to your messages, so all four buttons will generate the exact same result.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Top
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
Posted by GertJ: Thu. Feb 24, 2005 - 10:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, I have external pull-ups and I've used the same set before so there shouldn't be no problem. And I've done some measurements on them to test them out as well. They do work as they should.

>>try writng 0xFF to PORTC at the top of thsi block, to see if it fixes teh problem.

Do you meen I should do a
outp(0xff,PORTC); .....?
Why do this?

/Gert

Top
glitch
glitch's picture
Level: Raving Lunatic
Joined: Sat. Jan 12, 2002
Posts: 8710 View posts
Location: Canada
Posted by glitch: Thu. Feb 24, 2005 - 10:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

To enable the internal pull-ups. But if you do indeed have external pull-ups it wont make any difference.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Top
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
Posted by GertJ: Thu. Feb 24, 2005 - 10:42 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ok! Thanks anyway.

If I'd put the
outo(0xff,PORTC);
in the code to enable the internal pull-ups, where in the code should I put it?

/Mr G

Top
NogginGTS
NogginGTS's picture
Level: Hangaround
Joined: Mon. Mar 17, 2003
Posts: 274 View posts
Posted by NogginGTS: Thu. Feb 24, 2005 - 10:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

PORTC = 0xFF is generally regarded as a better than outp(0xff,PORTC);

Also, does bit_is_clear expect a bitmask or a shift value? I'm guessing a shift value, so what is PINC0 PINC1 PINC2 etc defined as? 0, 1, 2, 3, 4, .... or 0x01, 0x02, 0x04, 0x08, 0x10

try changing
if( bit_is_clear( PORTC , PINC0 ) == 1 ) to if ( ~PINC & (1<<0))
if( bit_is_clear( PORTC , PINC1 ) == 1 ) to if ( ~PINC & (1<<1))
if( bit_is_clear( PORTC , PINC2 ) == 1 ) to if ( ~PINC & (1<<2))

Top
donblake
donblake's picture
Level: Posting Freak
Joined: Sat. Jan 3, 2004
Posts: 1569 View posts
Location: Binghamton, NY, USA
Posted by donblake: Thu. Feb 24, 2005 - 11:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Does the bit_is_clear function return 0 or 1 or does it return zero or non-zero?

Instead of:

if ( bit_is_clear( PORTC, PINC0 ) == 1 )

I'd use

if ( bit_is_clear( PORTC, PINC0 ) != 0 )

Don

Top
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
Posted by GertJ: Fri. Feb 25, 2005 - 11:34 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello again,

NogginGTS wrote:

Also, does bit_is_clear expect a bitmask or a shift value? I'm guessing a shift value, so what is PINC0 PINC1 PINC2 etc defined as? 0, 1, 2, 3, 4, .... or 0x01, 0x02, 0x04, 0x08, 0x10

Well, I guess you're right, it's a shift value. All is defined inputs.

I tried the changes you suggested...

NogginGTS wrote:

try changing
if( bit_is_clear( PORTC , PINC0 ) == 1 ) to if ( ~PINC & (1<<0))
if( bit_is_clear( PORTC , PINC1 ) == 1 ) to if ( ~PINC & (1<<1))
if( bit_is_clear( PORTC , PINC2 ) == 1 ) to if ( ~PINC & (1<<2))

...and it works like a charm. Thank you very much.

Thanx everybody for your time and quick responds.

Peace
-Gert

Top
NogginGTS
NogginGTS's picture
Level: Hangaround
Joined: Mon. Mar 17, 2003
Posts: 274 View posts
Posted by NogginGTS: Fri. Feb 25, 2005 - 02:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One more change that I'd suggest, between the two lines

AvrXDelay(&MyTimer3, 100);    // Dake care of disco stuff
while( bit_is_clear( PORTC , PINC0 ) == 1 ) // Wait until released

you may need a 20-30ms delay, unless AvrXDelay is intended for debounce. Otherwise you could end up with a button being hit multiple times.

Top
GertJ
GertJ's picture
Level: Rookie
Joined: Mon. Nov 15, 2004
Posts: 33 View posts
Location: Sweden
Posted by GertJ: Fri. Feb 25, 2005 - 04:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok thanx!

This is how it looks now. I changed the
~PINC & (1<<0)
to
~PINC & 0x01
Something I had a discussion aboud with a friend. It not only works the same way but is more clear.

                if( ~PINC & 0x01 )	// Right button pressed?
		{
			AvrXDelay(&ButtonTimer, 20);	// Don't care for disco
			while( ~PINC & 0x01 )		// Wait until button released
			{
				AvrXDelay(&ButtonTimer, 20);
			}

			AvrXSendMessage(&button,&right);	// Send message
			AvrXWaitMessageAck(&right);		// Wait ack!
		}
Top