I/O problem with code

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

Hi guys, Ive been trying around now for hours to find a working solution for this code here. (Ive got 2 schmitt triggers connected to PD7 and PD6 (pin 13 and pin12 on Atmega8, the goal is, if you press on any of the switches, the LCD should display the name of the switch that was pressed on the screen, and if pressed again, it would remove that name.)

The code (does not belong to me originally so i take no claim here in any form or shape) works perfectly, and i mean PERFECTLY for one single switch, but as soon as i try to copy paste the code for the second trigger, nothing works!

Could you perhaps take a minute and have a look through it? I would appreciate any help:

#include 
#include 
#include "lcd-routines.h"
#include 

/* 
* define a threshold for debouncing
* the value depends on the mcu's clock speed
* greater the speed, higher the threshold
*/
#define DEBOUNCE_THRESHOLD 200

uint16_t count = 0;
int health=20;
int pressed=0;


int main(void)
{

	lcd_init();		
	set_cursor(0,1);
	lcd_string( "Welcome");
	
	
	
/* Configure PD6 and PD7 as input */
    
	DDRD = ~_BV((PD6)|(PD7));

/* Enable pullup on PD6 and PD7*/
	PORTD |= ((1<<PD7)|(1<<PD6));

	
    
	pressed = 0;

   
    for(;;)
    {
	 
		/* check if PD6 was pressed */
		if( PIND & _BV(PD6) )
        {
            count = 0;
            
            /* yes, but for how long? */
            while( PIND & _BV(PD6) )
			{
                count++;
			}
            
            /* if count is greater the threshold value, intentional press encountered*/
            if( count >= DEBOUNCE_THRESHOLD )
			{
            
				if(pressed==0)
					{
						set_cursor(0,2);
						lcd_string( "PD6");
						pressed = 1;
						
					}
			
					else
					{
						set_cursor(0,2);
						lcd_string( "      ");
						pressed = 0;
						
					}
			}
            
        }




		
		/* check if PD7 was pressed */
        else if( PIND & _BV(PD7) )
        {
            count = 0;
            
            /* yes, but for how long? */
            while( PIND & _BV(PD7) )
			{
                count++;
			}
            
            /* if count is greater the threshold value, intentional press encountered*/
            if( count >= DEBOUNCE_THRESHOLD )
			{
            
				if(pressed==0)
					{
						set_cursor(0,2);
						lcd_string( "PD7");
						pressed = 1;
						
					}
			
					else
					{
						set_cursor(0,2);
						lcd_string( "      ");
						pressed = 0;
						
					}
			}
            
        }
        
		
		
		
		
    }
    
    return 0;
}

So the above does not work, but if you just modify the following:


DDRD = ~_BV((PD6));

meaning that you removed the PD7 from the above line, the code works PERFECTLY for the switch at PD6, when pressed.

What am i doing wrong?

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

Quote:
PORTD = ~_BV((PD6)|(PD7));
This will not set PD6 and PD7 to 0.

It should be

Quote:
PORTD = ~(_BV(PD6)|_BV(PD7));

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

Right, testing it out!

I think that this is what you meant so I modified the code with the following:

    DDRD = ~(_BV(PD6)|_BV(PD7)); 

Didn't do it... still the same problem.

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

You do know that _BV(PD6) is the same as (1 << PD6) don't you.

Regards,
Steve A.

The Board helps those that help themselves.

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

You set pullups on PD6,PD7.
So I guess your switches are active low.

But you test them for high.

if( PIND & _BV(PD6) )
while( PIND & _BV(PD6) )

It should be

if(!( PIND & _BV(PD6) ))
while (!( PIND & _BV(PD6) ))

Edit:

      while( PIND & _BV(PD7) )
         {
                count++;
         } 

"count" increments every 7 cycles here.
As "count" is 16-bit, it overflowes each cca 45 ms if clock is 10 MHz.
So after you release the switch, the value of "count" is random in fact.
I would declare "count" as 32-bit.

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

My switches are connected this way:

I tried the ! variant but it also didnt work. When i tried your code

if(!( PIND & _BV(PD6) ))
while (!( PIND & _BV(PD6) )) 

for PD6 and left PD7 without the !, then suddenly PD7 started to work, but PD6 didnt. (Originally, when no ! are in, only PD6 works, but not P7)

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

Using Uint32_t for count, makes the code more unreliable. When PD6 switch is pressed, it doesnt register sometimes, whereas uint16 makes sure that every push on the switch of PD6 registers perfectly.

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

Your system looks at the duration of the key-press. You need a 'long enough' press to register. But if you hold the key down for a long time then the 'count' will rollover around zero.

Using a 'uint32_t' for 'count' will just ensure that the 'count++' loop will take longer to execute. Which meads a short key-press may not register. It does however ensure that a monster key-press is less likely to rollover.

Either way, you need to fine tune the values for your compiler. In years to come, code like this comes back to bite you. If you use a Timer then you just decide a 50ms threshold or whatever.

David.

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

I understand your point david. The problem is, that whenever i change the uint16 for a unit32, the system becomes unreliable. Its totally illogic, and against all expectations which indicates that the code is perhaps inherently faulty?

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

1.
According to your picture your switches are active high on PD6,7.
So your code for testing is right.

if( PIND & _BV(PD6) )
while( PIND & _BV(PD6) )

2.
Try this code with another way of debouncing

#include 
#include 
#include "lcd-routines.h"
#include 
#include 

uint8_t pressed=0;


int main(void)
{

/* Configure PD6 and PD7 as input */
   DDRD = ~((1<<PD6)|(1<<PD7));

   lcd_init();      
   set_cursor(0,1);
   lcd_string( "Welcome");
   
   pressed = 0;

   
    for(;;)
    {
   
/* debounce PD6 */
      if( PIND & _BV(PD6)) _delay_ms(50);
      if( PIND & _BV(PD6))
      {

/* wait for switch released */
         while( PIND & _BV(PD6));
           
         if(pressed==0)
         {
            set_cursor(0,2);
            lcd_string( "PD6");
            pressed = 1;
         }
         else
         {
            set_cursor(0,2);
            lcd_string( "      ");
            pressed = 0;
         }
            
      }//if(PIND



/* debounce PD7 */
      if( PIND & _BV(PD7)) _delay_ms(50);
      if( PIND & _BV(PD7))
      {

/* wait for switch released */
         while( PIND & _BV(PD7));
           
         if(pressed==0)
         {
            set_cursor(0,2);
            lcd_string( "PD7");
            pressed = 1;
         }
         else
         {
            set_cursor(0,2);
            lcd_string( "      ");
            pressed = 0;
         }
                       
      }//if(PIND
       
    }//for(;;)
   
}//main()
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Point 1; When someone like david.prentice tells you that something is wrong, and you change that line of code, and it still doesn't work, what do you do next?
a) Put it back how it was and assume d.p knows diddly squat.
b) Leave the suggested modification and assume there may be additional problems with your code. (hint: The answer should usually be b)

