I2C improvement in NXC?

[ricardocrl]

Hi,

Is there anyway to improve the I2C communication speed. I read in a previous post about some kind of fast I2C that John has tried, but I also found something about "incompatibility" with 3rd party components like Hitechnic products.

I'm specifically looking for improvements on the following functions:

MMX_ReadTachometerPosition(...);
ReadSensorHTAngle(...);

Thanks a lot in advance!
Ricardo

[ricardocrl]

Ok, I've worked on 4 improvements (some of them oriented to my application):

1) I've found out that I2CBytes is slower than using a separate I2CWrite plus an I2CRead (I don't know if there is any major drawback, but I believe I2CBytes has advantages). So I used this to decrease the running time of both functions (HiTechnic angle, and mindsensors NXTMMX).

With this I decreased from 16ms to 14ms.

2) To decrease time on the NXTMMX reading function, I gathered both positions in the same packet asked through I2C. I don't really understand why this isn't the default option, since it takes the same time as asking for a single tachometer value, since they are in consecutive registers.
With this, I got half time spent to get the values: 14ms for both values.

3) I need values from 2 different Angle sensors. Because I was using the same function to retrieve info, I had to protect it, which was making them not running "at the same time" when using different threads. By duplicating them with different names, I solved this problem and got a much faster result. Thanks to this post: https://sourceforge.net/apps/phpbb/mind ... 018&p=9750

Using 2 threads reading both sensors and a third thread waiting for completion of both, I got 18,5ms per read, instead of 28ms with a single function protected by safecall.

All 4 values (2 tachometers + 2 angles) with concurrent threads are collected in around 21,5ms/read.

4) Instead of having threads in parallel getting the values all the time, I combined the readings in a single loop and saved the values in global variables. This removed any time consumed in "while" statements or whatever else. Instead of ~21,5 I got ~16,25ms.

Here are the codes for the separate functions and the function with all readings together. Note that I assumed some addresses and register values, instead of keeping them as parameters to send when calling.

Code: Select all

void ReadAngle_Custom(byte port, int &Angle, long &AccAngle, int &RPM)
{
  byte cmdbuf[] = {0x02, 0x42};
  byte buf[10];
  byte nByteReady = 0;
  byte loop, n;

  while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  I2CWrite(port, 8, cmdbuf);
  while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  n = I2CRead(port, 8, buf);

  if ( n == NO_ERR ) {
    Angle = buf[0]*2 + buf[1];
    AccAngle = buf[2]*0x1000000 + buf[3]*0x10000+
               buf[4]*0x100 + buf[5];
    RPM = buf[6]*0x100 + buf[7];
    while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  } else {
    Angle = 0; AccAngle = 0; RPM = 0;
  }
}

Code: Select all

void MMX_ReadTachometers(byte port, long & tacho1, long & tacho2)
{
  byte cmdbuf[] = {0x06, 0x62};
  unsigned byte buf[10];
  byte nByteReady = 0;
  byte n;

  while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  n = I2CWrite(port, 8, cmdbuf);
  while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  n = I2CRead(port, 8, buf);

  if(n == NO_ERR) {
    tacho1 = buf[0] + (buf[1]<<8) + (buf[2]<<16) + (buf[3]<<24);
    tacho2 = buf[4] + (buf[5]<<8) + (buf[6]<<16) + (buf[7]<<24);
    while (I2CStatus(port, nByteReady) ==  STAT_COMM_PENDING);
  } else {
    tacho1 = 0; tacho2 = 0;
  }
}

Code: Select all

