issue when using i2c and timer interrupt

Hello evryone,

I'm using nano every boards connected as slave via i2c to a Raspberry. Each board is used as PWM driver or analog read and centralised to Pi.
Evrything works good, I've to devices connected, I can send and get data. In another hand, I've programed an isr on TCA overflow, which works fine too.
My problem is that I can't connect device with isr running. When I'm mapping with i2cdetect from Pi, the first nano (not using ISR) is detected but not the second one.

It's not my first issue with i2c and interrupts, and I would like to know if it's common issue

Thank for your help

code (not all part of it, just issue related):

Root file:

void setup() {
  Serial.begin(9600);

  Wire.begin(SLAVE_ADDRESS);
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);

  InitPwm(res,0x2);
  ratio0 = RatioMin;
  ratio1 = 100;
  T = timer_us();

}

///////////////////////////////////////////////////
// Loop
///////////////////////////////////////////////////

void loop() 
{
  SetRatio(ratio0, 0);
  SetRatio(ratio1, 1);
  
  while(1)
  {
    t = timer_us();
    if((t - T)>1000) break;
  }
  
  T= timer_us();
  ratio0++;
  
  if(ratio0>RatioMax)
  {
    ratio0 = RatioMin;
  }
}

Timer handling

// TCA configuration

void InitPwm(unsigned int res, byte divider)
{
  
  pinMode(5, OUTPUT); //PWM compare output 2
  pinMode(9, OUTPUT); //PWM compare output 0
  pinMode(10, OUTPUT); //PWM compare output 1

  // Used to be able to count time, because when changing TCA config, micros() and millis() didn't works properly
  
  clk_per_100ps = pow(2,divider)*1e4/freq_MHz;
  ovf_per_ns = (long)clk_per_100ps*(long)resolution/10;
  minute_cnt = 60e10/(clk_per_100ps*resolution);
  resolution = res;

  //////////////////////////////////////////////////////////////////////////////
  
  TCA0.SINGLE.PER = resolution; //Set TOP (determine frequency)
  TCA0.SINGLE.CTRLA = (TCA0.SINGLE.CTRLA & 0b11110001) | divider<<1; // CLK Divider 1
  
  TCA0.SINGLE.CTRLA = (TCA0.SINGLE.CTRLA & 0b11111110) | 0b1; // enable TCA0
 
  TCA0.SINGLE.CTRLB = (TCA0.SINGLE.CTRLB & 0b11111000) | 0x3; // Single slope PWM mode
  PORTMUX.TCAROUTEA=(PORTMUX.TCAROUTEA & 0b11111000) | 0x01; //TCA compare output on port B (32->WO.0, 33->WO.1, 28->WO.2)
  
  TCA0.SINGLE.CTRLB = (TCA0.SINGLE.CTRLB & 0b10001111) | 0b01110000; // Compare Output 0-2 enable 

  TCA0.SINGLE.INTCTRL = (TCA0.SINGLE.INTCTRL & 0b11111110) | 1; // Activate overflow interrupt
}

// Interrupt handling

ISR(TCA0_OVF_vect)
{
  ovf_cnt++;
}

I2c handling

void receiveData(int byteCount)
{
  cli();
  lenReceived = Wire.available();

  while(Wire.available())
  {
    dataReceived[rcvcnt++] = Wire.read();
  }
  sei();
}

void sendData()
{ 
  cli();
  
  if(dataReceived[0]<=0xF) Wire.write((const uint8_t*)&I2cSend, 2);
  sei();
}

Every problem with I2C is a common thing, because there are about 20 to 40 common problems. The only a unique problems are sensors with very rare demands.

Does the Raspberry Pi support clock pulse stretching ? That is needed when an Arduino board is the Slave.
The ISR is very short, that should be no problem, unless there are other timing specific problems. Is the ISR running very often ? for example 50 kHz or more ?

Please show the full sketch.
There is a special website for that: https://snippets-r-us.com/.
You snippets give very little information. I don't know if there are other libraries in use that disable interrupts. I don't know where 'ovf_cnt' is used and if it is volatile. I don't know ... there are so many things that I don't know.

Nano Every: Arduino Nano Every — Arduino Official Store
That is a 5V board from the megaavr branch.

That means there is a voltage mismatch on the I2C bus. The Raspberry Pi use 3.3V signals for SDA and SCL and the Nano Every uses 5V signals. Since the pins of the Raspberry Pi are not 5V tolerant, you should not connect them together. You have to fix that before going on.
The Nano Every does not have onboard pullup resistors, so it is only the internal pullup resistors to 5V that could cause a problem. Since they are weak pullups, the Raspberry Pi is probably not damaged.

You don't have to turn off the interrupts in the I2C handler functions. I don't know if that causes problems, but the 'onReceive' and 'onRequest' handlers are called from an interrupt routine in the Wire library.

