Skip to content

Wire do not work in a multimaster i2c bus #5645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pat1 opened this issue Jan 20, 2019 · 19 comments
Open

Wire do not work in a multimaster i2c bus #5645

pat1 opened this issue Jan 20, 2019 · 19 comments

Comments

@pat1
Copy link

pat1 commented Jan 20, 2019

I am using Wemos D1 mini with arduino uno (avr 328P) configured as multimaster i2c.
While two avr mcu work as expected, with esp8266 when esp8266 start communication as master seems that it starts regardless of the i2c status or arbitration crashing communication.

I use the last git version with arduino 1.8.8 in a Linux Fedora workstation.
I can check communication with a logic analyzer in a specific situation if required.

@Jason2866
Copy link
Contributor

Please fill out issue template. Needed infos are missing.

@andrethomas
Copy link

@pat1 I assume from Issue #5644 that you are using Core 2.4.0 - In which case the answer is the same as provided for Issue #5644

@pat1
Copy link
Author

pat1 commented Jan 21, 2019

As written before I use the last git version branch master with arduino 1.8.8 in a Linux Fedora workstation.

Platform

  • Hardware: ESP-8266EX
  • Core Version: 2019-01-21
  • Development Env: Arduino IDE 1.8.8
  • Operating System: Linux Fedora

