AVR hanging in TWI code

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

I've been playing around with Fleury's TWI code and a RTC.  It was pretty simple to get up and running, but I noticed that some of my test setups would appear to freeze and require a reset to resume working again.  I did some troubleshooting and found that if I disconnected the SDA wire while my program was running, it would lock things up.  Interruptions on the SCL connection didn't harm anything, the program would pick up right where it left off.  I decided to take a closer look at the Fleury code (included below for reference) and the read/write/start commands all have empty while loops waiting for the hardware to complete it's duty for that cycle of communication.

 

while(!(TWCR & (1<<TWINT)));

 

I don't have an AVR dragon I can connect, but based on the troubleshooting information I can pass to the LCD, I have confirmed the hangups occur when using the i2c_start/write/read commands and the SDA wire is disconnected.  Does anyone have any suggestions for improving the error handling of this code?  It seems unlikely to expect a 100% response rate from a serially connected slave device, my assumption is that the disconnected SDA simulates this.  I thought about a watchdog, but I don't really want to reset my AVR all the time.  I was thinking of adding a timer to the commands and using break() to get me out of the call on a time out.  Any thoughts on the best way to accomplish this?

 

Does it sound likely that these while loops waiting on TWINT status are what's hanging me up?

 

/*************************************************************************	
  Issues a start condition and sends address and transfer direction.
  return 0 = device accessible, 1= failed to access device
*************************************************************************/
unsigned char i2c_start(unsigned char address)
{
    uint8_t   twst;

	// send START condition
	TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN);

	// wait until transmission completed
	while(!(TWCR & (1<<TWINT)));

	// check value of TWI Status Register. Mask prescaler bits.
	twst = TW_STATUS & 0xF8;
	if ( (twst != TW_START) && (twst != TW_REP_START)) return 1;

	// send device address
	TWDR = address;
	TWCR = (1<<TWINT) | (1<<TWEN);

	// wail until transmission completed and ACK/NACK has been received
	while(!(TWCR & (1<<TWINT)));

	// check value of TWI Status Register. Mask prescaler bits.
	twst = TW_STATUS & 0xF8;
	if ( (twst != TW_MT_SLA_ACK) && (twst != TW_MR_SLA_ACK) ) return 1;

	return 0;

}/* i2c_start */
/*************************************************************************
  Send one byte to I2C device
  
  Input:    byte to be transfered
  Return:   0 write successful 
            1 write failed
*************************************************************************/
unsigned char i2c_write( unsigned char data )
{	
    uint8_t   twst;
    
	// send data to the previously addressed device
	TWDR = data;
	TWCR = (1<<TWINT) | (1<<TWEN);

	// wait until transmission completed
	while(!(TWCR & (1<<TWINT)));

	// check value of TWI Status Register. Mask prescaler bits
	twst = TW_STATUS & 0xF8;
	if( twst != TW_MT_DATA_ACK) return 1;
	return 0;

}/* i2c_write */


/*************************************************************************
 Read one byte from the I2C device, request more data from device 
 
 Return:  byte read from I2C device
*************************************************************************/
unsigned char i2c_readAck(void)
{
	TWCR = (1<<TWINT) | (1<<TWEN) | (1<<TWEA);
	while(!(TWCR & (1<<TWINT)));    

    return TWDR;

}/* i2c_readAck */


/*************************************************************************
 Read one byte from the I2C device, read is followed by a stop condition 
 
 Return:  byte read from I2C device
*************************************************************************/
unsigned char i2c_readNak(void)
{
	TWCR = (1<<TWINT) | (1<<TWEN);
	while(!(TWCR & (1<<TWINT)));
	
    return TWDR;

}/* i2c_readNak */

 

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

I'm guessing here and my two cents worth is sometimes not worth the effort to understand what I'm saying.  But since no else has responded as yet, I'll start the circus.

 