void ReadAllValues(byte a1port, byte a2port, byte mmxport){
  byte cmdbuf1[] = {0x02, 0x42}; // Angle1
  byte cmdbuf2[] = {0x02, 0x42}; // Angle2
  byte cmdbuf3[] = {0x06, 0x62}; // MMX
  byte buf1[10], buf2[10], buf3[10];
  byte nByteReady1 = 0, nByteReady2 = 0, nByteReady3 = 0;
  byte n1, n2, n3;
  byte ready1 = 0, ready2 = 0, ready3 = 0;

  while (I2CStatus(a1port, nByteReady1) ==  STAT_COMM_PENDING);
  I2CWrite(a1port, 8, cmdbuf1);
  while (I2CStatus(a2port, nByteReady2) ==  STAT_COMM_PENDING);
  I2CWrite(a2port, 8, cmdbuf2);
  while (I2CStatus(mmxport, nByteReady3) ==  STAT_COMM_PENDING);
  I2CWrite(mmxport, 8, cmdbuf3);

  while (I2CStatus(a1port, nByteReady1) ==  STAT_COMM_PENDING);
  n1 = I2CRead(a1port, 8, buf1);
  while (I2CStatus(a2port, nByteReady2) ==  STAT_COMM_PENDING);
  n2 = I2CRead(a2port, 8, buf2);
  while (I2CStatus(mmxport, nByteReady3) ==  STAT_COMM_PENDING);
  n3 = I2CRead(mmxport, 8, buf3);

  if (n1 == NO_ERR){
    angle1 = buf1[0]*2 + buf1[1];
    accAngle1 = buf1[2]*0x1000000 + buf1[3]*0x10000+
           buf1[4]*0x100 + buf1[5];
    rpm1 = buf1[6]*0x100 + buf1[7];
  } else {
    angle1 = 0; accAngle1 = 0; rpm1 = 0;
  }

  if (n2 == NO_ERR){
    angle2 = buf2[0]*2 + buf2[1];
    accAngle2 = buf2[2]*0x1000000 + buf2[3]*0x10000+
           buf2[4]*0x100 + buf2[5];
    rpm2 = buf2[6]*0x100 + buf2[7];
  } else {
    angle2 = 0; accAngle2 = 0; rpm2 = 0;
  }
  
  if (n3 == NO_ERR){
    tacho1 = buf3[0] + (buf3[1]<<8) + (buf3[2]<<16) + (buf3[3]<<24);
    tacho2 = buf3[4] + (buf3[5]<<8) + (buf3[6]<<16) + (buf3[7]<<24);
  } else {
    tacho1 = 0; tacho2 = 0;
  }
}

Also note that I don't care if I have only one ready to read. The variables are only updated after all of them are read from the I2C devices, because synchronization is helpful in my application. Besides that, in my case, they take approximately the same time to read. One could keep reading "asynchronously". Every time a reading was complete, the I2CWrite would be sent again, in an infinite loop.

[mightor]

Ricardo,

Please note that the HiTechnic angle sensor does not support speeds faster than the official ~10KHz I2C clock. It may work at faster speeds, but it is not guaranteed or supported.

- Xander

[ricardocrl]

OK, nice to have the confirmation. But I don't even know how to use a faster routine to read/write I2C data. Can you point me to such information?

Anyway, with the other tweaks, I think the performance is good enough. But I'm curious about such option for other devices..

[afanofosc]

1) I've found out that I2CBytes is slower than using a separate I2CWrite plus an I2CRead (I don't know if there is any major drawback, but I believe I2CBytes has advantages). So I used this to decrease the running time of both functions (HiTechnic angle, and mindsensors NXTMMX).

I don't understand this since ReadSensorHTAngle from the official NXC API does not call I2CBytes. Are you talking about some other function called ReadSensorHTAngle? Also, can you mention what version of the compiler you have installed, i.e., are you running the latest test release build or some older official release?

3) I need values from 2 different Angle sensors. Because I was using the same function to retrieve info, I had to protect it, which was making them not running "at the same time" when using different threads. By duplicating them with different names, I solved this problem and got a much faster result.

If you used the official ReadSensorHTAngle function from the NXC API you would not have this problem as the official API function is thread safe and can be called simultaneously from multiple threads so long as you use a constant port number (i.e., S1 rather than a variable containing S1). There's no need to pass the port around your program via function parameters if you just use #defines at the top of your code to define the configuration of your robot's sensors.

Using 2 threads reading both sensors and a third thread waiting for completion of both, I got 18,5ms per read, instead of 28ms with a single function protected by safecall.

This sounds complicated. Can you post code that shows what you are doing here? I suspect there are much better options. I would like to see how you are timing things - especially whether you are writing to the LCD at all inside your timing loops.

Support for high-speed I2C has its drawbacks since it hogs the CPU while completing the I2C transaction at the fast baud rate. Depending on your application this can negatively impact other modules in the firmware - such as motor control.

John Hansen

[ricardocrl]

I don't understand this since ReadSensorHTAngle from the official NXC API does not call I2CBytes. Are you talking about some other function called ReadSensorHTAngle? Also, can you mention what version of the compiler you have installed, i.e., are you running the latest test release build or some older official release?

I'm using version 1.2.1.r4. I didn't change or used any other ReadSensorHTAngle() function. I assumed it uses I2CBytes() function, because that's what is present in the code available on HiTechnic website: http://www.hitechnic.com/cgi-bin/commer ... ey=NAA1030 Maybe it has been modified when incorporated in the NXC API.
Well, at least I noticed the difference from 16ms to 14ms.

If you used the official ReadSensorHTAngle function from the NXC API you would not have this problem as the official API function is thread safe and can be called simultaneously from multiple threads so long as you use a constant port number (i.e., S1 rather than a variable containing S1). There's no need to pass the port around your program via function parameters if you just use #defines at the top of your code to define the configuration of your robot's sensors.

