understanding twi library

Hello,

I’m trying to understand the twi library. To fix my I2C code, which is about handling NACK after receiving the last byte of data_read function.

I started to understand the general arrangement of the library.
But, now I’m in front of the code design.
The twi.c file is in the attachment.

I have two questions, first one is about a function, the second about the code handling between functions and ISR.

  1. I want to understand this while condition, how it works?
  while(TWI_READY != twi_state){
    continue;
  }
  twi_state = TWI_MRX;

I tried to do examples about while loop with break and continue in codeblocks and see the results, I understand it works as a lock.
My understanding about this three lines of code:
When the while checks for the condition, if “twi_state” is 0; then it doesn’t enter the while loop at all. And go to the rest of the read function, is that right? Or if the condition is not true then it locks until the condition is true.
But, what are the factors which affect “twi_state”?

Because, the third line updates the “twi_state” right after the while condition! I don’t understand that? Why this update comes after the while condition?

  1. My second question is, what’s the difference between the twi functions and ISR? I didn’t understand the relationship between them.

twi.c (16.8 KB)

I'm trying to understand the twi library.

which one?

the one included is this one using the [url=https://github.com/arduino/Arduino/blob/cd9a15ebd1647336200b9328bc0912495e3a7195/hardware/arduino/avr/libraries/Wire/src/utility/twi.c#L399]ISR(TWI_vect)[/url] notation

or are you referring to is the ninjablocks/arduino/Wire one (which is 6 year old)

you are right that twi_state is changed from underneath you through interrupts

In the old code the ISR is in [url=https://github.com/ninjablocks/arduino/blob/f3604787b5e6293236afd4e54b66cff589e45135/Wire/utility/twi.c#L326-L326]SIGNAL(TWI_vect)[/url]

Now if you go read the details in the AVR doc for interrupts you will see that they state (at the very end of the page)

#define SIGNAL (vector)
Introduces an interrupt handler function that runs with global interrupts initially disabled.

This is the same as the ISR macro without optional attributes.

Deprecated:
Do not use SIGNAL() in new code. Use ISR() instead.

so that's why the embedded one use ISR() now


as you mention NACK, in that twi.c file they state the following which might be helpful?

twi_masterBufferLength = length-1; // This is not intuitive, read on...
// On receive, the previously configured ACK/NACK setting is transmitted in
// response to the received byte before the interrupt is signalled.
// Therefore we must actually set NACK when the next to last byte is
// received, causing that NACK to be sent in response to receiving the last
// expected byte of data.

Yes, I'm referring to current twi library for AVR which has ISR(TWI_vect).

OK, this library contain a lot of C coding skills for me as a beginner.

So, I looked over and over to understand and connect things in the code.

I started to understand some techniques of the code and I knew that I2C needs more care for very efficient code.

Now, the things I want to understand:

  1. How ISR(TWI_vect) is triggered?
  2. What the things that ISR do and things which the actual functions do; like,
    twi_readFrom();? I see that the ISR do the sending, receiving the data, filling the buffers. If the ISR do all the work then what's the role of the actual functions?
  3. How the ISRs do the operations after receiving the value of the TWSR? Because what I know is that the value of TWSR is the result of what happened not the condition to operate the function.
  1. How ISR(TWI_vect) is triggered?

It's triggered automatically by defining the right vector for the TWI interrupt.

The ISR() macro helps introduce an interrupt handler function (interrupt service routine = ISR). The vector must be one of the interrupt vector names that are valid for the particular micro-controller, here using TWI_vect to state that this is the function being called when a TWI interrupt is triggered

in "standard" (higher level) arduino coding this is obfuscated by attachInterrupt(digitalPinToInterrupt(pin), ISR, mode); for more traditional code.

The attachInterrupt() function is defined here in WInterrupts.c if you want to see how it works. (helicopter view is that there is an array related to the interrupts in which an interrupt Number is mapped to a function intFunc[interruptNum] = userFunc; and there is a macro that defines the right ISR (looking into the table to find which functions to call).

#define IMPLEMENT_ISR(vect, interrupt) \
  ISR(vect) { \
    intFunc[interrupt](); \
  }

if you also look at the end of the file (and also here) you'll see that the ISR for TWI_vect is now commented out

I would refer to code to understand what twi_readFrom(); does. there is a high level description in the source

/* 
 * Function twi_readFrom
 * Desc     attempts to become twi bus master and read a
 *          series of bytes from a device on the bus
 * Input    address: 7bit i2c device address
 *          data: pointer to byte array
 *          length: number of bytes to read into array
 *          sendStop: Boolean indicating whether to send a stop at the end
 * Output   number of bytes read
 */
uint8_t twi_readFrom(uint8_t address, uint8_t* data, uint8_t length, uint8_t sendStop)

For your questions - they really pertain to the global understanding of the protocol. You could probably start with Nick's page on I2C

What is the problem ? I mean, what came up with a problem for the last NACK ? Is there a special sensor that is not compatible with the Arduino Wire library ? Did you write code using the Wire library and your code has a bug ? Do you want to use the Arduino as a Slave and it has problems with the last NACK ? Did you verify the bus signals with a logic analyzer ?
Have you heard about the XY-problem ? http://xyproblem.info/

Are you perhaps trying to write your own I2C library ? Many have tried that and many have failed. About 99.9% has failed.

To learn something, anything is better than the Wire/twi library! It is mostly confusing.

It started as a interrupt driven library. That is okay, since in the interrupt every situation is handled. The ISR is some kind of finite state machine.
But since the Wire.endTransmission() and Wire.requestFrom() wait until the I2C transaction has finished, a library that uses polling without interrupt is just as fast.

When the Master reads data from a Slave, the Master should NACK after the last databyte and after that a stop.
I'm sorry, but I don't want to look into the code and datasheet to see how that is implemented.

There are a number of loops in the Wire library. The 'twi_state' is changed in the interrupt. That should make it possible to bail out of the while-loops.
However, the Wire library assumes that in a normal running system, someday the 'twi_state' will be changed.
Try to run the i2c scanner and shortcut SDA or SCL to GND and see if you can halt the sketch.

I don't understand your question about the ISR and performing an action based on that result.
The ISR reads the status bits of the TWSR register, and takes the necassary (next) step in the process. That is the only thing that the ISR can do and should do.

I don’t want to use wire library.

Well, this is beyond my complete understanding of the connection between twi and wire libraries.

But of course twi is the base code and wire is the inherited code, and there is a compatibility between C and C++.

But I’m focusing on twi library and I know that anyone can implement any source code based on twi functions. I think wire library is just an Arduino version for twi functions with C++ features.

So I have to understand twi library to solve my problem in my I2C library.

But what I’m facing now is the design of the twi_readFrom function. I understand after looking into the code many times, that it’s handled with care.

But there’s a relationship and a difference which are not clear to me now about which functions in the twi_readFrom or the ISR which actually read and sends the data.

So, here are the parts I want to understand right now:

/*
 * Function twi_readFrom
 * Desc     attempts to become twi bus master and read a
 *          series of bytes from a device on the bus
 * Input    address: 7bit i2c device address
 *          data: pointer to byte array
 *          length: number of bytes to read into array
 *          sendStop: Boolean indicating whether to send a stop at the end
 * Output   number of bytes read
 */
uint8_t twi_readFrom(uint8_t address, uint8_t* data, uint8_t length, uint8_t sendStop)
{
  uint8_t i;

  // ensure data will fit into buffer
  if(TWI_BUFFER_LENGTH < length){
    return 0;
  }

  // wait until twi is ready, become master receiver
  while(TWI_READY != twi_state){
    continue;
  }
  twi_state = TWI_MRX;
  twi_sendStop = sendStop;
  // reset error state (0xFF.. no error occured)
  twi_error = 0xFF;

  // initialize buffer iteration vars
  twi_masterBufferIndex = 0;
  twi_masterBufferLength = length-1;  // This is not intuitive, read on...
  // On receive, the previously configured ACK/NACK setting is transmitted in
  // response to the received byte before the interrupt is signalled.
  // Therefor we must actually set NACK when the _next_ to last byte is
  // received, causing that NACK to be sent in response to receiving the last
  // expected byte of data.

  // build sla+w, slave device address + w bit
  twi_slarw = TW_READ;
  twi_slarw |= address << 1;

  if (true == twi_inRepStart) {
    // if we're in the repeated start state, then we've already sent the start,
    // (@@@ we hope), and the TWI state machine is just waiting for the address byte.
    // We need to remove ourselves from the repeated start state before we enable interrupts,
    // since the ISR is ASYNC, and we could get confused if we hit the ISR before cleaning
    // up. Also, don't enable the START interrupt. There may be one pending from the
    // repeated start that we sent ourselves, and that would really confuse things.
    twi_inRepStart = false;			// remember, we're dealing with an ASYNC ISR
    do {
      TWDR = twi_slarw;
    } while(TWCR & _BV(TWWC));
    TWCR = _BV(TWINT) | _BV(TWEA) | _BV(TWEN) | _BV(TWIE);	// enable INTs, but not START
  }
  else
    // send start condition
    TWCR = _BV(TWEN) | _BV(TWIE) | _BV(TWEA) | _BV(TWINT) | _BV(TWSTA);

  // wait for read operation to complete
  while(TWI_MRX == twi_state){
    continue;
  }

  if  (twi_masterBufferIndex < length)
    length = twi_masterBufferIndex;

  // copy twi buffer to data
  for(i = 0; i < length; ++i){
    data[i] = twi_masterBuffer[i];
  }

  return length;
}

So, this function:

  1. Has 4 parameters.
  2. First it checks if the user entered a length of the buffer more than the defined value which is 32.
  3. Then, the while loop waits for twi to be ready.
  4. Set twi_state = TWI_MRX; to be the master receiver. The next line to include the send stop option. Then next line to free of errors.
  5. Then it initializes the index variable to 0, and subtract 1 to NACK the last byte.
  6. Sets the read bit and ORing it with the address.
  7. Here’s the complicated stuff come :slight_smile: This function checks if the twi_inRepStart is true which would be set in the ISR only for Master Transmitter/Receiver ISRs, it comes as a third if statement condition if the array index is finished and the user didn’t send a stop, which means to continue and start a new transmitter operation.
  8. Next, another while loop to ensure it’s still in master transmitter and not affected by another interrupt. But what interrupt which would terminate twi functions?
  9. Next if it’s still not finished the buffer length, then it put the index value to the user included value??! But why to do that?
  10. Last thing is the for loop, which puts as I believe the “already read twi data from ISR” and saved to the twi_masterBuffer to user’s data pointer array.
  11. Finally, and this one I don’t understand why to return the length? It’s already has been entered by the user.

I studied the twi.c some weeks ago. I am a professional programmer with 25 years experience so I have seen many many source codes. The code is not ideal. Some additions where only patched-in in the years it exists. The code works if it is used over TwoWire class or with caution.

Nicholas Zambetti the original author of the library is original author of other base Arduino libraries like for example HardwareSerial.

The check which returns zero if the caller wants to read more than buffer length (your point 2) is not ok. The function should fill the users buffer. There are artifacts of code suggesting it was doing it (your point 9).

The main pach to twi code was the support of not release the twi line between 'transactions' (parameter stop).

So, you can't assume the libary code is ideal.

I studied it because I use it directly in an I2c device implementation. TwoWire class and Wire object were a unnecessary overhead for me.

After the interrupts are enabled there is a loop to wait until the data is received. That means the main part of receiving data is done in the ISR. After that, the number of received bytes is checked (length is corrected), and the data is copied.

I don't understand the way the async ISR is handled like that. I would have to spent a few hours to look into that, and I don't want to do that.

The length is corrected if an error did occur. Therefor the number of bytes that are received via the ISR is returned. The 'length' is changed, it is no longer the value of the parameter.

I can't follow what you say with point 8 and 9. Sorry.

Juraj:
I studied the twi.c some weeks ago. I am a professional programmer with 25 years experience so I have seen many many source codes. The code is not ideal. Some additions where only patched-in in the years it exists. The code works if it is used over TwoWire class or with caution.

Nicholas Zambetti the original author of the library is original author of other base Arduino libraries like for example HardwareSerial.

The check which returns zero if the caller wants to read more than buffer length (your point 2) is not ok. The function should fill the users buffer. There are artifacts of code suggesting it was doing it (your point 9).

The main pach to twi code was the support of not release the twi line between ‘transactions’ (parameter stop).

So, you can’t assume the libary code is ideal.

I studied it because I use it directly in an I2c device implementation. TwoWire class and Wire object were a unnecessary overhead for me.

That’s really good that you are a professional programmer, I need a lot of experience, hints, tips and programming recommendations. I hope to learn a lot from you :slight_smile:

So, you mean the code should work with a class, that means a C++ main code. Or with caution, means it should work alone but I have to be careful, you’re right I tried to implement my HMC5883L application code but didn’t get a clear valid results.

WOW, so “Nicholas Zambetti” is the author for all twi and wire libraries? That’s interesting! Because I was thinking about who wrote twi code, I thought it’s from Atmel development team :slight_smile: Because it’s in the AVR-GCC libraries set.

The check which returns zero if the caller wants to read more than buffer length (your point 2) is not ok. The function should fill the users buffer.

After a thorough look into that line, first I didn’t know how that line should work because the while loop never ends unless the condition terminates the while condition and then it should break it the user input length number is not larger than the defined number which is 32.

There are artifacts of code suggesting it was doing it (your point 9).

OK, now I just looked into the code and I think I understood what that piece of code is doing!

  while(TWI_MRX == twi_state){
    continue;
  }

After this condition is terminated in the ISR and read is finished.

This code comes:

  if  (twi_masterBufferIndex < length)
    length = twi_masterBufferIndex;

  // copy twi buffer to data
  for(i = 0; i < length; ++i){
    data[i] = twi_masterBuffer[i];
  }

  return length;

It checks again if the buffer index isn’t bigger than the user input value, it copy the buffer index to the user length and returns it again. And the part of code after finally copied the received data into the user pointer.

I concluded to this solution but didn’t test it.

I edited the two most important functions for now which are the I2C receive function in the I2C library, and the dataRead function in HMC5883L library.

  1. The I2C read function
void I2C_rx(uint8_t* dat, uint8_t len)
{
   uint8_t i;
   if (len > I2C_BUFFER_LENGTH)
   {
       return 0;
   }

   I2C_MxBuffLen = len - 1;        // decrementing 1 to NACK the last byte
   I2C_MxBuffIndex = 0;

    for (i=0; i<I2C_MxBuffLen;i++)
   {
       dat[i]= TWDR;                           // Coping the received bytes to the receive pointer
       TWCR = (1<<TWINT)|(1<<TWEN)|(1<<TWEA);  // sneding ACKs
       while (!(TWCR & (1<<TWINT)));
   }

       TWCR = (1<<TWINT)|(1<<TWEN);            // sending NACK
       while (!(TWCR & (1<<TWINT)));
}
  1. The HMC5883L dataRead function
void data_read (uint8_t *results_ptr, uint8_t length)
{
 uint8_t lsb,msb, status_of_process = 0;

 I2C_start(HMC5883L_write);
 I2C_tx(Data_Output_X_MSB_Register);

 I2C_start(HMC5883L_read);
 Serial.print("x-axis\t\t");Serial.print("z-axis\t\t");Serial.println("y-axis");

 I2C_rx(results_ptr,length);

 I2C_stop();
 Serial.println();
}

Should these two functions work? I’m just concerning now about passing an array pointer from a function to another function which also receives a pointer to an array, is that OK?