...the hangups occur when using the i2c_start/write/read commands and the SDA wire is disconnected.

 

 It could be that the pull-up resistor on the master's SDA line is disconnected when the the slave device is disconnected.  In that case, the master would assume that the slave was asserting the SDA continously.  This assumes that the TWI unit is monitoring the SDA line after the master releases it to verify that it actually does rise to Vcc.    The length of time that each command takes is determined by the TWBR register setting.  The line in main code: while(!(TWCR & (1<<TWINT)));  simply lets main code know when the TWI has finished the command.   If the TWI is monitoring for a successful release on SDA, and SDA doesn't get pulled up when master releases it, then the TWI will never set the TWINT flag, and that line will loop endlessly.

 

The TWI unit does all of its operations (all of its asserting and releasing of SCL/SDA)  in the background after getting its command.  After a START or REPEAT-START, the  TWI unit assumes that master is going to be controlling a SLA command.  After that SLA, the R/W bit status determines it will control or read the SDA line.

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

Without seeing the rest of your code, what you have posted is not enough.  What clock speed are you using?  100khz?  400khz?  WHat are the values of the resistors you are using?

 

Fleury's TWI library is pretty solid...Used it myself a couple of times with no real fuss.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

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

You can b*gger most things if you cut a wire / unplug halfway through a write operation.

Whereas you can watch for common mishaps like power failure,    I don't see how you can physically stop a knife / axe / explosion.

 

I suppose that you could use a PIR or capacitance to detect a human intrusion.     This will generally give you enough time to shut down before the axe makes contact.

An explosion or other catastrophe is a lot more difficult.

 

Simple measures like switches on a cover or encapsulating the whole board in resin may deter the human and protect from physical harm.

 

If hot-swapping is an essential requirement,   you should anticipate problems.    e.g. create redundant copies of data.    ensure that every write / read is consistent.    timeout / watchdog recovery.

 

In practice,   you start embedded programs from cold and don't do hot-swapping.     You probably force a cold re-start when you detect a hardware change.

 

David.

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

AKnick wrote:

...It seems unlikely to expect a 100% response rate from a serially connected slave device...

 

Why do you say that? You are talking about I2C which was designed to link chips which are a few inches apart on the same PCB.

 

I2C = IIC = Inter IC

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

My comment about the response rate it based on my experience with other serial bus transmissions (modbus etc.) they work great, but there are always 1-0.5% missed slave responses.  I mention this, because its the missed response part that is hanging the AVR.  I don't want to make it hot swappable or idiot proof the design.  I'm just seeing that the code will hang on the TWI communication portions about once every 24 hours.  I suspect this is due to data loss, which I tried to simulate by pulling the SDA wire.

 

I agree Fleury's routines are solid, they work out of the box and are easy to use.  I'm trying to use them in an application that runs 24/7 w/o resets.  This is the first time I've used an AVR in this capacity, so maybe there is something else that I missed.  I did look my board over for bad solder joints and it looks good. 

 

I am using 4.7K pull up resistors and the system is running at 100khz.  Here is my test program that is actually calling Fluery's code:

/*
 Aquarium controller and light timer

 References:
 Required I2C and LCD libraries by Peter Fleury:
 http://homepage.hispeed.ch/peterfleury/avr-software.html
 */ 
//*********REQUIRED HEADERS*********
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <avr/sfr_defs.h>
#include "lcd.h"
#include "i2cmaster.h"
#include "ChronoDot.h"

//*********DEFINE PORT FUNCTIONS*********
#define PORT_ON( port_letter, number )			port_letter |= (1<<number)
#define PORT_OFF( port_letter, number )			port_letter &= ~(1<<number)

//*********VARIABLES*********
uint8_t seconds = 0;	//store the seconds reading
uint8_t minutes = 0;	//store the minutes reading
uint8_t hours = 0;		//store the hours reading

//*********FUNCTIONS*********
//This function takes a 1-2 digit number and ensures
//that the LCD always used two digits to display the value
void lcd2digit(uint8_t x, uint8_t y, uint8_t number){
	lcd_gotoxy(x,y);			//send LCD cursor to desired location
	char buffer[33];			//this buffer is bigger than 2 digits, no buffer overrun
	itoa(number, buffer, 10);	//convert the given value to a string
	if (strlen(buffer)==1){		//if the string is only 1 long, we need to add a "0" in front
		lcd_puts("0");			//print "0" at location
		lcd_gotoxy(x+1,y);		//move one x coordinate over
		lcd_puts(buffer);		//print the buffer
	}
	else{						//else the string is longer than one, no work needed
		lcd_puts(buffer);
	}
}

