electronicayciencia / EasyMCP2221

The most complete Python library to interface with MCP2221(A).
https://easymcp2221.readthedocs.io/
MIT License
15 stars 3 forks source link

`I2C_write` got stuck in a loop #1

Closed rcvlr closed 1 year ago

rcvlr commented 1 year ago

Hi! First of all, thanks for this library and for its documentation, I like it a lot.

I have a script that writes 128 pages of an EEPROM in a loop. For each page it writes 257 bytes at once using I2C_write.

Everything works wellmost of the times, however, it got stuck in this loop sometimes. The root cause are severe signal intergrity issues on the I2C bus.

To avoid getting stuck in there, I added these lines of code that raise an exception. My script can then retry to write the whole 257 bytes packet.

while last_byte < len(data) and self._i2c_buffer_counter() > 0:
    #time.sleep(1e-6)
    if not self._i2c_ack():
        self.I2C_cancel()
        raise RuntimeError("I2C got stuck")
        pass

You also may have noticed that I commented time.sleep(1e-6). On Windows (but I think on any non-realtime OS) I think it's impossible to ask for such a short sleeping time, in fact, I noticed the shortest sleep time on my machine is ~12 ms, which slows down a lot the process of writing the whole EEPROM. In any case I don't see the need for any sleep time in that loop, but I may be wrong.

What do you think?

electronicayciencia commented 1 year ago

Hello, Riccardo! I'm glad you like it.

Thank you for your suggestion.

I am not sure how your code will behave with clock stretching. What if your EEPROM takes a few ms to write before ack'ing? However, I think it is better to raise a exception that getting stuck some random times.

About waiting interval, you are absolutely right. Sleep suspends the thread and relies in system's schedule to wake it up, so timing accuracy is very limited. To wait a small fixed amount of time, time.perf_counter() should be used. Like in DAC_sin example.

By the way, I am working in V1.6 right now (check out "wip" branch). I will include your code and I will be happy to get more feedback from you.

Regards.

rcvlr commented 1 year ago

I haven't thought about clock stretching, actually.

In case of clock stretching, the slave can hold the SCL down at any time, usually before or after the ACK (only after the ACK in HS mode, but HS mode is not supported by MCP). All of this is managed by the HW.

Question: does this line

 r = self.send_cmd(header + data_chunk)

returns once the I2C transaction is over, or it returns immediately?

If it returns at the end of the I2C transaction, we don't have a problem because clock stretching was handled already, and the N/ACK was already sent. If it returns immediately, maybe we can use some information returned by the CMD_POLL_STATUS_SET_PARAMETERS command to wait before calling self._i2c_ack().

electronicayciencia commented 1 year ago

According to my tests, all commands return immediately. There is a delay from command to response of 2ms on average. Regardless of the command.

For example, if you launch a 1000 bytes read command, you send a I2C READ DATA command (0x91) for the first 60 bytes chunk. MCP will call the slave, then if the slave acks, proceed to read 60 bytes into MCP's internal buffer.

When the buffer is full, you must send a I2C READ DATA – GET I2C DATA command (0x40). That comes back right at the moment, returns the 60 bytes read, clears the buffer and triggers another 60 byte read. But if you call I2C READ DATA – GET I2C DATA while the slave has not yet sent all the 60 bytes, a timeout error will be reported.

Same for I2C write cycle. That is the reason for some arbitrary sleeps in my loops. I don't fully understand MCP's I2C engine. I am thinking of use the response byte "Internal I2C Engine state (at the moment the command was issued) – useful for monitoring the I2C Engine’s status" somehow.

electronicayciencia commented 1 year ago

Got it!

By internal status byte I can tell when the MCP is reading data, when buffer is full and data is ready and some other useful states.

I hope I2C routines will be much more robust in release 1.6.

electronicayciencia commented 1 year ago

Hello.