The 'avr' branch is the base that Arduino was built on and many bugs have been fixed. Just a few weeks ago the Wire library got timeouts for the I2C bus. The 'megaavr' branch is newer and there could be still a few bugs in the libraries. Keep that in mind when you combine things that no one else has done before with the Nano Every.

Could TCA0 interfere with millis() ?

Do you want to move this thread to this: Nano Every - Arduino Forum ? You can ask a moderator for that.

Thanks for this reply,

I've share full sketch, it's in zip file because there 7 files. Not all code is used, some is comming from other projects and I keep it to be used. Tell me if it's better to share a "clean" code?

Regarding Pi compatibility, I don't think it can be a problem. I'm using it from a long time with arduino.

The interrupt is repeated at 62 kHz, because it's used to generate PWM. As I was saying in my first post, TCA makes trouble with millis and micros. I was expecting it's the timer used by these two functions, and I have change TOP.

I've also used scope to monitor bus during detection process and it seems to have an unexpected beaviour. I've attached sreenshot.

I'm ok to move this topic to nano every. Can you explain me how to do this?

PWM.zip (5.34 KB)

I'm sorry but everywhere I look I see serious problems. I will try to write down the things that catch my eye, but perhaps you should take a step back and do something that is easier. Adafruit has nice tutorials with led strips 8)

Adafruit has a very nice PWM module. Could you use something like that ? I don't know if there are Raspberry Pi drivers for it.

  1. O no, 62 kHz on a 20 MHz microcontroller :o That is a major problem. I don't see how you can have a working I2C Slave with that high rate of interrupts.
    I have done stress tests with a Arduino Uno, and it is not hard to make it stop from working as I2C Slave. Everything should run smooth and nice and then the I2C Slave part will also work reliable.
    Could you try to lower the I2C bus speed in the Raspberry Pi to 50kHz or 10kHz ? There might be a very little chance that you can squeeze the I2C between the 62 kHz interrupts.
    This means you have to rethink how to implement your project.

  2. Does the Raspberry Pi support clock pulse stretching ? I don't know that, and that it worked before is not enough for me ::slight_smile:

  3. You should not connect a 3.3V I2C bus to a 5V I2C bus.

  4. I don't know which timer the millis() uses at this moment for the megaavr branch. You should not mess with the millis() timer, because many libraries rely on it.

AD.ino

  1. This can be improved:
unsigned int T;
T = micros() & 0xFFFF;

When you convert the 'unsigned long' from the micros() to a 'unsigned int', then you can use a cast to tell the compiler what it should do. When you want only the lower 16 bits, then you can use a cast as well. That means you only need the cast:

unsigned int T = (unsigned int) micros();
  1. This should be avoided:
if(Prev_T>T)
  TimeStep = (T+0xFFFF)-Prev_T;
else
  TimeStep = T - Prev_T;
Prev_T = T;

The rule of thumb is not to try to handle a rollover of millis() and micros(). Use the register math of the microcontroller.
The Blink Without Delay will work perfect, even during a rollover: https://www.arduino.cc/en/tutorial/BlinkWithoutDelay.

I2c.h

  1. Never declare variables in a *.h file. That is not how to use the 'c' or 'c++' language.

  2. You probably don't need all the function prototyping. The Arduino preprocessor can take care of that in most cases. It is not a bad thing, so keep the prototyping if you like that.

I2c.ino

  1. Please don't use cli() or sei() in the 'onReceive' or 'onRequest' handlers.

  2. Don't use Serial functions in the 'onReceive' or 'onRequest' handlers. That is used in examples, but those are bad examples :frowning: You use Serial.println() in the SetParameters().

PWM.ino

  1. Do not use cli() and sei() when using Serial functions. I can not think of any reason why that would be needed.

TCA.ino

  1. I don't understand the function timer_us() and what it is doing with 'ovf_cnt'.
    Since you use it only in the Arduino loop(), can you use micros() ?
    In the function timer_us(), the interrupts are turned off. You should limit that to the very minimum and only if it is really needed.

Suppose a 'volatile unsigned long' is used in a interrupt on a 8-bit microcontroller. Then I like to make a copy in the loop().

void loop()
{
  noInterrupts();
  unsigned long ovf_cnt_copy = ovf_cnt;
  interrupts();

  // do the calculations with ovf_cnt_copy
}
  1. Do not use cli() and sei() in the ISR(TCA0_OVF_vect).

TCB.ino

  1. When the compiler sees this
minute_cnt = 60e10/(clk_per_100ps*resolution);

then it sees:

'unsigned long' = 'float' / ( 'unsigned int' * 'unsigned int');

That is a lot of conversions that the compiler has to do. You can add a few casts to make clear what is going on.

  1. Function timer_usB(). Same as number 12.

