while (!(TWCR & (1<<TWINT))); never become 1 " TWI I2C ATmega328p "

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

ATmega328p TWI

The problem is the program stuck in the while loop some of reason

I added a green LED to the project to found the problem

The green LED turns on and stays on forever at "while (!(TWCR & (1<<TWINT)));" this point

Does anyone have any idea why the TWINT newer become 1?

//ATmega328p
#define F_CPU 1000000UL
#include <avr/io.h>
#include <util/delay.h>

int main(void)
{
	TWBR = 2;			//Bitrate 2 at 1MHz gives 50KHz
	DDRD = 0b00001100;
	PORTD = 0b00000000;

	while (1)
	{
		_delay_ms(1000);

		//wake the sensor
		PORTD |= 1<<PIND2;				// Progress green LED on
		TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN);	// Send Start
		while (!(TWCR & (1<<TWINT)));			// Wait for TWINT set to 1
		PORTD &= (~1<<PIND2);				// Progress green LED off
		if ((TWSR & 0xF8) != 0x08)	PORTD |= 1<<PIND3;	// Turn on red LED if there's an error

		TWDR = 0xB8;
		TWCR = (1<<TWINT) | (1<<TWEN);	//start transmission
		while (!(TWCR & (1<<TWINT)));
		if ((TWSR & 0xF8) != 0x18)	PORTD |= 1<<PIND3;

		TWDR = 0x00;
		TWCR = (1<<TWINT) | (1<<TWEN);	//start transmission
		while (!(TWCR & (1<<TWINT)));
		if ((TWSR & 0xF8) != 0x28)	PORTD |= 1<<PIND3;

		TWCR = (1<<TWINT)|(1<<TWEN) |(1<<TWSTO); // Stop

		_delay_ms(1000);
		PORTD = 0b00000000;
	}
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Is the LED on long enough for you to see? You won't see single flashes much shorter than 50ms.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

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

Hello, are you using external pull ups ? , if not and you want to use internal pull ups you can do this by setting PORTC high on the SDA,SCL Pins on your controller.

 

Last Edited: Sun. Jul 29, 2018 - 08:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You shouldn't be using the PIND register to control LEDs.  Instead read and write the PORTD register:   PORTD |= (PORTD & LED);

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

In a Mega328P, writing to a bit in PINx toggles that pin. To absolutely control a pin, set or clear the corresponding bit in PORTx. 

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

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

 

ka7ehk wrote:

Is the LED on long enough for you to see? You won't see single flashes much shorter than 50ms.

This is pretty critical.  Unless you are certain that the LED on/off periods will be long enough to be detected by the human eye (and if you're that certain, why are you debugging it? wink), this technique can end up being more confusing than clarifying.  It's always best to put a scope on such a debug output to make sure you catch activity too fast for direct human perception.  If you don't have a scope, turn on separate LEDs to indicate the start of a pulse and the end of a pulse, instead of pulsing a single LED.

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

He has a 1 second delay at both the start and end of the while() loop.

so the green LED (pin 2 of port D) will switch off for 2 seconds when TWINT is set to 1.

 

Edit

PORTD &= (~1<<PIND2);	

should be

PORTD &= ~(1<<PIND2);	

 

Last Edited: Sun. Jul 29, 2018 - 11:35 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Is there a slave on the bus to acknowledge the SLA+RW?

 

If an error occurs you turn on a red LED, but continue on to the next step as if there was no error.

 

 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

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

Simonetta wrote:
You shouldn't be using the PIND register to control LEDs.

1)  Why not?

ka7ehk wrote:
In a Mega328P, writing to a bit in PINx toggles that pin.

 

2)  OP's posted code has no references to PIND.

 

 

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

No I would not see the led to light up but the program stops "stuck" at the while loop and therefore the LED stays on.

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

Take a look at The iom328p.h

 

#define PIND _SFR_IO8(0x09)
#define PIND0 0
#define PIND1 1
#define PIND2 2
#define PIND3 3
#define PIND4 4
#define PIND5 5
#define PIND6 6
#define PIND7 7

 