Release 1.6 is almost ready with a lot of changes (see https://easymcp2221.readthedocs.io/en/wip/history.html).

I reworked almost all I2C code to make it faster and more robust (I hope). However, there may be some bugs and corner cases I did not found yet.

It would be great if you could check out the branch wip and tell me your impressions. There is a guide about how to install an unreleased version for testing here: https://easymcp2221.readthedocs.io/en/wip/install.html#editable-for-testing if you need it.

If it works, I'll close this issue.

rcvlr commented 1 year ago

Hi! I didn't review the whole file, but it works very well with my scripts when writing the EEPROM (x3 faster than v1.5). What did you do to make if so fast? Removing time.sleep(1e-6) in I2C_write improve x2. Where does the rest come from?

A question that I have is: if the writing of a chunk fails,

rbuf = self.send_cmd(header + list(chunk))

and you end up here

if rbuf[I2C_INTERNAL_STATUS_BYTE] in (
    I2C_ST_WRITEDATA,
    I2C_ST_WRITEDATA_WAITSEND,
    I2C_ST_WRITEDATA_ACK):
    continue

don't you end up sending it again and again until it succeeds?

electronicayciencia commented 1 year ago

The other speed optimization is not polling I2C status prior to read or write data (each call to Status/Set Parameters command adds 2ms delay).

I just try. If it works, proceed with the next chunk. If it fails, check I2C internal status byte in the very same response.

There are two options:

don't you end up sending it again and again until it succeeds?

It will timeout eventually (MCP2221.py#L1212-L1215):

if time.perf_counter() > watchdog:
    self.I2C_cancel()
    raise TimeoutError("Timeout.")
rcvlr commented 1 year ago

When I send packets of 5 chunks, I see the following

Chunk 0 is sent with success (rbuf[RESPONSE_STATUS_BYTE] == RESPONSE_RESULT_OK is True) Chunk 1 fails with error I2C_ST_WRITEDATA_WAITSEND Chunk 1 is re-sent with success Chunk 2 is sent with success Chunk 3 fails with error I2C_ST_WRITEDATA_WAITSEND Chunk 3 is re-sent with success Chunk 4 is sent with success

What is the meaning of the I2C_ST_WRITEDATA_WAITSEND error code? From your description (# buffer sent, waiting for additional Write commands) it's not clear whether the slave received the whole chunk or not.

My concern is that when you retry to send a chunk, the same data is sent twice over the I2C, which is not a problem in my application, but could be in others.

electronicayciencia commented 1 year ago

I am unsure about internal workings, so I wrote a small test to answer you question:

Tracing internal status code during the write operation I get something like:

Writing EEPROM...
Bytes 64/16384.
Status byte: 10
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 40
Bytes 128/16384.
Status byte: 10
Status byte: 42
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 41
Status byte: 40
...

I used a 24LC128 EEPROM and slowest possible rate to force repeated calls to the write command.

As you know, 24LC128 has 64 bit internal page buffer. If you write more than 64 bits in a row, it will wrap over and overwrite the first ones.

After multiple tests, the checksums always matched. So I can be reasonably sure that write command does not send the same bits twice when it fails with I2C_ST_WRITEDATA_WAITSEND (0x41) status.

I am not sure, however, what will happen if some bytes are ack'ed by the slave and some others are not. Probably the whole operation will fail with some NACK status, breaking the loop and won't re-send them either.

electronicayciencia commented 1 year ago

I merged the changes into master and released 1.6. It should work fine now. If you find any other issues, please let me know. Regards.

rcvlr commented 1 year ago

Hi! Thanks for the experiments you did with the EEPROM, I agree with your conclusion. I think that with the help of a logic analyzer, one could better understand the timing between USB and i2c packets. I may try next week and let you know if I find something interesting. I've seen there is a Linux kernel driver for this chip, maybe I could get some insights from there as well.

electronicayciencia commented 1 year ago

Hello!

I've just released some timing diagrams that you may find illustrative: I2C internal details