Debugging help needed

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

There's an intermittent bug, I suspect in the code, but possible a hardware problem.  Was hoping fresh eyes might see something I missed.

 

Target device is an ATtiny85.  Using Atmel Studio 7 on a Windows 7-64 platform if that matters.  This is all built on a breadboard using a tactile board mounted NO button with a 33k pull-down resistor on the input pin.  Perhaps the breadboard's the problem?

 

The program is pretty simple.  There is a single button input which should distinguish between a long and a short press.  Three LEDs are attached to PB2:PB0, but one is a power indicator and always on (PB1).  The other two are toggle to indicate a long or short press of the button. 

 

I realize my debounce method isn't the most effective, but I don't think that is causing the problem.   The problem is that the LEDs only sometimes behave as intended.  The error varies, but this one in particular should be easy to trace.  Namely, a short press results in the long press LED toggling.  Hold the button slightly longer and usually the short press LED lights as intended.  Hold much longer and the long LED lights as intended.

 

Sometimes, I've noticed both LED's toggle together which I don't see how that's possible. 

 

I've tried lots of things.  No optimization.  Lower "KEY_LOCK_PERIOD" values, changing buttons, removing button from breadboard connecting with alligator clips to the button.  There is a cascade portion of code that starts out by assuming the key_status was LONG with subsequent if() to reassign the value dependent upon the key press duration.  Nothing has seemed to affect behavior.

 

I'd appreciate suggestions.

 

/*
 * timer.c
 *
 * Created: 7/19/2016 10:51:59 AM
 * Author : Sherri
 */ 
#define F_CPU 1000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/sleep.h>
#include <util/atomic.h>
#define KEY_LOCK_PERIOD 250UL			// number of milliseconds key is locked out after keypress
#define ISR_TIMER_LEN 976UL				// time between interrupts in micro seconds

// function declarations
void KeyLongPress();
void KeyShortPress();

volatile uint32_t uSec_976_cnt=0UL;		// counter of 976uS segments
volatile uint32_t keydown_ms, keyup_ms;
enum keyStatus {
	NONE,
	SHORT,
	LONG
};

int main(void)
{
	// ---------timer setup--------------
	// USE Timer 0
	// Step #1 set the parameters for the timer TCCR0 registers
	TCCR0A = (1<<WGM01);				//value sets timer to CTC mode
	TCCR0B = (1<<CS02) | (1<<CS00);		// value sets prescaler to clkio/1024
	// Step #2 Choose the number of ticks to compare to the timer
	OCR0A = 0;			// with prescaler set at /1024, each tick will be equal to approximately 976us (1Mhz / 1024)
	// OCR0B is a second value for a second compare if desired.
	// Step #3 Enable the interrupt for the timer OCR0A.  When OCR0A matches the timer, the ISR function will execute
	TIMSK = (1<<OCIE0A);  
	// Step #4 enable the AVR interrupt bit
	sei();
	// Step #5 Add the ISR routine for OCR0A (See below Main module)
	
	// -------------Port setup----------------
	//DDRB&=~(1<<PB3);							// button input
	DDRB = (1<<DDB0) | (1<<DDB1) | (1<<DDB2);	// LED output
	PORTB = (1<<PB1);							// Power indicator LED
	
	// initialize
	uint8_t current_pin_state = 0;			// pin state 0 = open, 1 = closed
	int prev_pin_state = 0;					// previous pin state 0 = open, 1 = closed
	uint32_t key_press_time = 0;
	enum keyStatus key_status = NONE;
	uint32_t key_duration = 0;
	while (1) 
    {	
		current_pin_state = PINB & (1<<PINB3);
		// detect a change in pin from low to high or high to low
		// if pin is low...
		if(!prev_pin_state){
			// test for high pin state
			if(current_pin_state){
				// initial detection of key press has occurred. pin high
				prev_pin_state = 1;				// record that key has been pressed
				// time key press occurred
				 ATOMIC_BLOCK(ATOMIC_FORCEON)
				{
					key_press_time = uSec_976_cnt; 
				} 
			}
		}else{
			// prev_pin_state is high
			// current pin can be either high or low
			// if high, do nothing.  Waiting for key press to end.  Watch for pin to go low.
			if(!current_pin_state){
				//current_pin_state is low/unlike previous/key press has ended
				// check duration of key press, if longer than valid is it short or long press, record
				prev_pin_state = 0;									// record that key has been released
				// test time: >= 1000 ms is long press.  Less than 1000 but more than Key_lock_period is a short, else cancel
				ATOMIC_BLOCK(ATOMIC_FORCEON)
				{
					key_duration = uSec_976_cnt - key_press_time - KEY_LOCK_PERIOD;  
				}
				key_press_time = 0;   
				// key_status cascades to determine 
				key_status = LONG;
				if(key_duration<1000){key_status = SHORT;}
				if(key_duration<0){key_status = NONE;}	
				key_duration = 0;		
			}
		}
		switch (key_status){
			case LONG:
				KeyLongPress();
				break;
			case SHORT:
				KeyShortPress();
				break;
		}
		key_status = NONE;
	}
}
ISR(TIMER0_COMPA_vect){
	uSec_976_cnt++;
}