Maybe your right. I only got the this problem when I got the code from HiTechnic site and modified it with I2CWrite and I2CRead. After that, the function was not protected. After protecting it with "safecall" I couldn't call the symultaneously. How is that implemented in the NXC API?

Using 2 threads reading both sensors and a third thread waiting for completion of both, I got 18,5ms per read, instead of 28ms with a single function protected by safecall.

This sounds complicated. Can you post code that shows what you are doing here? I suspect there are much better options. I would like to see how you are timing things - especially whether you are writing to the LCD at all inside your timing loops.

The usage of 3 threads is not so complicated, I guess. It's like this:

Code: Select all

task ReadAngle1(){
  long accAngle;
  int angle, rpm;
  
  for (int i = 0; i<100; i++){
    ReadAngle1_Custom(S1, angle, accAngle, rpm);
  }
  angle1Done = 1;
}

task ReadAngle2(){
  long accAngle;
  int angle, rpm;

  for (int i = 0; i<100; i++){
    ReadAngle1_Custom(S4, angle, accAngle, rpm);
  }
  angle2Done = 1;
}

task main(){
  long accAngle;
  int angle, rpm;
  
  SetSensorLowspeed(ANGLE_PORT_M1);
  SetSensorLowspeed(ANGLE_PORT_M2);
  
  long tic = CurrentTick();

  start ReadAngle1;
  start ReadAngle2;
  while(!angle1Done || !angle2Done);

  NumOut(0, LCD_LINE1, (CurrentTick()-tic)/100.0);
  Wait(3000);

}

...where ReadAngle1_Custom() and ReadAngle2_Custom() are implemented as stated previously. This was just a setup for testing the sample rate.

But now I got surprised! With this I got 18.5ms. If I use the official ReadSensorHTAngle() I get ~18.0ms. But, if I only compare readings of one port, like:

Code: Select all

  for (int i = 0; i<100; i++){
    ReadSensorHTAngle(S1, angle, accAngle, rpm);
  }

vs.

Code: Select all

  for (int i = 0; i<100; i++){
    ReadAngle1_Custom(S1, angle, accAngle, rpm);
  }

..I get 14ms against 16ms for the HiTechnic one. Why is that?

[afanofosc]

I'm using version 1.2.1.r4.

So you are not using a test release at all, then. You might want to consider doing so as there have been a number of API improvements since 1.2.1.r4 was released.

I didn't change or used any other ReadSensorHTAngle() function. I assumed it uses I2CBytes() function, because that's what is present in the code available on HiTechnic website

So are you using the code from the HiTechnic website or are you using the function built into the NXC API headers? I can't tell from what you said. In other words, do you include a header file that defines a function called ReadSensorHTAngle that calls I2CBytes or does the compiler just seem to know how to compile a reference to that function without you including its definition explicitly?

How is that implemented in the NXC API?

The function is thread-safe in the NXC API because it is inline except for the guts of the I2C code, in which case it uses a different subroutine based on the constant port specified (i.e., S1 or S4). So a call to ReadSensorHTAngle(S1,...) would expand inline and include a call to __ReadLSBytes0 while a call to ReadSensorHTAngle(S4,...) would call __ReadLSBytes3 instead. Each of these subroutines, which are defined in NXTDefs.h, use a different set of variables so there is no problem with simultaneous calls from different threads.

The usage of 3 threads is not so complicated, I guess. It's like this:

Code: Select all

  long tic = CurrentTick();

  start ReadAngle1;
  start ReadAngle2;
  while(!angle1Done || !angle2Done);

  NumOut(0, LCD_LINE1, (CurrentTick()-tic)/100.0);

This test times a) how long it takes the firmware to start up 2 threads, b) how long a for loop takes to keep checking its iterator, c) how long it takes for a third thread to spin around in an empty while loop while two other threads are trying to do their job. So it is not just timing how long it takes to do an 8 byte I2C read. Most of the time it takes to do an I2C transaction is waiting for the firmware to tell you that the bytes are ready from the device.

But now I got surprised! With this I got 18.5ms. If I use the official ReadSensorHTAngle() I get ~18.0ms. But, if I only compare readings of one port, like:

Code: Select all

  for (int i = 0; i<100; i++){
    ReadSensorHTAngle(S1, angle, accAngle, rpm);
  }

vs.

Code: Select all

  for (int i = 0; i<100; i++){
    ReadAngle1_Custom(S1, angle, accAngle, rpm);
  }

..I get 14ms against 16ms for the HiTechnic one. Why is that?