I'm using the atmel studio 7. For arduino these are may defined differently but theres nothing arduino related of my project

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

O yeah sorry (~1<<PIND2)  that's a typo but the LED stays on anyway. The point is that he program stops at the while loop which means that after this operation TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN); the TWINT bit must change 0 to 1 and the while loop keep running over and over again as long as the TWINT bit of TWCR regisert is 0 which means the I2C modul is busy and 1 when ready for transmittion.

Last Edited: Wed. Aug 1, 2018 - 10:16 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

14.4.10 PIND – The Port D Input Pins Address FOR R/W

page 92 

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

Which of the three while loops is the code getting stuck on?

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

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

It has to be the first one doesn't it? He's saying the LED is stuck on. If it passed through the first while:

		while (!(TWCR & (1<<TWINT)));			// Wait for TWINT set to 1
		PORTD &= (~1<<PIND2);				// Progress green LED off

it would have hit the line to turn the LED off and it remains off from there on.

 

Thankfully I have never had to deal wiht TWI on AVR so perhaps others know more about this but in the line before the while() it sets the TWINT bit to 1. Isn't this like bits such as ADSC then? With those you set the bit and it remains set until the operation is finished. In which case the while loop would be +ve (no !) not -ve wouldn't it?

Suppose I'll have to go and read a datasheet to find out now!

 

(but why wouldn't one simply use Fleury and get on with life??)

 

EDIT: OK, that was educational, so I'm wrong. TWINT is like any normal "IF" flag such that you write 1 to it to clear it then it remains clear until the START has been sent. So the while() loop is in the right sense. Sorry for the noise.

 

EDIT2: so I had to look at Fleury's code. This is what he does for a START:

/*************************************************************************
  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 */

So that looks pretty similar to what OP is attempting. In fact I can't help noticing there is no timeout on the initial while() loop so it must be considered that it will always terminate in a timely fashion?

Last Edited: Wed. Aug 1, 2018 - 01:58 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
I can't help noticing there is no timeout on [Fleury's]  initial while() loop 

I seem to remember noting that in a thread here some time ago ...

 

EDIT

 

https://www.avrfreaks.net/commen...

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Fri. Jan 31, 2020 - 09:36 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
it would have hit the line to turn the LED off and it remains off from there on.

In a quick rescan of the thread, I don't see where OP told connections -- active-high or active-low LED?

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

@clawson wrote:

It has to be the first one doesn't it? He's saying the LED is stuck on. If it passed through the first while:

		while (!(TWCR & (1<<TWINT)));			// Wait for TWINT set to 1
		PORTD &= (~1<<PIND2);				// Progress green LED off

it would have hit the line to turn the LED off and it remains off from there on.

Wouldn't that depend on whether he implements the fix Chuck99 indicated above?  He has acknowledged it:

O yeah sorry (~1<<PIND2)  that's a typo but the LED stays on anyway.

 

EDIT: And, as theusch pointed out, we don't know if the LEDs  are active high or low. 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

Last Edited: Wed. Aug 1, 2018 - 03:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
In a quick rescan of the thread, I don't see where OP told connections -- active-high or active-low LED?
Good point. I made the possibly fatal mistake of trusting the comments:

PORTD |= 1<<PIND2;				// Progress green LED on
PORTD &= (~1<<PIND2);				// Progress green LED off

I sort of assume OP would not have made such a comment if experience showed it to be otherwise but we all know about ass-u-me !!

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

clawson wrote:
(but why wouldn't one simply use Fleury and get on with life??)

 

Indeed, I'll add a link:  http://homepage.hispeed.ch/peter...

 

This assumes the OP is attempting to write an I2C master.

Start with proven code, to verify your h/w works as expected, then if you must, write your own code.

This way you have something working you can compare against.

Good luck,

 

Jim

 

 

(Possum Lodge oath) Quando omni flunkus, moritati.

"I thought growing old would take longer"

 

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

can anyone explain the logic of:-

1) TWCR=(1<<TWSTA)|(1<<TWEN)|(1<<TWINT);

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

 

   

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