void KeyLongPress(){
	PORTB ^= (1<<PB2);
}
void KeyShortPress(){
	PORTB ^= (1<<PB0);
}


 

This topic has a solution.
Last Edited: Mon. Jul 25, 2016 - 09:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First question:  Is your AVR really running at 1MHz as the code indicates?  Have you proven that?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

I'm satisfied it is, approximately.  I'm writing the program in increments.  Previous incarnations had the power on LED flashing in time.  I timed the flashes over time... it was within 5% of what I expected.  Is there some other accepted method of "proving" it?

 

<edited punctuation for clarity>

 

 

Last Edited: Mon. Jul 25, 2016 - 07:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

With respect to the toggling of both LEDs simultaneously:

 

The question in my mind is with regard to the switch case block.  It seems that a single button press may at times result in both cases being executed (calling the long press and short press functions).  I don't see how that would be possible.  But something is causing both output pins to go high faster than my eye can detect.  If it's a bounce, the button would have to bounce on from the off position for almost a quarter of a second (more precisely, 250 consecutive samplings 976us apart).  Seems unlikely for a tactile button from what I've read

 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

chipz wrote:
key_duration = uSec_976_cnt - key_press_time - KEY_LOCK_PERIOD;

I can't really trace your operations in my head.  I'll point out that your above is for >>one button<<.  My apps usually have several buttons/inputs to be debounced; up to a couple dozen.  Your head is going to hurt extending your method.

 

Summary is that I do parallel read/debounce on the buttons/inputs.  For key-held-down, I have auxiliary counters.  typical is a 10ms. tick (compared to your 1ms.), and an 8-bit auxiliary "key held" reaches a couple seconds.  Examples upon request.

 

Anyway, consider the results of the above formula when cnt is (say) 5 and press_time is 4.  REsult is a huge number (subtraction underflow).

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Thanks.  My goal is to find why it isn't working as is, first. 

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

chipz wrote:
My goal is to find why it isn't working as is, first.
theusch wrote:
consider the results of the above formula when cnt is (say) 5 and press_time is 4. REsult is a huge number (subtraction underflow).

 

Studio simulator can help you trace through...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Mon. Jul 25, 2016 - 09:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay... I didn't understand that.

 

Thanks.  That was it.  I put in an upper limit to set key_status to NONE if exceeded.  Works beautifully!  yes

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

chipz wrote:
I'm writing the program in increments.

A terrible approach. Programs should not be one great amorphous lump of code that just grows organically as you tack more and more bits onto it. You should spit the code into distinct and discrete modules. Work on each in turn in isolation and only continue when that block is totally proven to cater for all circumstances you might use it in. It now becomes a "black box" building block that you should not need to get inside again (unless some serious flaw comes to light later). Do this for each module in the system then the final program design just becomes a case of integrating the building blocks. C++ encourages this kind of approach to program design but it's quite possible in C too.

 

In fact the modern approach to software development is that once you have defined you requirements for a module you actually right your unit test plan and test code first that will verify the operation of the module. Then you implement to the module itself based on a design that comes out of the requirements and operate the tests upon it until it is proven to implement all the requirements. At this stage it truly is a black box.

 

History shows that other approaches where you just keep expanding some core loop and adding in more and more statements to now handle X, Y or Z is very error prone and very difficult to maintain.

 

With the black box, modular approach say one of your modules is driving an external DAC or something and later the hardware designer comes along and tells you that they have found a better device for half the price. If you now refactor the operation of your DAC driver code to now operate with this new device (or LCD or modem or whatever it is) and the tests prove that it offers exactly the same interface to the rest of the system then nothing else in the code will even know that it has changed. If, on the other hand, main() is peppered with statements both in the init code and in the infinite loop that are interacting with the DAC it would be very difficult to "unwind" all the interactions and replace them.