I can't tell if you are saying ReadSensorHTAngle took 14ms and ReadAngle1_Custom took 16 or vice versa. I would try upgrading to the latest test release if you can (depending on what platform you are running on) and use repeat(100) {} for any timing tests since that looping construct has the least amount of overhead. I would be surprised if any function written in NXC is faster than ReadSensorHTAngle which is implemented in optimized NBC code.

John Hansen

[ricardocrl]

OK, sorry if I was unclear. Let's see...

So you are not using a test release at all, then. You might want to consider doing so as there have been a number of API improvements since 1.2.1.r4 was released.

Well, you were right. There was a big change. Using the new test release and the official ReadSensorHTAngle() from the NXC API, I don't get 16ms any more. I get ~13,35 on average! By using repeat(100) as you suggested, I reduced the average time to ~13,05. Great!

I didn't change or used any other ReadSensorHTAngle() function. I assumed it uses I2CBytes() function, because that's what is present in the code available on HiTechnic website

I mean that I'm using the NXC API that was already included. I didn't add any definition/header with a new one. But, after checking HiTechnic website, I thought the official NXC API was using I2CBytes. Then, I decided to try I2CWrite and I2CRead instead of the ReadSensorHTAngle(). With that, I got 14ms on average (also with the new test release). Now I see that the official one takes less, around 13,35ms (not using repeat()).

Again back to I2CBytes vs I2CRead/I2CWrite. I2CBytes, with the new test release is taking less time than Read/Write functions. Not updating the compiler really messed my analysis, sorry for this.

The function is thread-safe in the NXC API because it is inline except for the guts of the I2C code, in which case it uses a different subroutine based on the constant port specified (i.e., S1 or S4). So a call to ReadSensorHTAngle(S1,...) would expand inline and include a call to __ReadLSBytes0 while a call to ReadSensorHTAngle(S4,...) would call __ReadLSBytes3 instead. Each of these subroutines, which are defined in NXTDefs.h, use a different set of variables so there is no problem with simultaneous calls from different threads.

Ah, now I understand. Thanks for the explanation!

The usage of 3 threads is not so complicated, I guess. It's like this:

Code: Select all

  long tic = CurrentTick();

  start ReadAngle1;
  start ReadAngle2;
  while(!angle1Done || !angle2Done);

  NumOut(0, LCD_LINE1, (CurrentTick()-tic)/100.0);

This test times a) how long it takes the firmware to start up 2 threads, b) how long a for loop takes to keep checking its iterator, c) how long it takes for a third thread to spin around in an empty while loop while two other threads are trying to do their job. So it is not just timing how long it takes to do an 8 byte I2C read. Most of the time it takes to do an I2C transaction is waiting for the firmware to tell you that the bytes are ready from the device.

I knew that there were a lot of CPU consuming with this methodology. That's why I implemented a sequenced I2CWrite to request data from different ports, followed by a sequenced I2CRead to get all values, in a single thread. This is the code:

Code: Select all

void ReadAllAngles(byte a1port, byte a2port){
  byte cmdbuf1[] = {0x02, 0x42}; // Angle1
  byte cmdbuf2[] = {0x02, 0x42}; // Angle2
  byte buf1[10], buf2[10];
  byte nByteReady1 = 0, nByteReady2 = 0;
  byte n1, n2;
  byte ready1 = 0, ready2 = 0;

  while (I2CStatus(a1port, nByteReady1) ==  STAT_COMM_PENDING);
  I2CWrite(a1port, 8, cmdbuf1);
  while (I2CStatus(a2port, nByteReady2) ==  STAT_COMM_PENDING);
  I2CWrite(a2port, 8, cmdbuf2);

  while (I2CStatus(a1port, nByteReady1) ==  STAT_COMM_PENDING);
  n1 = I2CRead(a1port, 8, buf1);
  while (I2CStatus(a2port, nByteReady2) ==  STAT_COMM_PENDING);
  n2 = I2CRead(a2port, 8, buf2);

  if (n1 == NO_ERR){
    angle1 = buf1[0]*2 + buf1[1];
    accAngle1 = buf1[2]*0x1000000 + buf1[3]*0x10000+
           buf1[4]*0x100 + buf1[5];
    rpm1 = buf1[6]*0x100 + buf1[7];
  } else {
    angle1 = 0; accAngle1 = 0; rpm1 = 0;
  }

  if (n2 == NO_ERR){
    angle2 = buf2[0]*2 + buf2[1];
    accAngle2 = buf2[2]*0x1000000 + buf2[3]*0x10000+
           buf2[4]*0x100 + buf2[5];
    rpm2 = buf2[6]*0x100 + buf2[7];
  } else {
    angle2 = 0; accAngle2 = 0; rpm2 = 0;
  }
}

With this code, I read both angle sensors in ~15ms. Do you think I could do better? Or maybe the same, but "cleaner" (less code, less complicated or so...)?