Richard-Gemmell / teensy4_i2c

An I2C library for the Teensy 4. Provides slave and master mode.
MIT License
92 stars 19 forks source link

Simultaneous use of Master (0) and Slave1 #11

Closed mr-j-tree closed 3 years ago

mr-j-tree commented 3 years ago

Hello!

I have just started developing with teensy4_i2c on my Teensy 4.1 boards and so far it is performing really well! Thanks!

This is probably not actually an issue, but I'm unsure how this is supposed to work and hoping you can explain. I have the following setup:

Board 1: Teensy 4.1 acting as only I2C Master, using Master (pins 18/19). Board 2: Teensy 4.1 acting as I2C master for an attached LED driver using Master (pins 18/19), and I2C Slave to receive reads/writes from Board 1 on Slave1 (pins 16/17).

Both I2C busses work perfectly, except in one scenario. If I register a handler for reads or writes on the I2C buss that interconnects the two boards, then inside that handler I have code that sends data on the other I2C buss (to the LED driver) then those writes fail. If, however, I allow control to pass out of the registered read/write handler back to the main loop and then use the other I2C buss from there, both I2C busses continue working perfectly.

I'm only using the nice I2CDevice wrappers, with code that is structured the same as the provided example in the library.

I guess my understanding of the insides of the library and also I2C is not excellent, but I'm wondering if maybe the code inside the read/write handler causes something to happen in terms of clock stretching that prevents the other I2C buss from being used at the same time?

Thanks in advance for your help!

Richard-Gemmell commented 3 years ago

Hi Josh,

I'm glad to hear that the library is useful for you.

I'll start by discussing the design of these classes. I'll talk about a single I2C connection with a master at one end and a slave at the other. (Like the link between your two Teensies.)

The most important thing to know is that I2C communications on the Teensy 4 are asynchronous and happen in parallel to application code. It's all done by dedicated hardware calling hidden interrupt service routines. This is true of a lot of IO on this CPU.

I2CDevice::read() and write() make I2C synchronous by blocking. (See finish() in i2c_device.h) I wrote them this way to make them convenient to call in loop(). The downside is that it's not safe to call them from an interrupt service routine. They take too much time. Worse than that, they'll block all other ISRs that have the same priority level. (Only one ISR runs at a time.)

I wrote I2CRegisterSlave on the assumption that the vast majority of the work will be done in loop(). I added I2CRegisterSlave::after_read() so that the slave can mark the data when it's been read. This is useful for 2 reasons. Firstly, the master can use the flag to see if it's seen this set of data already. Secondly, it can help to solve the partial read problem where the master reads the data while the slave is in the process of updating it. This means that the master gets part one set of data and part of the next.

I added I2CRegisterSlave::after_write() for symmetry really.

What I didn't say in the header file is that after_read() and after_write() are called inside one those hidden interrupt service routines that I mentioned. So it's not safe to call any I2CDevice methods from them. I suspect that's what happened in your case. I'll update the documentation on i2c_register_slave to warn about this problem.

In short, do as much work as possible in loop() and do the bare minimum in after_read() and after_write(). Certainly don't do any IO in those callbacks. I think you've discovered that already. :)

There's one other possible problem I thought of. I2CRegisterSlave registers callbacks on the underlying I2CSlave object. If you register your own after_receive(), before_transmit() or after_transmit() callbacks then you'll stop I2CRegisterSlave doing anything useful.

Please let me know if this answers your question. If not, please send me the simplest possible code which triggers the behaviour and I'll dig a bit further.

cheers, Richard

mr-j-tree commented 3 years ago

Wow thanks Richard, that's exactly what I needed to know! Thanks for taking the time to write this up. I definitely think some form of this info would be helpful for others in the Readme or docs.

I'll definitely adjust my model for what code is executed where and try to move as much as possible to the main loop().

I'm only using your higher level after_read() and after_write() so your idea about after_receive() etc probably doesn't apply to my situation, but that's good to know! So far I've managed to only use your higher level wrappers, and I'd like to keep it that way.

One further question (if I may!)... from my reading elsewhere about how I2C works, my understanding of clock stretching is that you can have a slave device that takes some amount of time to perform its work based on received changes to register values, and the master is effectively aware of this because the slave prevents the clock from rising/falling until it has finished its work... thus the whole I2C buss is effectively prevented from moving on until the slave's work is done. This is the general idea of how my system needs to work, and whilst I understand that I can achieve this through read flags while still minimising code in my interrupt routines, I'm wondering what happens with regard to the clock while these interrupt routines are running? Does this stretching behaviour happen during a long running ISR? Or is that a lower level thing that has no bearing here, given the asynchronous nature of I2C on the IMXRT1062?

Cheers,

Josh

Richard-Gemmell commented 3 years ago

Hi Josh,

I was hoping these wrappers would help someone. It's great to hear that they're useful to you.