//*********MAIN PROGRAM*********
int main(void)
{
	DDRB = 0b00001111;		//Split B ports, B0-B3 are outputs, B4-B7 are inputs
	PORT_ON(PORTB,PORTB0);	//turn all output ports on
	PORT_ON(PORTB,PORTB1);	//when output ports on on, relays are de-engergized
	PORT_ON(PORTB,PORTB2);	//so on in code is off electrically
	PORT_ON(PORTB,PORTB3);
	i2c_init();			    // initialize the i2c hardware
	lcd_init(LCD_DISP_ON);  // initialize lcd, display on, cursor on
	lcd_clrscr();           // clear screen of lcd
	lcd_home();				// bring cursor to 0,0
	//this code does not yet allow for external setting of the ChronoDot
	//enter the current time values in the commented out section of code
	//flash the AVR then comment out the code again and re-flash.  This 
	//will prevent power loss to the AVR from reseting your clock times.
/*	set_register(SECONDS_REG,00);			//sets the seconds in the RTC, range 0-59
	set_register(MINUTES_REG,57);			//sets the minutes in the RTC, range 0-59
	set_register(HOURS_REG, 0b00011000);	//see DS3231 data sheet for setting these bits
	set_register(DAYS_REG,8);				//sets the current day in month, range 1-31
	set_register(MONTH_REG,11);				//sets the current month in year, range 1-12, also clears century bit
	set_register(YEAR_REG,14);				//sets the current year, range 00-99  */
	while(1)
		{
		//get the current time and display in the leftmost portion of the first row on the LCD
		hours = get_register(HOURS_REG);
		minutes = get_register(MINUTES_REG);
		seconds = get_register(SECONDS_REG);
		lcd2digit(0,0,hours);
		lcd_gotoxy(2,0);lcd_puts(":"); 
		lcd2digit(3,0,minutes);
		lcd_gotoxy(5,0);lcd_puts(":");
		lcd2digit(6,0,seconds); 
		//get the current date and display in the leftmost portion of the second row on the LCD
		lcd2digit(0,1,get_register(MONTH_REG));
		lcd_gotoxy(2,1);lcd_puts("/");
		lcd2digit(3,1,get_register(DAYS_REG));
		lcd_gotoxy(5,1);lcd_puts("/");
		lcd2digit(6,1,get_register(YEAR_REG));
		//NOTE: turning a port off enables the output relay, so off=on electrically
		//turn main lights on from 10:00 to 18:00
		if ((hours>=10)&&(hours<18))
			{
				PORT_OFF(PORTB,PORTB1);
				PORT_OFF(PORTB,PORTB2);
				lcd_gotoxy(0,3);
				lcd_puts("L1&2:ON ");
			}
		else{
				PORT_ON(PORTB,PORTB1);
				PORT_ON(PORTB,PORTB2);
				lcd_gotoxy(0,3);
				lcd_puts("L1&2:OFF");
			}
		//morning moonlight period from 09:00 to 11:00, night from 17:00 to 22:00
		if ( ((hours>=9)&&(hours<11)) || ((hours>=17)&&(hours<22)) )
			{
				PORT_OFF(PORTB,PORTB0);
				lcd_gotoxy(9,3);
				lcd_puts("ML:ON ");
			}
		else{
				PORT_ON(PORTB,PORTB0);
				lcd_gotoxy(9,3);
				lcd_puts("ML:OFF");
			}
		//half second delay before the next data polling
		_delay_ms(500);
		}  //end while(1)
}  //end main

Also, the code that I am using to interface to the RTC:

/*
DS3231 temperature compenstated RTC
Modified code from sources to make the ChronoDot RTC using teh DS3231 usable on a standard AVR
Nick Stadnicky 2014

References: 
Required I2C library by Peter Fleury:
http://homepage.hispeed.ch/peterfleury/avr-software.html
*/