Point 2; If you are using 74HC14 devices the pull-ups are largely irrelevant.

Point 3; What david tells you about overflow is also true. The code that increments count should check for an upper limit, and saturate count to this value.

Point 4; IMHO, one of the first things you should do is find a way to display values on the LCD, that way you can see what's happening with count. The "stare-the-code-into-submission" rarely works. Sometimes it's all you have, but in this case you have an LCD. If it's this hard to read a simple switch, how hard will it be to build that remote controlled operating theatre robot?

Four legs good, two legs bad, three legs stable.

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

Quote:
Try this code with another way of debouncing

I tried this code it didnt work unfortunately. Thanks for trying Visovian.

Quote:
One of the first things you should do is find a way to display values on the LCD

I tried to visualize the value of count with this code:


char biffer[];
....
if(pressed==0)
         {
            itoa(count, biffer);
            set_cursor(0,2);
            lcd_string( biffer );
            pressed = 1;
}

but all i get is jibberish on the lcd screen...

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

Ok, perhaps ill just give up with my "own" code and jump over to Dannis:

/************************************************************************/
/*                                                                      */
/*                      Debouncing 8 Keys				*/
/*			Sampling 4 Times				*/
/*                                                                      */
/*              Author: Peter Dannegger                                 */
/*                                                                      */
/************************************************************************/