1) not clear if you mean you want the C syntax explained or the intention of setting TWSTA, TWEN and TWINT bits in the TWCR register? The comment says:

// Send Start

as for the syntax. The register is:

So if you want to set TWINT, TWSTA and TWEN bits in this register you would need to write 0b10100100 to the register. That is a byte with 1 bits in positions 7, 5 and 2. Well the header file has:

#define TWIE 0
#define TWEN 2
#define TWWC 3
#define TWSTO 4
#define TWSTA 5
#define TWEA 6
#define TWINT 7

So:

TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN);

is nothing more than:

TWCR = (1<<7) | (1<<5) | (1<<2);

which is a byte that has bits 7, 5 and 2 set - as required.

 

2) Similarly:

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

means, keep reading the TWCR register and & this with (1 << 7). The result of such an AND is either 0 if the bit is not set or (1 << 7) if it is set (ie bit 7 is set). Lets say the bit was 0 then TWCR  & (1 <<7) will be 0. If you use !0 you get 1 so it will perform while(1) which means "stay in the while loop". After a while the TWINT bit will become set. TWCR & (1 << 7) will therefore be (1 << 7) which is a positive value. !<any non 0> is 0 so the while now changes to while(0).A while loop ends when the conditional value is 0 so bit 7 (TWINT) becoming set causes the while() loop to end.

 

In effect the code just waits at that line until TWINT is set. (presumably indicating that a TWI interrupting event has occurred).

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

why would I need to check on the bit (TWINT) after I just set it by writing this statement TWCR=(1<<TWSTA)|(1<<TWEN)|(1<<TWINT);!?
  

shouldn't this statement "TWCR=(1<<TWSTA)|(1<<TWEN)|(1<<TWINT);" means that TWINT now is HIGH
    

Last Edited: Fri. Dec 13, 2019 - 04:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see the confusion.

This is because it is following a common pattern where, with some 'special' bits typically associated with interrupt flags in the hardware, the act of writing a 1 to that bit clears the flag in the hardware.

When you read it back you get the state of the flag in the hardware.

So writing 1 to TWINT casues the hardware flag to be cleared.

When you read it back, you get the state of the hardware flag, so initially it is 0 beacuse you just cleared it (by writing 1), and then when the operation has completed it is set to 1 by hardware.

In other words, these 'special' bits behave differently for write vs read.

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

Another example:

ADCSRA |= (1 << ADSC); // start conversion
while (ADCSRA & (1 << ADSC)); // wait for completion
result = ADC;

In the case of the ADC you set the ADSC bit to start the conversion, for the next 66us (or however long the conversion takes) the bit remains at 1 (so the while() loop holds there until this is finished). When the analog conversion is complete the ADSC bit automatically switches back from 1 to 0.

 

Another way to do the same thing is:

ADCSRA |= (1 << ADSC); // start conversion
while (!(ADCSRA & (1 << ADIF))); // wait for completion
result = ADC;

In this case the conversion is started and the ADIF (interrupt flag) remains at 0 while the conversion is being performed. When complete the ADIF bit is set to 1 and the while() loop ends. This is actually very like the TWINT example above.

 

(only downside of using ADIF for analog  conversions is that if you wait for the bit to go to 1 you have to manually clear it afterwards - in this sense waiting for ADSC to go from 1 to 0 is "better" as it auto-clears)

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

This is some kind of hardware problem.   The command to do START:

TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN);

completes in two TWI cycles, so you should never see the green LED.  The Master asserts SDA (pulls down PC4), waits one TWI cycle, then asserts SCL.  The TWI then checks if both SDA and SCL are at GND (both are asserted).  If so, then it sets the TWINT flag, which indicates that the TWI operation of the previous instruction has completed.

 

 The TWI probably never sees SDA and/or SCL go to GND.  Which means that the AVR can't assert these lines ( pull them to GND).  Possibly because the pull-up resistors are too stiff (low in ohms value).   The pull-up values should be 2-5K ohms.  If they are 200 ohms or 20 ohms (misreading of the color code band) then the AVR can't pull PC4 or PC5 to Gnd, and START condition never gets registered by the TWI unit.