ATtiny85 PB2 won't go high?

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

Hi - I have a simple comparator+delay circuit where the PB0 output is inverted by hardware (pin 5 active LOW), PB2 is active HIGH on pin 7, and PB3 is active HIGH on pin 2.  My PB0 and PB2 are behaving as intended, but my PB3 just stays LOW.  The odd thing is that the code line for driving PB3 HIGH is right smack after the line that drives PB0 LOW.  My development environment is AS7.  I've checked for shorts on pin 2, it looks fine (1.38K to ground - drives a 680Ohm resistor and the base of a common emitter BJT).  Can anybody shed light on why my PB3 won't go HIGH when PB0 is going LOW as expected?  Here's my code:  MUCH THANKS!!

/*
 * Comparator_2.c
 *
 * Created: 9/16/2016 12:06:34 AM
 * Author : paul
 */ 

#include <avr/io.h>
#include <util/delay.h>
	// set to internal oscillator at 8MHz????????????????????
#define F_CPU 8000000L
#define count_on_threshold 15
#define count_off_threshold 10
int count_on, count_off, out_flag, LED_ON;

int main (void)
{
	// Set Analog Comparator Negative Mux Input to AIN1 => ACME = 0
	ADCSRB &= ~(1<<ACME);
	// disable digital inputs
	DIDR0 = (1<<AIN1D) | (1<<AIN0D);
	ACSR = (0<<ACD) | (1<<ACBG);
	// set PB0, PB2, PB3, PB4 to be outputs, PB1 (AIN1) for input
	DDRB  = 0b00011101;
	PORTB = 0b00000001;								//clear outputs, clear the pump output on PB0 to OFF (inverted in HW)

while (1) {

WAIT:	PORTB &= 0b11111011;						//clear the indicator LED (PB2) & Alarm LED (PB3)
													//and clear the PB0 output (inverted in hardware)
		_delay_ms(200);
		if (LED_ON == 1) {
			PORTB |= 0b00000100;					//set the indicator LED ON (PB2), blinking
		}
		else {
			PORTB &= 0b11111011;					//clear the indicator LED (PB2)
		}
		_delay_ms(200);
		
		if (ACSR & (1<<ACO)) {						//Comparator is NOT tripped
			LED_ON = 0;								//clear the indicator LED
			count_on = 0;							//clear the count_on
			if (out_flag==1) {
				count_off++;
				if (count_off > count_off_threshold) {
					PORTB |= 0b00000001;			//clear the pump output to OFF (PB0 inverted)
					PORTB &= 0b11110111;			//clear PB3 output (Alarm)
					out_flag = 0;					//clear the output flag
					count_on = 0;					//clear both counters
					count_off = 0;
					goto WAIT;						//go back to WAIT
				}
				else {
					goto WAIT;						//go back to WAIT
				}
			}
			else {
				goto WAIT;							//go back to WAIT
			}
		}
		else {										//Comparator is tripped
			LED_ON = 1;								//set the indicator LED ON (PB2)
			count_off = 0;							//reset the count_off
			if (out_flag==1) {						//? is the output ON?
				goto WAIT;							//if so go back to WAIT
			}
			else {									//output is NOT ON
				count_on++;							//increment the counter_on
				if (count_on > count_on_threshold) {	//ready to turn on output?
					PORTB &= 0b11111110;			//yes - Set the pump output ON
					PORTB |= 0b00001000;			//and the Alarm output PB3
					out_flag = 1;					//set the output flag
					count_on = 0;					//reset the count_on
					count_off = 0;					//reset the count_off
					goto WAIT;						//go back to WAIT
				}
				else {
					goto WAIT;						//go back to WAIT
				}
			}
		}		
	}
	
	return 1;
}

 

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

Which line(s) are you talking about?

I see 3 references in the comment about PB3, but one of them acts only  on PB2:

paulofelora wrote:
WAIT: PORTB &= 0b11111011; //clear the indicator LED (PB2) & Alarm LED (PB3)

 

When using "0b00100010" it's very easy to make mistakes like that.

You will make it easier on yourself if you define logical, human readable names for the outputs you want to control.

Just as in the firs few lines of main() where you change the ADCSRB, DIDR0 and ACSR registers.

 

If you use your defined names consequently it will make your code partly self documenting. There wont be differences between comment & the actual code and it also makes it trivial to move a certain function to another port pin. For example for a quick test if there might be something wrong with the hardware. Could your PB3 pin be damaged?

 

Also note that:

#define F_CPU 8000000   // ????????????????????

does not change or set the cpu frequency.

The only thing it does is define a macro to tell some library functions (such as _delay_ms()) that it shoud generate code for an AVR which runs at 8MHz.

The actual setting of the AVR clock is done with the fuse bits.

(Note that Xmega's are capable of changing the clock on the fly, but tiny's and mega's are not).

 

 

And please do something about those goto's. They make my mind do uncomfortable things.

 

 

 

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Last Edited: Mon. Nov 28, 2016 - 01:29 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks Paulvdh - I'll take your advice on my constant namings... Interesting about the "goto WAIT".  I drew out a state map for how I wanted this to work, and the WAIT just fell out of it.  But I suppose there must be a way to make an identically-functioning state machine without goto's, tho.  Wonder if there's a formal theorem that covers that...

 

Wow - I plugged in another ATtiny85, and it worked as expected!! Bad PB3 output driver!

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

I guess I would write this as a state machine with C's switch. And make a enum with the different states (with a name that make sense ;) ).

And for the wait it will then be 2 case's that do the same. 

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

goto? Really?

 

At the very least just use "continue;" (though it's not much better)

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

Thanks for the guidance, fellows, will do state machine Right - paul