#include 		// need "--std=c99"


typedef unsigned char	u8;
typedef signed short	s16;

#define	XTAL		8e6		// 8MHz

#define KEY_PIN		PINB
#define KEY_PORT	PORTB
#define KEY_DDR		DDRB
#define KEY0		0
#define	KEY1		1

#define LED_DDR		DDRD
#define LED_PORT	PORTD
#define LED0		0
#define	LED1		1
#define LED3		3
#define LED4		4
#define LED5		5
#define LED6		6
#define LED7		7


u8 key_state;				// debounced and inverted key state:
					// bit = 1: key pressed
u8 key_press;				// key press detect

u8 ct0 = 0xFF, ct1 = 0xFF;		// internal debouncing states


ISR( TIMER1_COMPA_vect )
{
  u8 i;

  i = key_state ^ ~KEY_PIN;		// key changed ?
  ct0 = ~( ct0 & i );			// reset or count ct0
  ct1 = ct0 ^ (ct1 & i);		// reset or count ct1
  i &= ct0 & ct1;			// count until roll over ?
  key_state ^= i;			// then toggle debounced state
  key_press |= key_state & i;		// 0->1: key press detect
}


u8 get_key_press( u8 key_mask )
{
  ATOMIC_BLOCK(ATOMIC_FORCEON){
    key_mask &= key_press;		// read key(s)
    key_press ^= key_mask;		// clear key(s)
  }
  return key_mask;
}


void display_debounce_key0( void )	// demonstrate debounce working
{
  u8 i;

  ATOMIC_BLOCK(ATOMIC_FORCEON){
    i = ct0 & 1<<KEY0;			// collect the two bits for KEY0
    if( ct1 & 1<<KEY0 )			// from vertical counter
      i |= 2;
  }
  switch( i ){				// LED7..4 = debounce counter for KEY0
    case 0:
	      LED_PORT &= ~(1<<LED4);
	      LED_PORT |= 1<<LED5;
	      LED_PORT |= 1<<LED6;
	      LED_PORT |= 1<<LED7;
	      break;
    case 1:
	      LED_PORT |= 1<<LED4;
	      LED_PORT &= ~(1<<LED5);
	      LED_PORT |= 1<<LED6;
	      LED_PORT |= 1<<LED7;
	      break;
    case 2:
	      LED_PORT |= 1<<LED4;
	      LED_PORT |= 1<<LED5;
	      LED_PORT &= ~(1<<LED6);
	      LED_PORT |= 1<<LED7;
	      break;
    case 3:
	      LED_PORT |= 1<<LED4;
	      LED_PORT |= 1<<LED5;
	      LED_PORT |= 1<<LED6;
	      LED_PORT &= ~(1<<LED7);
	      break;
  }
  if( key_state & 1<<KEY0 )		// LED3 = debounced state
    LED_PORT &= ~(1<<LED3);
  else
    LED_PORT |= 1<<LED3;
}


int main( void )
{
  TCCR1A = 0;
  TCCR1B = 1<<WGM12			// Mode 4: CTC
	 ^ 1<<CS02^1<<CS00;		// divide by 1024
  OCR1A = XTAL / 1024.0 * 1 - 1;	// 1s
  TCCR1C = 0;
  TIMSK1 = 1<<OCIE1A;			// enable T1 interrupt

  KEY_DDR = 0;				// input
  KEY_PORT = 0xFF;			// pullups on
  LED_PORT = 0xFF;			// LEDs off (low active)
  LED_DDR = 0xFF;			// LED output
  key_state = ~KEY_PIN;			// no action on keypress during reset
  sei();

  for(;;){					// main loop
    display_debounce_key0();

    if( get_key_press( 1<<KEY0 ))	// LED0 = toggle on keypress
      LED_PORT ^= 1<<LED0;

    if( get_key_press( 1<<KEY1 ))	// LED1 = toggle on keypress
      LED_PORT ^= 1<<LED1;
  }
}