timers.h

  1. As number 7. Never declare variables in a *.h file.
    When a variable is a fixed number, you can add the 'const' keyword in front of it as you already did with 'freq_MHz'. Then the compiler can do other optimizations. A 'const' variable should still not be declared in a *.h file.

Many thanks for this long answer,

And sorry for the way I'm writing code, but it's not easy to use differents langages (Python, PHP, C) and to switch good writting rules ... But I will take your comments into account and pay attention for next time

Just one question about this, why not using .h files? for me it's easier to see what I'm doing when I don't have too much tings in one page, and to use header at the beginnig for all globals of differents parts of program helps on this.

I can understand that I can do something like my program do with some ready to use modules, but it's not final goal. This is just first tests to control the chip, finality is more to use it in power electrics device, controled and monitored by Rpi.

I've attached a claen code with just timer part and want you to test three steps:

  • First run program and look serial output: T, which should be microsecond count, is not really precise (count 8 seconds instead of 2)

  • Second, change global varible 'use_micros' to true (at the beginning of the main sketch)

#define SLAVE_ADDRESS 0x14

bool use_micros = true;

And look at the serial output. After 20 seconds, count is more than 1000 seconds.
It's the reason I'm using my own timer function

  • Last thing: comment the timer init function in init():
void setup() {
  Serial.begin(9600);

  Wire.begin(SLAVE_ADDRESS);
  Wire.onReceive(receiveData);
  Wire.onRequest(sendData);

  //InitPwm(res,0x1);
  //InitPwmB0(0xFF,0);
  
  ratio0 = RatioMin;
  ratio1 = 100;

  if(use_micros)  T = micros();
  else            T = timer_us();
}

And look at serial output: seconds count is normal

I've use cli()-sei() with serial print, because if not, program crash after 10 seconds (when using timer isr)

Last thing, I've tried to increase interrupt time, by increasing divider and TOP, but as soon as the ISR bit is set, I2C stop working.
Same thing if using TCB0 in 8b PWM mode with interrupt

PWM_clean.zip (2.95 KB)

A #include inserts that file at text level, before the compiler starts compiling. In theory, you can put anything in it.
However for a project, the *.h is the glue between the *.cpp or *.ino files and it can be included into more than one *.cpp or *.ino file. Global variables should be in a *.cpp or *.ino file.

The Arduino preprocesser makes everything slightly different. All *.ino files are combined. That means you can use a global variable in every *.ino file without a 'extern' reference in the *.h file.

A *.h file is sometimes used for data :wink: for example for large amounts of graphic data for a display.

Disabling interrupts for Serial.println() can't be right. It seems as if you a fixing a problem with another problem.
I think you should remove the cli() and sei() can find what causes the crash.
A crash can be for example a buffer overflow (writing to memory that is no part of the variable).

Your 9600 baudrate is very low. Can you make it 115200 ?
Do you send a lot of text ? When too much text is transmitted, then the software TX buffer might get full, and that will almost halt the sketch. When the software TX buffer is full, the Serial.println() waits for an empty spot in the buffer. That empty spot is created when an interrupt gets a byte from the buffer and sends it to the serial port.
How often is 'T' printed ? Could there be a burst where 'T' is printed every millisecond ? Let's say 10 bytes per millisecond ? That is 10k bytes per second. That is too much, even for 115200 baud.

Can you use the BlinkWithoutDelay to make a 1 millisecond timer with micros() ?
Then you can also use the BlinkWithoutDelay with millis() to show data a few times per second on the serial monitor.

I don't know the Nano Every, so I don't know what the result of the timers is for the other code :frowning:
Can you connect the Nano Every with a serial port to the Raspberry Pi ?

One more time, thanks for this help. Your advises help me on writting less dirty code ... Thanks to this, I've been able to solve my problems.

I will share what I've learned, and hope this can help somebody else :

  • micros() and millis() are syncronised with TCA clock. So changing TCA clock divider change behavior of these two functions. So, to use it, you must shift result regarding clock divider you use :
TCA0.SINGLE.CTRLA = (TCA0.SINGLE.CTRLA & 0b11110001) | divider<<1; // CLK Divider
 scale_micros = 6 - divider;

t = micros()>>scale_micros;
  • When using ISR, flag must be clear, else interrupt function is repeated infinitely. I've check it by changing pin state during interrupt it's repeated all 5 µs. It slows execution and didn't allows other functions, like i2c for exemple.
    So' regarding datasheet, bit is cleared by writing '1'
ISR(TCB1_INT_vect)
{
  ovf_cnt++;
  TCB1.INTFLAGS = (TCB1.INTFLAGS & 0b11111110) | 1;  //extremely important
}

Now everythink works fine, I've I2c com, and I can have timestamp, even if overflow of micros() appends 64 times often.

Please appologies for my english, and I think this topic can be closed