Skip to content

SERCOM::startTransmissionWIRE() freezes on hardware issue. #476

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
Jolbas opened this issue Dec 9, 2019 · 4 comments
Open

SERCOM::startTransmissionWIRE() freezes on hardware issue. #476

Jolbas opened this issue Dec 9, 2019 · 4 comments

Comments

@Jolbas
Copy link

Jolbas commented Dec 9, 2019

I have a problem when using I2c with Wire library. When the SDA signal is disturbed by something then sometimes when the process reaches line 530 in SERCOM.cpp it gets stuck in an infinite loop running SERCOM::startTransmissionWIRE() over and over again. If I add a 1500 µS delay before line 530 it seems to get out of the loop. Its like the hardware do not get time to recover without the delay. But a delay would maybe be bad for performance if it's common for this code to be run.
I have this problem on two MKR GSM 1400. Maybe someone can confirm this on different hardware. If I connect SDA to 3V3 I run into this problem.

Here is the lines around SERCOM.cpp:530 with my fix.

if(!isBusOwnerWIRE())
{
  // Restart communication after small delay
  delayMicroseconds(1500);
  startTransmissionWIRE(address >> 1, flag);
}
@ghost
Copy link

ghost commented Jun 9, 2020

I've struck the same issue. The recursion blows the stack when a major hardware issue occurs. As you mentioned your fix just delays the inevitable!

I've come up with something that works for me. I've started with a clone from issue #439 and added a time out (of sorts) to limit the recursion within SERCOM::startTransmissionWIRE() when a hardware issue occurs.

Try replacing your code with the following,
` if(!isBusOwnerWIRE()) {

    if (!timeoutInterval) return false;
    
    if (restartTX_cnt > timeoutInterval) {
        // only allow limited restarts
        restartTX_cnt = 0;
        return false;
    } else {
        // Restart communication after small delay
        delayMicroseconds(1000);
        restartTX_cnt++; // increment restart counter
        // Restart communication
        startTransmissionWIRE(address >> 1, flag);
    }
} else restartTX_cnt = 0;`

and within SERCOM.h change line 231 to,

uint32_t timeoutRef, restartTX_cnt;

@ghost
Copy link

ghost commented Sep 4, 2020

PR submitted, but you can checkout branch wire_timeout if you want to test my mods directly.

As an update, from my field testing, I've been running around 5x units continuously for the past couple of months now. All seem to be able to handle and recover from i2c glitches without complete system lockups, of which I was experiencing at least once per week per device previously.

My thanks go to @jrowberg and others for the work that went into the initial time out implementation.

@nmaas87
Copy link
Contributor

nmaas87 commented Sep 4, 2020

I have no time to test or qualify your PR at the moment, but I just wanted to thank you, as I wanted to start using I2C on AT-SAM-D and this will probably save me in the future, so - thanks a lot for your time and effort :)!

@ghost
Copy link

ghost commented Sep 6, 2020 via email

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

2 participants