Does anyone know what kind of hardware i should use with this?

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

and from where do io get atomic.h?

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

Quote:

and from where do io get atomic.h?

Your AVR-LibC doesn't already have this??

http://www.nongnu.org/avr-libc/u...

(what, read the user manual? am I mad?)

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

We illiterate manual ignorers, we! :D

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

angrysurgeon wrote:

Does anyone know what kind of hardware i should use with this?

Uh, buttons and leds with ports described in the code and an AVR model that has 16-bit timer1.

Or in other words, you should change the code to what hardware and AVR model you have, not the other way around.

You know, button debouncing is not black magic, and like everything else, it requires a bit of thinking how it works. Or if you want to code random lines one after another, you could at least do us a favor and use AVR Studio Simulator to run your code, so can see what it does.

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

Returning to your original code. I thought that I would simulate it on a 8MHz Mega32.

With -Os optimisation, the 'count++' loop takes 1us so your threshold is a 200us keypress. And if you hold the key down for a whole 66ms the count will rollover.

Using a uint32_t 'count' takes 1.5us giving a threshold of 300us for the keypress. If you hold the key down for 4295 seconds (71 mins) it will rollover.

Removing optimisation does not make that much difference.

I am probably older than you, with a slower reaction time. So I cannot always get a reliable keypress that fits inside your 200us to 66000us window.

David.

Edit. Yes. Both PD& and PD6 switches 'toggle'. I assume that you had corrected the:

   DDRD &= ~((1<<PD6)|(1<<PD7));  // ensure inputs
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the optimization David.

Quote:
It requires a bit of thinking how it works.

Of course, like all other things, a bit of knowledge is required. But the thinking behind "my" original debouncing code was not (in theory) that bad now, was it?

Anyway, i'm new to AVR programming and Danni's code looks very intimidating to me. A couple of questions regarding the code: To which pin on portD do i need to connect the switch to toggle the leds? Would this switch work?

To port the code to an Atmega8, you said that i needed to change the timer syntax of Danni's code, since its written for a 32bit AVR. Does anyone have a good porting documentation for Atmega8 and Atmega16 (a pdf file), like the one that exists for AT90S2313 and ATTINY2313? Or the one that helps you substitute Atmega8 with Atmega88 http://www.atmel.com/dyn/resources/prod_documents/doc2553.pdf

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

Your earlier circuit would be fine . (with or without the 74HC14).
You already use the internal pull-ups, so your other sketch would work too. I would probably use a 1k safety resistor to GND.

Porting AVR code from a Tiny13 to a Mega2560 generally requires some checking of the data sheet. But most PORT and BIT names will stay the same.

Looking at Danni's code, it is things like the TCCR1C register that will be different. Danni's project will say which AVR he used. Try compiling it for the Mega8, and copy-paste the error messages. Read BOTH data sheets and compare the offending register descriptions.

Then ask here if you are unsure.

Your original code should work fine anyway. If you have a problem, it is probably hardware related.

Your first switch circuit has a 1600us risetime and 200us falltime. (or squared up after the inverter)

You can toggle an LED on any available pin. Just remember that if you share pins on a PORT, you need to |= and &=~ the individual pins.

David.

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

Hmmm, a hardware problem you say. Ok, ill rip apart everything from the breadboard and start anew. Will let you know if it worked.

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

Right, i redid everything, everything was the same. So i put a voltmeter at PD6 and PD7 to monitor things. Now, with the original above code, PD6 is 0V but PD7 was 4.8V at startup of the Atmega8.

