ATMega4809 TWI start condition sends additional 8 clock signals

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

I am trying to read a simple i2C temperature sensor. I can write to the sensors registers no problem and I receive all the ACK's as expected. The problem comes in when I try to read from the sensor. The start condition is initiated and the sensor ACK's then without doing anything else the SCL line is clocked an additional 8 times. What I am confused about is why are there additional clock cycles initiated after the start instruction? I cant tell if the master is driving the clock or if the slave is driving the clock for those 8 additional cycles.

 

The bus hangs because the master is not ACKING but who and why were those additional clock cycles initiated? When I debug the code the start function hangs on while(!(TWI0.MSTATUS & (TWI_WIF_bm))); so as far as i can tell it should have only clocked out the address of the slave with the read bit then waited for the ack and that's it.

 

void i2c_init()
{
	//set BAUD
	TWI0.MBAUD = BAUD;
	//flush
	TWI0.MCTRLB |=  TWI_FLUSH_bm;
	//enable master
	TWI0.MCTRLA |=  TWI_ENABLE_bm | TWI_SMEN_bm;           
	//set bus initial state to idle
	TWI0.MSTATUS |= TWI_BUSSTATE_IDLE_gc;
}

void i2c_start(uint8_t i2c_addr)
{
	TWI0.MCTRLA |= TWI_TIMEOUT_200US_gc;
	//setting address automatically creates a start condition
	TWI0.MADDR = i2c_addr;
	while(!(TWI0.MSTATUS & (TWI_WIF_bm)));                         //During debugging the function never gets past this point so why are additional clock signals toggled?
}



int main(void)
{
    i2c_init();
    
    while (1)
    {
        i2c_start(TEMP_SEN_READ);
    }
}

 

This topic has a solution.
Last Edited: Sun. May 23, 2021 - 02:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried submitting a picture but the forum gives me a message " You are not authorized to access this page. "

 

 

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

A master read will do the read after an address is ack'd, so you get a RIF when the read is done (no WIF). If the address is nack'd, then you get a WIF, so you have to check for both WIF and RIF.

 

A simple interrupt driven twi for an avr0/1 you can use to get ideas-

https://github.com/cv007/Avr01Dx...

 

 

edit- the datasheet - 'Transmitting Address Packets'- (you are at Case M4). Also, if above explanation was not clear, your additional scl clocks are because the master is now clocking in the read and is not waiting around for you to do anything after the address was ack'd.

Last Edited: Sun. May 23, 2021 - 10:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Go on.  What Temperature Sensor?  What Slave address ?

 

void functions are the work of the Devil.

 

It is always wise to start with proven libraries.  Use the return value from the library function(s).  Get your application to do what you want.    Only then do you re-invent the wheel.   e.g. you can compare the performance of hexagonal, square, ... wheels against the proven circular ones.

 

David.

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

If I understand I2C right the master drives the clock.  The slave is allowed to stretch the SCL low period once it is low.

And on the slaves I've worked with to do a read you first need to write a register address to the slave.  So the master sequence is START, ADDR+W, REG, START, ADDR+R, read, STOP.

Last Edited: Sun. May 23, 2021 - 01:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks curtvm was right. I failed to understand that the MCMD instruction that is set is based on the next operation to happen. I might still be mistaken but it is working now. It does not work with smart mode disabled though.

 

David you are def right with regards to not returning anything from the functions. I will download a library and rather use that then.

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

If I understand I2C right the master drives the clock.  The slave is allowed to stretch the SCL low period once it is low.

And on the slaves I've worked with to do a read you first need to write a register address to the slave.  So the master sequence is START, ADDR+W, REG, REPEATED_START, ADDR+R, read, STOP.

 

Alternatively you can use on many Slave devices:

START, ADDR+W, REG, STOP, START, ADDR+R, read, STOP.

 

The worrying thing about calvingloster 's code is that there is no return value from his i2c_start() function.  And no attempt to check for a NAK.

 

I suggest that the OP starts with proven library code and uses the return values.

 

The TWI peripheral provides the status information.   I2C is easy to use and most importantly easy to debug.

 

Yes,  if everything is perfect you do not need to know what has gone wrong.

Likewise,  you can drive your car on the motorway with your eyes and ears closed.   If you turn the steering wheel in the correct direction you get to your destination.

 

curtvm has posted a link.  There are Microchip App Notes.   There is Start code.  

Just pick one proven method and use it correctly.

 

David.

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

 I cant tell if the master is driving the clock or if the slave is driving the clock for those 8 additional cycles.

If you really want to know, you can install some small series resistors for the slave chip...so when it pulls the I2C lines low, it won't get quite to solid gnd compared to master.   Don't go overboard with too much---you don't want to create an erroneous voltage level, 100mv or so is fine!!  Some chips may offer enough difference by themselves to discern without resistors, but you have to have an eagle eye.

Note also, this is for scoping the lines...a logic analyzer (at least not a $10k one), will just show 0/1.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sun. May 23, 2021 - 03:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Yes,  if everything is perfect you do not need to know what has gone wrong.

Typically, handling the "everything perfect" case is the easy part - it's handling the failure cases that accounts for the majority of the development work!

 

It is common that a lot of demo/sample code only covers the "everything perfect" case - to keep the demo/sample simple.

 

david.prentice wrote:
Likewise,  you can drive your car on the motorway with your eyes and ears closed.   If you turn the steering wheel in the correct direction you get to your destination.

Same analogy for using blind delays with AT Commands:

I wrote:
This is like driving a car with your eyes shut, relying only on time delays for when to turn the wheel!!

 

surprise

 

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

 

#BlindDelays #UseReturnValues

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: Tue. Jun 22, 2021 - 08:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

it's handling the failure cases that accounts for the majority of the development work!

That leads to the we saw it was working 2 months ago at the concept demo, what's all the engineering "delay"??...well it was "working" perfectly fine & quite usable under ideal conditions....however:

it really should verify that the file was written correctly.  

it really should check that the desired file name is legit & request a re-entry

it really should warn if an exiting file will be overwritten

it really should abort & warn (rather than stall) if the file cannot be opened

it really should apply some error detection/CRC to the contents

etc

Same thing not just for files, but most anything; reading a keypad & entering a bad value, or the keypad becomes shorted, gets stuck keys, etc.

Yes, you saw the motor spinning a month ago, but there was not yet overtemperature shutdown, torque limiting to prevent gear stripping, backup limits for speed sensor failure, etc

So you can get "the basics" operating quickly, but that is often the tip of the iceberg.  It's not even that these things haven't been thought about, they simply haven't yet been put in place to get a demo running asap.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

david.prentice wrote:
void functions are the work of the Devil.

How? surprise

I have been writing void functions like crazy. Almost all of them get inlined. 

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

I was brought up with Assembly Language.    You pass parameters to a subroutine.   The subroutine returns a value.   It is just how I do things.

 

The C language was invented to do the same.   i.e. pass arguments to a function.  Return a value.

 

void was not part of the original C language.   It came later (as a good feature).    And then the Devil got involved.

 

It is your choice.   If you are happy driving a car with your eyes shut,  just carry on driving.

 

David.

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

david.prentice wrote:

void functions are the work of the Devil.

Heisen wrote:
How? surprise 

In the case where it's important to know that the function succeeded or not - as discussed here.

 

Of course, there are also other ways to get such status.

 

And a non-void function is effectively a void function when you ignore its status return.

 

EDIT

 

Some languages (eg, Pascal) make a distinction between a "function" and a "procedure":

 

  • a "function" is something which does some stuff, and returns some value as a result;
  • a "procedure" is something which just does some stuff - with no return value.

 

C is not one of those languages.

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: Mon. May 24, 2021 - 11:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

Heisen wrote:

How? surprise

I have been writing void functions like crazy. Almost all of them get inlined. 

void functions have no return so no opportunity to let the caller know if they worked or not. In a function that is trying to communicate to an external device that may not even be there or that may be in some "error" condition that means it cannot respond correctly it's good to be a able to return an error code to highlight the problem. Otherwise the calling code is just going to do this(), that(), the_other() without realising that this() did not succeed so everything after that was pointless.

 

In Fleury for example you have:

D:\fleury_i2c>grep i2c_.*( i2cmaster.h | grep extern
extern void i2c_init(void);
extern void i2c_stop(void);
extern unsigned char i2c_start(unsigned char addr);
extern unsigned char i2c_rep_start(unsigned char addr);
extern void i2c_start_wait(unsigned char addr);
extern unsigned char i2c_write(unsigned char data);
extern unsigned char i2c_readAck(void);
extern unsigned char i2c_readNak(void);
extern unsigned char i2c_read(unsigned char ack);

Things like i2c_start() have an unsigned char return and can return for example:

 @retval   0   device accessible 
 @retval   1   failed to access device 
 */
extern unsigned char i2c_start(unsigned char addr);

Having cited Fleury as a good example. He then presents:

 int main(void)
 {
     unsigned char ret;

     i2c_init();                             // initialize I2C library

     // write 0x75 to EEPROM address 5 (Byte Write) 
     i2c_start_wait(Dev24C02+I2C_WRITE);     // set device address and write mode
     i2c_write(0x05);                        // write address = 5
     i2c_write(0x75);                        // write value 0x75 to EEPROM
     i2c_stop();                             // set stop conditon = release bus


     // read previously written value back from EEPROM address 5 
     i2c_start_wait(Dev24C02+I2C_WRITE);     // set device address and write mode

     i2c_write(0x05);                        // write address = 5
     i2c_rep_start(Dev24C02+I2C_READ);       // set device address and read mode

     ret = i2c_readNak();                    // read one byte from EEPROM
     i2c_stop();

     for(;;);
 }

as an example that completely ignores any return values that the functions may be giving - this is NOT a good example !!

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

clawson wrote:
He then presents ...  an example that completely ignores any return values

See #9:  this is common with examples which are trying to show how the stuff works - without obscuring that behind all the complications of error handling.

 

Such examples should come with a "health warning" that this is not the way to write real code!

 

EDIT

 

Perhaps a better approach would be something like

 

     result = i2c_start_wait(Dev24C02+I2C_WRITE); // set device address and write mode
     ASSERT( result==OK );

     i2c_write(0x05);                             // write address = 5
     ASSERT( result==OK );

     i2c_write(0x75);                             // write value 0x75 to EEPROM
     ASSERT( result==OK );

     i2c_stop();                                  // set stop conditon = release bus
     ASSERT( result==OK );

 

which emphasises the possibility that calls may fail, so some error checking/handling is required - without going into the details & confusing the issue ... ?

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: Mon. May 24, 2021 - 11:32 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
If you are happy driving a car with your eyes shut,  just carry on driving.

My eyes are open now. 👀

 

Thanks clawsonawneil, understood everything. yes

 

 

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

 

   And then the Devil got involved

 

Not known as a designer. And, btw why you lot use a capital d, he does not deserve it.