As I understand it, clock stretching is a low level implementation detail from your point of view. I think it exists so that very, very slow slave devices can reduce the I2C bus speed to give themselves time to process the next I2C byte. For these sorts of devices, the application code and I2C code are essentially the same thing. The IMXRT1062 is so fast that the application code can just treat I2C as a low level service. To look at it differently, the master proposes an I2C clock speed but the slave is allowed to slow it down a bit. With the IMXRT1062 I'd expect the bus to run at full speed. However, if one of ISRs is a little slow then clock stretching will help by plastering over the timing gap.

If I understand you correctly, you're trying to work out how to co-ordinate the slave and the master so that certain things happen in an orderly manner. e.g. the master receives every message it never sees a partial read. You were hoping that you could stall a request from the master while the slave readies it's answer.

The good news is that you can stall the request like this using the raw API. I2CSlave::before_transmit() blocks the I2C transmission until it completes. You could use this to prepare the message you want to send.

The bad news is that this rarely solves the problem because we nearly always have to do work in loop() to gather data for the response. (Such as your I2C call on another bus.) In that case we've got the same problem in a different place. Instead of co-ordinating the master and slave we're trying to co-ordinate the callback and the loop().

To throw a further spanner in the works, I2C isn't the most reliable protocol. It doesn't have any kind of built in error correction or validation. Every now and then you'll find that a read or write fails or the data gets corrupted in transit. On one of my test beds the error rate is approx 1 in a million bytes.

The strategy I've followed for my project has been to accept that the I2C link is inherently asynchronous and unreliable and to design accordingly. For example, if I need to integrate some sensor data, I get the slave to do it. The master asks the slave for the total not the raw data. That way, it doesn't matter if the master misses a message from time to time. It still has the correct total.

Having said all that, I'm not an I2C expert. There may well be standard ways of attacking these problems that I'm not aware of.

cheers, Richard

mr-j-tree commented 3 years ago

Thanks, that's great context. I'm going all-in on a model that uses a set of readable flags that the master can use to observe when work is complete and will do 100% of the work on the main loop.

Just out of curiosity, given that the CPU only has a single core, can an ISR literally interrupt (pause) a running block of code (e.g. the main loop() or another one of my functions) or does it always run at a predictable point in the code cycle? I think that's the final link in the chain of me properly understanding what's going on... :)

mr-j-tree commented 3 years ago

Also that's interesting about the unreliability of I2C... I did a test yesterday where I left a counter incrementing at max speed while I ran an errand for an hour and it didn't error, but those were somewhat ideal conditions. I'll need to contemplate this further for what it means in the real world and how I can defend against it, if necessary. Fortunately the reason I'm using I2C is that the mission critical communication between other components of the project is happening on the SPI busses, and the I2C is for some less critical subsystems.

Richard-Gemmell commented 3 years ago

I'm 99% sure that interrupts will jump in as soon as the current CPU instruction completes. (Give or take a few CPU cycles.) A line of code like "a = b * c" probably involves 2 instructions on this CPU; one for the addition and one for the assignment. In that case, an interrupt service routine (ISR) could run in the middle of the line.

Interrupts can be triggered by specialised bits of hardware which run in parallel with the main CPU. This means that more interrupts can be raised whilst an ISR is executing. Interrupts have priority levels. If a new interrupt is the same or lower priority then it'll be executed when this ISR finishes. If it's higher priority then it'll actually interrupt the current interrupt!

This is why it's important to keep ISRs as short as possible. Slow interrupts can start to interract with each other making them even slower.

I always work on the basis that nothing is sacred. An interrupt can be squeezed into places that seem unimaginable. For example, on some 32 bit systems storing a 64 bit integer to memory takes 2 instructions. The interrupt can fire when the 64 bit variable has half the old value and half the new value.

TLDR: Interrupts can happen at any time including during an ISR.

For more details see https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/beginner-guide-on-interrupt-latency-and-interrupt-latency-of-the-arm-cortex-m-processors.

WRT I2C reliability, I found that links from one Teensy to another were much more reliable than a link from a Teensy to a Raspberry Pi. (In fact I lost hair over that one.) Some environments are much noisier than others so reliability may well be worse in production than during development.

cheers, Richard

mr-j-tree commented 3 years ago

Amazing. That's cleared it right up!

Thanks again Richard for your time! I'm off to read that ARM article you linked.... 😉

Richard-Gemmell commented 3 years ago

My pleasure.

Would you mind doing me a favour? I don't have a Teensy 4.1, only a 4.0. I've never tested port 2 which is on pins 24 and 25. Do you know if that works Ok. If not, please could you try it for me.

Thanks very much.

mr-j-tree commented 3 years ago

No worries at all! I'll wire up my spare board, try it out, and get back to you!

mr-j-tree commented 3 years ago

Tested and confirmed working on Master2 object, SCL2 on pin 24, SDA2 on pin 25.

Richard-Gemmell commented 3 years ago

Thanks Josh. That's good to know.