Now, what could be wrong? Could this mean that i shouldnt pullup portD? and use pulldown instead since im "pulsing" the ports with 5V from the schmitt trigger?

Sorry for my newbie questions...

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

angrysurgeon wrote:
Right, i redid everything, everything was the same. So i put a voltmeter at PD6 and PD7 to monitor things. Now, with the original above code, PD6 is 0V but PD7 was 4.8V at startup of the Atmega8.

Now, what could be wrong? Could this mean that i shouldnt pullup portD? and use pulldown instead since im "pulsing" the ports with 5V from the schmitt trigger?

Sorry for my newbie questions...

If you use the schmitt trigger, it is in charge of driving the AVR pins hard. Wimpy AVR pullups have no effect.

So if something is wrong, it it not the AVR but the schmitt.

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

From your 74C14 schematic, when the switches are open, the 74C14 i/p's are both 5V. So the o/p's are both at 0V. That means both PD6 and PD7.

Normally with an AVR, you just enable internal pull-ups and take the switch from the pin to GND. No resistors, no capacitors. Debounce the switch in software.

Of course any switch with trailing wires, industrial environment, 'r' in the month ...

It may be safer to use transorbs, RC, limiting diodes ...

But seriously in an office environment none of this is necessary. And from my first paragraph, my conclusion is that you have a hardware problem. The PD7 switch is shorted. Which accounts for your original complaint that PD7 does not work.

David.

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

Quote:
So if something is wrong, it it not the AVR but the schmitt.

Could it be that ive burnt pd 6 and pd7 of the avr using the schmitt trigger constantly? Could this be the reason why pd6 = 0V and pd7 is 4.8V when ive enabled internal resistances all over port D with

PORTD |= ((1<<PD6)|(1<<PD7)...

Quote:
And from my first paragraph, my conclusion is that you have a hardware problem. The PD7 switch is shorted. .

(This is a question out of despair)

But how can that be? Its impossible, since nothing is in contact with PD7. Im using a single (schmitttrigger) switch which i move back and forth between pd6 and pd7...

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

Right, i figured out why pd6 was 0V while pd7 was 4.8V while portD resistors was pulled up. This happens as soon as you connect the schmitt trigger to the pin and take a voltage measurement. Without the schmitttrigger, the pd is high.

So david was right, this is a hardware problem. Omitting the schmitt trigger now.

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

The circuit posted more then likely may not work as intended. There should be no need for the 10K resistor from the switch to ground! You must however use the internal pullup by setting PORTx=1, as you have no external pullup. A pull up is essential.
By leaving the 10K resistor as shown, the voltage at the PORT will be the voltage division that occurs between the internal pull up (approx 68K pullup if I recall) and the 10K resistor.
Whilst OK when the switch is open (as you will get a HI), when the switch is closed you may not get a valid LO.

If you like move the 10K to pull the PORT pin to Vss and you will have a nice "stiff" pullup, rather then the weak internal pullup.

By the way, it is handy to use a Logic probe if unsure. If the logic probe sees a good HI-LO transition then you code has a chance of working, if you don't then it never will.

What is worse, a programmer with a soldering iron or a hardware person with a compiler?

Charles Darwin, Lord Kelvin & Murphy are always lurking about!
Lee -.-
Riddle me this...How did the serpent move around before the fall?

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

I already explained that you don't need the pull-ups if you're using a 74HC14, as it is not an open collector/open drain device. You say "the above original code", does that mean you gone back to your original, faulty code? I think you need to attack this in a logical way, rather than moving from one assumption to another.

Disconnect the 74HC14 outputs from the AVR, and check if they behave as you expect.

Fix your code(correct the BV() errors in the setup and check the count value for an upper limit).

Post the code so that someone can check it for you.

Run the code and see if PD6 and PD7 are both high(assuming you are enabling the internal pull-ups and the 74HC14 is still disconnected).

Four legs good, two legs bad, three legs stable.

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

Thank you all for your input. Ill be doing as you have suggested and will be reporting back with the progress.