nox771 / i2c_t3

Enhanced I2C library for Teensy 3.x devices
157 stars 44 forks source link

define I2C_AUTO_RETRY, added resetBusCountRead() of times resetBus ex… #8

Closed systronix closed 7 years ago

systronix commented 7 years ago

Please Ignore the two changes of I2C clock speed (I don't know how to keep those out of a pull request). The other two are use of a resetBusCount, very minimal, adds no overhead to normal execution. I find it useful to know when auto retry has taken place and how many time the resetBus was needed. Thanks for a wonderful library!

nox771 commented 7 years ago

I'd like to add in a more generic error read function to cover more things than just resetBus. I know this was discussed previously on Teensy forum. I haven't forgotten about this, just been too busy. I'll see if I can push it up in priority. I'll check the changes you submitted and use it as a reference. I'll probably make a test version and get feedback on the forum before committing here.

nox771 commented 7 years ago

I've pushed a new branch which is heavily modified from the last v9.4 master release. It has a number of new things, one of which is an error tracking system. I don't have docs written on it yet, but it will track a number of different errors and can be optionally switched off via user define.

If you do a pull on branch 'develop' then look in the i2c_t3.h file, you will see a new enum i2c_err_count. Those enum values can be used with two new functions. You can retrieve an error count like this (this is a resetBus example): uint32_t count = getErrorCount(I2C_ERRCNT_RESET_BUS); And you can zero the count like this: zeroErrorCount(I2C_ERRCNT_RESET_BUS);

There is a user define I2C_ERROR_COUNTERS near the top of the i2c_t3.h file which I will default on, but it can be commented out to remove the counter code for anyone who has a performance concern.

Other important changes I've done are unbinding SCL/SDA pin assignment, and adding several Maser callbacks for handling background transfers and errors. Once it is fully tested I'll update the docs on these new features and do a more detailed forum post on PJRC.

I'm still testing and debugging on this branch, so let me know if you test and run into any problems.

systronix commented 7 years ago

OK I will pull this and try it.

Is I2C_AUTO_RETRY now defined by default? I can't imagine when you would not want that to be so.

I have an adafruit PCA9548A breakout with Teensy3.2 now running long term test, writing and reading every possible value to the control register, well actually walking a '1' so that only a single output turns on at a time. That's an odd part because you can turn on more than one output at the same time, a bit odd for an I2C MUX. Here are results so far:

et:4294967 Good:31534583680 7342/sec bad:75 resetBus: 0

yes: 31.5 x 10e9 cycles through the test, running for 4.29 x 10e6 seconds. Failure rate is about 1 per 500x10e6 cycles which is not that great, short of 1 ppb which is considered the minimum for things like aircraft and nuclear power plants (well, see how well that worked at Fukushima).

What's interesting to me is that there are 75 failures but none required resetting the bus. The kinds of failures vary too. I haven't logged them all: that version is on my todo list with logging to uSD card vs TyQt serial log files.

Here is some detail:

https://github.com/systronix/Systronix_PCA9548A/blob/master/examples/PCA9548A_Test/PCA9548A_Test_logs.md

The breakout board is more error prone than our own PC layout. I should run some extended I2C tests on it now that we are in production and actually have enough on hand to use ourselves.

Our product uses i2c_t3 as the backbone of communication to several PCA9557 which twiddle outputs to create an oddball bitstream which talks to several AC and DC load I/O boards. About a month ago the hardware started shipping to installations across the US.

Thanks for the heads up and for writing SUCH A GREAT LIBRARY. Seriously. If only all the Arduino code was as well written, commented, and documented as i2c_t3...

Dinner or whatever is on me if we ever meet. I may be flying into Austin or San Antonio in a month or so...

Wondering if you develop on Windows, MAC, or Linux? I'm finding Linux tool setup to not be easy.

Thanks again

Bruce Boyes Systronix Inc

On Wed, Oct 11, 2017 at 11:22 PM, Brian notifications@github.com wrote:

I've pushed a new branch which is heavily modified from the last v9.4 master release. It has a number of new things, one of which is an error tracking system. I don't have docs written on it yet, but it will track a number of different errors and can be optionally switched off via user define.

If you do a pull on branch 'develop' then look in the i2c_t3.h file, you will see a new enum i2c_err_count. Those enum values can be used with two new functions. You can retrieve an error count like this (this is a resetBus example): uint32_t count = getErrorCount(I2C_ERRCNT_RESET_BUS); And you can zero the count like this: zeroErrorCount(I2C_ERRCNT_RESET_BUS);

There is a user define I2C_ERROR_COUNTERS near the top of the i2c_t3.h file which I will default on, but it can be commented out to remove the counter code for anyone who has a performance concern.

Other important changes I've done are unbinding SCL/SDA pin assignment, and adding several Maser callbacks for handling background transfers and errors. Once it is fully tested I'll update the docs on these new features and do a more detailed forum post on PJRC.

I'm still testing and debugging on this branch, so let me know if you test and run into any problems.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nox771/i2c_t3/pull/8#issuecomment-336023830, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6FTvu91d0W7WZl2IgZ8VBos9Gkyg_Vks5sraH6gaJpZM4NFR63 .

nox771 commented 7 years ago

No I2C_AUTO_RETRY is off by default. Some people complained that it was causing problems so I changed the default to off, but of course it is easily enabled.

That is quite a lot of data. I think you have done more long term testing on the lib than anyone else. I'm guessing the bad results are where auto-retry triggers? But ultimately the transfer did work after retry? The address NAK errors are curious, maybe due to crosstalk? Or perhaps some issue with mux changing target.

I've always wondered if the all the other interrupt activity on the part would cause issues. Many times I've seen Serial (USB) bus or similar background activity interrupt during transfer and stretch out the signals. In theory I2C shouldn't care about it, but I can imagine some parts might be sensitive.

FYI - either you or someone else reported seeing ARBL errors on occasion. I've found what causes this. IIRC, disconnecting the pullup off SDA can cause it to throw ARBL errors, and disconnecting off SCL will give Timeouts. Anyway it is a pullup issue.

Also FYI - Here is a test sketch I use to test the Master callbacks and the error tracker. It will dump all the tracked errors. The onError() callback is something you might be able to use to aid in logging. It avoids the need to explicitly add error check code after every call, it's sort of a catch all. In the errorEvent() handler I call resetBus() on certain errors, as I find it helps unlock the bus. I suppose it could just be called on all errors too. Callbacks work on both blocking and non-blocking forms.

// -------------------------------------------------------------------------------------------
// Basic Master Callback
// -------------------------------------------------------------------------------------------
//
// This creates a simple I2C Master device which when triggered will send/receive a text 
// string to/from a Slave device.  It is intended to pair with a Slave device running the 
// basic_slave sketch.  This sketch uses callbacks to handle the results and errors, and 
// can optionally use background transfers.
//
// Pull pin12 input low to send.
// Pull pin11 input low to receive.
// Pull pin10 input low to dump error diagnostics.
//
// This example code is in the public domain.
//
// -------------------------------------------------------------------------------------------

#include <i2c_t3.h>

// Memory
#define MEM_LEN 256
char databuf[MEM_LEN];
int count;

//#define NONBLOCKING

void setup()
{
    pinMode(LED_BUILTIN,OUTPUT);    // LED
    digitalWrite(LED_BUILTIN,LOW);  // LED off
    pinMode(12,INPUT_PULLUP);       // Control for Send
    pinMode(11,INPUT_PULLUP);       // Control for Receive
    pinMode(10,INPUT_PULLUP);       // Control for Diag

    // Setup for Master mode, pins 18/19, external pullups, 400kHz, 50ms default timeout
    Wire.begin(I2C_MASTER, 0x00, I2C_PINS_18_19, I2C_PULLUP_EXT, 400000);
    Wire.setDefaultTimeout(50000); // 50ms

    // Data init
    memset(databuf, 0, sizeof(databuf));
    count = 0;

    Serial.begin(115200);

    Wire.onTransmitDone(transmitDone);
    Wire.onReqFromDone(requestDone);
    Wire.onError(errorEvent);
}

void loop()
{
    uint8_t target = 0x66; // target Slave address

    // Send string to Slave
    //
    if(digitalRead(12) == LOW)
    {
        digitalWrite(LED_BUILTIN,HIGH);   // LED on

        // Construct data message
        sprintf(databuf, "Data Message #%d", count++);

        // Print message
        Serial.printf("Sending to Slave: '%s' ", databuf);

        // Transmit to Slave
        Wire.beginTransmission(target);   // Slave address
        Wire.write(databuf,strlen(databuf)+1); // Write string to I2C Tx buffer (incl. string null at end)
        #if defined(NONBLOCKING)
            Wire.sendTransmission();          // Transmit to Slave, non-blocking
        #else
            Wire.endTransmission();           // Transmit to Slave, blocking
        #endif

        // After send complete, callback will print result

        digitalWrite(LED_BUILTIN,LOW);    // LED off
        delay(100);                       // 100ms delay
    }

    // Read string from Slave
    //
    if(digitalRead(11) == LOW)
    {
        digitalWrite(LED_BUILTIN,HIGH);   // LED on

        // Print message
        Serial.print("Reading from Slave: ");

        // Read from Slave
        #if defined(NONBLOCKING)
            Wire.sendRequest(target, (size_t)MEM_LEN); // Read from Slave (string len unknown, request full buffer), non-blocking
        #else
            Wire.requestFrom(target, (size_t)MEM_LEN); // Read from Slave (string len unknown, request full buffer), blocking
        #endif

        // After request complete, callback will print result

        digitalWrite(LED_BUILTIN,LOW);    // LED off
        delay(100);                       // 100ms delay
    }

    // Diagnostics
    //
    if(digitalRead(10) == LOW)
    {
        digitalWrite(LED_BUILTIN,HIGH);   // LED on

        // Print errors
        Serial.printf("I2C_ERRCNT_RESET_BUS: %d\n", Wire.getErrorCount(I2C_ERRCNT_RESET_BUS));
        Serial.printf("I2C_ERRCNT_TIMEOUT:   %d\n", Wire.getErrorCount(I2C_ERRCNT_TIMEOUT));
        Serial.printf("I2C_ERRCNT_ADDR_NAK:  %d\n", Wire.getErrorCount(I2C_ERRCNT_ADDR_NAK));
        Serial.printf("I2C_ERRCNT_DATA_NAK:  %d\n", Wire.getErrorCount(I2C_ERRCNT_DATA_NAK));
        Serial.printf("I2C_ERRCNT_ARBL:      %d\n", Wire.getErrorCount(I2C_ERRCNT_ARBL));
        Serial.printf("I2C_ERRCNT_NOT_ACQ:   %d\n", Wire.getErrorCount(I2C_ERRCNT_NOT_ACQ));
        Serial.printf("I2C_ERRCNT_DMA_ERR:   %d\n", Wire.getErrorCount(I2C_ERRCNT_DMA_ERR));

        // uncomment to zero all errors after dumping
        //for(uint8_t i=0; i < I2C_ERRCNT_DMA_ERR; i++) Wire.zeroErrorCount((i2c_err_count)i);

        digitalWrite(LED_BUILTIN,LOW);    // LED off
        delay(100);                       // 100ms delay
    }
}

//
// Trigger after Tx complete (outgoing I2C data)
//
void transmitDone(void)
{
    Serial.print("OK\n");
}

//
// Trigger after Rx complete (incoming I2C data)
//
void requestDone(void)
{
    Wire.read(databuf, Wire.available());
    Serial.printf("'%s' OK\n",databuf);
}

//
// Trigger on I2C Error
//
void errorEvent(void)
{
    Serial.print("FAIL - ");
    switch(Wire.status())
    {
    case I2C_TIMEOUT:  Serial.print("I2C timeout\n"); Wire.resetBus(); break;
    case I2C_ADDR_NAK: Serial.print("Slave addr not acknowledged\n"); break;
    case I2C_DATA_NAK: Serial.print("Slave data not acknowledged\n"); break;
    case I2C_ARB_LOST: Serial.print("Arbitration Lost, possible pullup problem\n"); Wire.resetBus(); break;
    case I2C_BUF_OVF:  Serial.print("I2C buffer overflow\n"); break;
    case I2C_NOT_ACQ:  Serial.print("Cannot acquire bus, possible stuck SDA/SCL\n"); Wire.resetBus(); break;
    case I2C_DMA_ERR:  Serial.print("DMA Error\n"); break;
    default:           break;
    }
}

As far as devel environment, right now I'm using Windows. I have a Linux setup I use for unrelated Verilog stuff, but for this it is Windows. I'm actually looking for a better code editor right now. Been looking at UltraEdit, Sublime, Atom, VSCode, and such, but they are all lacking one thing or another. I wish someone would implement NEdit's mouse-driven block editing into a modern editor, nobody does it as good as that.

nox771 commented 7 years ago

Sorry had a rx index bug in the slave code. I just pushed a fix to develop. Pull that branch again if you are using it.

systronix commented 7 years ago

I'm buried in production issues for the next few days then head to Arm TechCon in SJC next week for a day of embed training and chance to chat with vendors there at the show. They just bought Arduino as you probably know.

I'll be in San Antonio and Kerrville Nov 16-19 to visit my father. Planning to spend a day in Austin at various museums. Maybe meet up with you for Lunch or ??? on Fri or Sat (haven't decided which day we will be in Austin)?

We don't want to change i2c_t3 this late in product dev cycle at least not without careful examination so I need to be careful to keep existing and new separate and not accidentally change known stable builds. I want to start long term test with our hardware and board layout but we keep having to ship every working copy of everything to meet some urgent need of the week while we try to ramp up production. Still, good problems to have.

best regards

Bruce Boyes

nox771 commented 7 years ago

I've pushed the new develop branch to master branch. This has all the new stuff for the error tracking system. Details are on the forum here: https://forum.pjrc.com/threads/21680-New-I2C-library-for-Teensy3?p=157187&viewfull=1#post157187

I'm going to close this pull request as the changes should address this. If you have trouble with it open a new issue or post on PJRC forum.

All previous releases are still available in the archive directory if you need them.

Sorry I won't be available in November. I'll actually be out of state visiting family. One reason I'm trying to close all this now. Good luck with your new product.

systronix commented 7 years ago

OK thanks, looking forward to working with the new version, in a week or two.

Bruce