#ifndef DS3231_H
#define DS3231_H 1
#include <stdio.h>
//*********DEFINITIONS*********
#define DS3231_ADDR		(0x68<<1)	//DS3231 I2C address per data sheet, left shifted to allow easy appending of R/W bit
#define SECONDS_REG		0x00		//Register holding second data
#define MINUTES_REG		0X01		//Register holding the minute data
#define HOURS_REG 		0x02		//Register holding the hour data
#define WEEKDAY_REG		0x03		//Register holding the day of week data
#define DAYS_REG		0x04		//Register holding the day of month data
#define MONTH_REG		0x05		//Register holding the month of year data
#define YEAR_REG		0x06		//Register holding the year data
#define DS3231_cont		0x0E		//Control register for the DS3231 chip
#define DS3231_stat		0x0F		//Status register for the DS3231 chip
//Temps are not BCD, but two's complement
#define DS3231_tempU	0x11		//Upper 8 bits of 10 bit temperature
#define DS3231_tempL	0x12		//Lower 2 bits of 10 bit temperature

//*********FUNCTIONS*********
extern void set_register(uint8_t dev_reg, uint8_t setvalue);
extern uint8_t get_register(uint8_t dev_reg);
#endif
/*
DS3231 temperature compensated RTC
Modified code from sources to make the ChronoDot RTC using the DS3231 usable on a standard AVR
Nick Stadnicky 2014

References: 
Required I2C library by Peter Fleury:
http://homepage.hispeed.ch/peterfleury/avr-software.html

parts of the code taken from:
https://github.com/adafruit/RTClib
*/

//*********REQUIRED HEADERS*********
#include "avr/io.h"
#include "avr/pgmspace.h"
#include "i2cmaster.h"
#include "ChronoDot.h"

//*********FUNCTIONS*********
//convert decimal number to BCD
uint8_t dec2bcd(uint8_t val) {
	return val + 6 * (val / 10);
}
//convert BCD data to standard binary format
 uint8_t bcd2bin (uint8_t val){
return val - 6 * (val >> 4);
}
//writes value to specified register of the ChronoDot
void set_register(uint8_t dev_reg, uint8_t setvalue) {
	i2c_start(DS3231_ADDR|I2C_WRITE);	//waiting for I2C traffic to clear, then access ChronoDot with write
	i2c_write(dev_reg);					//specify the register to be written
	if (dev_reg==HOURS_REG)	{			//HOURS_REG should not be written in BCD
		i2c_write(setvalue);			//specify the value to be written to the register
	} 
	else{								//else, the register requires BCD data
		i2c_write(dec2bcd(setvalue));	//specify value to be written to the register
	}
	i2c_stop();							//release I2C bus	
}
//Gets value from specified register of the ChronoDot
uint8_t get_register(uint8_t dev_reg) {
	uint8_t reads = 0;
	if (dev_reg==HOURS_REG){						//if we are dealing with the hours register - ONLY 24H clock supported
		i2c_start(DS3231_ADDR|I2C_WRITE); 			//waiting for I2C traffic to clear, then access ChronoDot with write
		i2c_write(dev_reg);							//specify that write will take place to SECONDS_REG
		i2c_start(DS3231_ADDR|I2C_READ); 			//waiting for I2C traffic to clear, then access ChronoDot with read
		reads = i2c_readNak();						//storing the bit that is read as BCD
		reads = bcd2bin(reads &= 0b00111111);		//bitmask to isolate the hours portion of the register from the setup bits
		return reads;								//return the read value
	} 
	else{
		i2c_start(DS3231_ADDR|I2C_WRITE); 			//waiting for I2C traffic to clear, then access ChronoDot with write
		i2c_write(dev_reg);							//specify the device register to write to
		i2c_start(DS3231_ADDR|I2C_READ); 			//waiting for I2C traffic to clear, then access ChronoDot with read
		reads = bcd2bin(i2c_readNak());				//storing the bit that is read as a decimal
		i2c_stop();									//release I2C bus
		return reads;								//return the read value
	}
}

 

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

You really need timeouts on I2C - as you've found, a small upset locks it up.  As you've found , disconnecting wires upsets it, ESD also upsets it. Depending on your application, you can build timeouts into the driver or just rely on the watchdog to resolve the situation.