Settings in IDE

  • Module: (lolin)Wemos D1 r1 & mini
  • Flash Mode: I do nor know
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory
  • Reset Method: I do not know
  • Flash Frequency: I do not know
  • CPU Frequency: 80MHz/160MHz (https://wiki.wemos.cc/products:d1:d1_mini)
  • Upload Using: SERIAL
  • Upload Speed: 921600

@andrethomas
Copy link

Still incomplete... nobody will be able to reproduce your problem if you do not provide all the information requested by the template.

More specifically I would be interested to see the sketch you are using to reproduce this problem because I am not having any problems with I2C communication with a number of slave devices tested using the beta2 release (0fd86a0)

@pat1
Copy link
Author

pat1 commented Jan 21, 2019

test_i2c_multimaster.zip

The problem is not with multislave with esp as master; is not a problem with esp as slave (implemented by version 2.5).
Is a problem using esp as master in a multimaster i2c bus.
I attach a sample test program that I use to check communication putting some board communicating on a bus two by two.

@rp3tya
Copy link

rp3tya commented Jun 13, 2019

Confirming this problem.
I have multiple Arduinos (Nano) which can address each other over i2c. Using the same sketch, I can not add ESPs to the pool.
ESP master and Arduino slaves work as expected.

@Tech-TX
Copy link
Contributor

Tech-TX commented Oct 27, 2019

Currently the library is only checking for collision / arbitration in the START condition, so it will fail in a multi-master bus. Once it's past the START, it doesn't check to see if the bit it wrote to SDA was successful.

bool Twi::write_start(void)
{
    SCL_HIGH();
    SDA_HIGH();
    if (!SDA_READ()) // should probably be (!SDA_READ() || !SCL_READ() after a brief risetime pause)
    {
        return false;
    }

To meet the spec on arbitration, collision testing must be performed with every (high) bit written from the master, something like this:

bool Twi::write_bit(bool bit)
{
    SCL_LOW();
    if (bit)
    {
        SDA_HIGH();
    }
    else
    {
        SDA_LOW();
    }
    busywait(twi_dcount + 1);
	if (bit && !SDA_READ())
       {
	   SDA_HIGH();
	   SCL_HIGH();
	   return false; // arbitration
	}
    SCL_HIGH();
    WAIT_CLOCK_STRETCH();
    busywait(twi_dcount);
	if (bit && !SDA_READ())  // should probably check SCL, too
       {
	   SDA_HIGH();
	   SCL_HIGH();
	   return false; // arbitration
	}
    return true;
}

The function that called it has to check, as well:

bool Twi::write_byte(unsigned char byte)
{
    unsigned char bit;
    for (bit = 0; bit < 8; bit++)
    {
        if (!write_bit(byte & 0x80) return false; // arbitration or NACK from this function
        byte <<= 1;
    }
    return !read_bit();//NACK/ACK
}

If it fails all the way back to the user's program without caching the request and trying later, then it would be up to the user to check the status of every Wire request and resend if needed.

The same arbitration / collision test is also required for the READ bit, the REPEATED START, and the STOP condition. I don't have anything but ESPs here, so I can't really test your environment.

I never did understand the concept of I2C clock synchronization. If one master is talking, the second master has to back off until the first master has released the bus. The clocks will never stay in sync if one or the other does a REPEATED START due to the minimum timing requirement for it; it will always be at least 1 1/2 clock wide minimum (going from memory).

@devyte
Copy link
Collaborator

devyte commented Oct 27, 2019

@Tech-TX do you intend to pursue implementing that?

@Tech-TX
Copy link
Contributor

Tech-TX commented Oct 28, 2019

I'm not sure if it's possible or reasonable. The spec says you must have clock synchronization in a multi-master bus as well as arbitration, and I don't see a way to do synchronization with this library. I could implement the arbitration parts and that should work for 95% of the collisions, but I'm guessing. I'd have to buy a couple of Arduinos as test masters since the comments I've seen around the 'net revolve around them being able to do multi-master successfully. I don't have any experience with multi-master: the boards at my last company only had single-master designs.

I'll look at the Arduino Wire library to see if I can implement an equivalent function. I only have time to play on the weekends, so I'll have limited progress until then. I'll look at the Arduino code before I make any promises.

One issue: I've been good at various microcontroller assembly languages, but I'm still learning C. We had a few programmers to handle the code, and I handled the hardware. Right from the beginning, I don't know how to cache the Wire request the user has sent so I can wait and re-send when the bus is clear.

@devyte
Copy link
Collaborator

devyte commented Oct 28, 2019

95% > 0% i. e. partial support with limitations is better than no support.
About synchronization, if it's not possible/reasonable to do, we can document the limitation.
About C/C++, I don't know the specifics of I2C, but I do know the language and practices, so I can help you there.
About looking at other Arduino libs, please be aware that some are hardware based and some are software based. We only have sw I2C.
About your availability, there is no hurry or deadline or anything, so it's only a matter of whether you're willing to pursue this. In other words, it's ok to go slowly as your time allows. I ask, because you seem to understand the protocol and can analyze problems and find solutions for it, and we need help with I2C (among other things).
To better coordinate, please @ me in our gitter channel .

@Tech-TX
Copy link
Contributor

Tech-TX commented Oct 28, 2019

I'm game. By checking both the SCL and SDA during a transmission we can detect a collision two different ways and then abort. Typical I2C specification just checks SDA and does that funny clock sync. I just looked at the ATmega328 datasheet and saw the hardware I2C core. The way it must be implemented on Arduino is the user gets a failed command and has to re-send themselves; it doesn't look like the I2C core handles that, although the library may handle lost arbitration. I looked at the example code that pat1 posted above and it's a mess. I'll see if I can find a clean example to look at.

'Core' will get confusing; it's used in FPGAs and chip design to mean a drop-in piece of hardware / logic that you can re-use, like a C or asm library. Here, it means the central CPU part of the code.

Your link for the gitter channel is a 404... Maybe I'm not worthy. ;-)

@devyte
Copy link
Collaborator

devyte commented Oct 28, 2019

link fixed

@Tech-TX
Copy link
Contributor

Tech-TX commented Oct 29, 2019

Got it. I'd recommend deleting that link to keep random surfers out. Merely a suggestion. ;-)

Wow... 800+ people in that one room, and no obvious way to un-thread the messages. How do you folks keep track of what's going on in there?

I'm looking at other people's implementation of multi-master on the Arduino, and I don't see anyone checking the return status of their I2C calls, which suggests the library handles arbitration fail re-send of packets. I see at least one Arduino open issue of multi-masters not playing well together and trashing the bus, so their arbitration isn't bullet-proof. Arduino previously didn't support Repeated Start, haven't found out yet whether they fixed that. I also see people (within the last year or two) getting junk data from I2C master reads or slave reads and attempting to 'filter' it by throwing out bad packets, so it looks like Arduino isn't a happy multi-master land yet. Arduino has buggy multi-master support.

In addition to adding the collision tests on high bits (the easy part) just before the rising SCL, I had a thought that should work. Slightly modify the Twi::onSdaChange ISR that looks at SDA and SCL, and store the current (microseconds) if the flag is STOP, then 2 or 3 clock times later if we're still clear we can start transmission. Might need the Twi::onSclChange ISR as well, dunno for sure yet. I'd have to weave a couple of (IF MASTER.mode) checks in there so I could use those slave functions. I haven't looked at the slave code at all yet, so I'll spend some time with it first so I know what's already available that I can use.

@Tech-TX
Copy link
Contributor

Tech-TX commented Nov 3, 2019

Collision detection and arbitration will add processing overhead, shifting the timing I'd taken for the current SCL speeds (just above). SCL is already a little slower than it used to be due to the added clock stretch detection, and I'm going to slow it down a little more.

Robust arbitration would check that neither SDA nor SCL have been driven low by another master. I don't think we can afford to check SCL due to the added timing overhead, and it only catches half of the arbitration situations anyways [other master SCL drops either before ours or after ours]. Collision detection on SDA is always valid so that's all I'm planning to do, which minimizes added overhead.

The other bug I noticed (not sending STOP after a byte NACK error) causes a different issue with multi-master: the return from Twi::write_byte is currently a bool, and needs to be an unsigned char with 0 = NACK (always send a STOP), 1 = OK, 2 = lost arbitration (exit without STOP). I'm working through the code right now to see what else needs to be added for arbitration.

I've looked at the AVR wire library and can't see it attempting a re-send, so I won't worry about it with ours. Theirs only returns an error to the calling program from what I can see.

The AVR library doesn't appear to handle arbitration correctly with Repeated Start, but I'll set ours up to deal with it properly. Multiple AVR masters may not play well together with Repeated Start, but ours won't step on their master after the changes I'm doing.

Currently I'd recommend 1.5 to 2 full clock cycles of [look at SCL/SDA and abort if they go low] before we send a START, so that we gracefully deal with the AVRs above in Repeated Start. I'll fire up one of the Pro Mini boards I have to verify what it does in both normal and Repeated Start cases. I haven't looked at the AVR yet beyond insuring that I can write to them. I suspect Atmel used the same I2C hardware core in their chips, so it should work the same for all AVRs.

I need to take a one week break to bone up on the classes I have to teach next week, so I won't be able to get back to this until Nov. 8.

@johnjoe57
Copy link

Hi, Surely its better to always check SCL for a busy line as it always goes low whereas SDA need not go low at all (ie when sending 0xFF) ? In fact if SCL is checked I wouldn't bother checking SDA at all.

@johnjoe57
Copy link

Actually yes SDA does go low but not nearly as often as SCL.

@iot-crazy
Copy link

I'm new to I2C and quite new to C also, so please excuse if I've got this wrong. I think I'm having a similar problem with a multi master setup when 2 of the masters are also used as slaves. The third master is only ever used as a master and that never has a problem. So I have 3 devices on the bus.
Specifically, the 2 that are used as both master and slave send data to each other, the third only sends to one of the others.

A - Receives from B and C, sends to C
B - Sends to A only
C - Receives from A, Sends to A

At first, these all work fine, but, after a few exchanges between A and C, they stop receiving messages from each other. But, I debug the lines with an oscilloscope and I see the data being accepted and the full data being transferred, but the onReceive() callback doesn't get triggered. If I persist, then from time to time, it will fire the onReceive, but rarely.
Resetting A and trying again then C will receive correctly, but, A does not receive from C unless I reset C. So, it appears that resetting the sender fixes things, at least for a few transmissions.

Happy to paste my code if it helps (but be warned, it's rough and dirty)

@devyte
Copy link
Collaborator

devyte commented Jun 21, 2020

I2C is pending some rework by @Tech-TX, but we all got side tracked with pwm. Fixes will be made eventually, but they certainly won't make it into 2.7.x, which means probably 3.0.0, which in turn means it will be a while.
In the meantime, feel free to investigate, you seem to already have a start on that. If you intend to do so, please look me up in our gitter channel https://gitter.im/esp8266/Arduino to coordinate and for guidance.

@iot-crazy
Copy link

I'm still learning C / C++ so not sure how much use I'd be and I'm also quite new to I2C so would need a lot of time to get through the protocol, all in addition to trying to reinvent our business after the recent events pretty much killed it. The irony being that our new venture is going to rely on this library :). I'll have a look if I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants