Attiny13 - interrupt INT0 - problem

[Jordan-Szwejda]

Hi,

Maybe someone may help me but for now it looks like there is a bug in microcore for attiny13. I am trying to handle external interrupt INT0 for detecting if a signal changes from high to low on pin 1. When I attach interrupt then the data loopNo gets corrupted and returns very strange values. Finally it causes restart of cpu. I have a floating wire attached to the interrupt pin what generates interrupts constantly.

The code is very simple.

#include <Arduino.h>

volatile unsigned int ticks = 0;

unsigned long loopNo = 0;
char buf[15];


#define INTERRUPT_PIN 1         //PB1

void interruptProc() 
{
    ++ticks;
}


void setup() {  
  pinMode(INTERRUPT_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(INTERRUPT_PIN), interruptProc, FALLING);
  Serial.write("START");
}

void loop() {
    Serial.write("loop:");
    Serial.write(itoa(++loopNo, buf, 10));
    Serial.write(", ticks:");
    Serial.write(itoa(ticks, buf, 10));
    Serial.write("\n");
   delay(1000);
}

I use attiny13 with following parameters:

[env:attiny13a]
platform = atmelavr
board = attiny13a
framework = arduino
upload_protocol = usbasp
board_build.f_cpu = 1200000L
board_build.mcu = attiny13a

build_flags =
  -D UART_TX_PIN=B,3
  -D UART_RX_PIN=B,4
  -D CUSTOM_BAUD_RATE=9600

Should I add something to interruptProc declaration to have it working? When I comment out 'attachInterrupt' then loopNo is getting incremented correctly.

I would appreciate any help.

Jordan Szwejda

[MCUdude]

I haven't had the time to investigate this, and what itoa does to the whole thing.

I was able to get it working by removing the itoa functions and let the print class handle it instead:

void loop()
{
  Serial.print("loop:");
  Serial.print(++loopNo);
  Serial.print(", ticks:");
  Serial.print(ticks);
  Serial.print("\n");
  delay(1000);
}

[Jordan-Szwejda]

itoa converts a number to string. I do not use Serial.print as it takes additional space in flash memory (which is very limited in attiny13). I will also verify print instead of itoa however when I do not call attachInterrupt then the main loop works fine with itoa operation.

[MCUdude]

Well, print is as efficient as it can get (without breaking general Arduino compatibility). I know this because I wrote larger parts of the Print class myself. It may not be as efficient as itoa, but it's still quite good for what it is.

I'll also recommend you to use the F() macro to store all strings in PROGMEM:

volatile unsigned int ticks = 0;

unsigned long loopNo = 0;


#define INTERRUPT_PIN 1         //PB1

void interruptProc() 
{
    ++ticks;
}


void setup() {  
  pinMode(INTERRUPT_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(INTERRUPT_PIN), interruptProc, FALLING);
  Serial.print(F("START"));
}

void loop()
{
  Serial.print(F("loop:"));
  Serial.print(++loopNo);
  Serial.print(F(", ticks:"));
  Serial.print(ticks);
  Serial.write('\n');
  delay(1000);
}

[Jordan-Szwejda]

Thanks for the hints. I will try F and print in my program. The program I attached is just for reproducing the issue with interrupt. It does not do anything useful, but same problem with this kind of data corruption and restarts I experienced in my another program. To investigate the issue I created just this simple example to be sure it is not something related with other pieces of my code. I spent a few days googling and trying to solve the issue but without success for now.

[MCUdude]

The program I attached is just for reproducing the issue with interrupt. It does not do anything useful, but same problem with this kind of data corruption and restarts I experienced in my another program.

Great! It's much more useful to get an example that doesn't contain all sorts of unrelated stuff.

About your issue with itoa, which I was able to reproduce, maybe @nerdralph knows why itoa returns bogus characters?

[nerdralph]

I haven't looked into why parts of avr-libc suck so much, but I've written much better versions of common things like printing a uint16. I believe my debugSerial library has the smallest possible implementations for printing in decimal, hex, and strings. https://github.com/nerdralph/debugSerial

On Wed, Mar 31, 2021 at 12:05 PM Hans @.***> wrote:

The program I attached is just for reproducing the issue with interrupt. It does not do anything useful, but same problem with this kind of data corruption and restarts I experienced in my another program.

Great! It's much more useful to get an example that doesn't contain all sorts of unrelated stuff.

About your issue with itoa, which I was able to reproduce, maybe @nerdralph https://github.com/nerdralph knows why itoa returns bogus characters?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MCUdude/MicroCore/issues/125#issuecomment-811139525, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNZ6WOXGJG5IOBEF253VLTGM225ANCNFSM42DA4VPQ .

[Jordan-Szwejda]

I have just ran the version with print instead write(itoa()) and it is working stable for now . I will do some more experimenting later. I guess it may be something connected with Serial.write and not exactly itoa. In my real programme I do not have itoa at all but it behaves same way unstable. Instead of this I use:

Serial.write((uint8_t *) &data, sizeof(data));

[Jordan-Szwejda]

It's very interesting. When I add memset to clear buf in the main loop then it works correctly. Theoretically this memset should not make any difference. It looks like there is a problem with compiler or linker itself. When I add such memset only in the setup it does not help.

Here is the version with added memset:

#include <Arduino.h>

volatile unsigned int ticks = 0;

unsigned long loopNo = 0;
char buf[15];


#define INTERRUPT_PIN 1         //PB1

void interruptProc() 
{
    ++ticks;
}


void setup() {  
  pinMode(INTERRUPT_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(INTERRUPT_PIN), interruptProc, FALLING);
  Serial.write("START");
}

void loop() {
    memset(buf, 0, sizeof(buf)); //----- memset added here
    Serial.write("loop:");
    Serial.write(itoa(++loopNo, buf, 10));
    Serial.write(", ticks:");
    Serial.write(itoa(ticks, buf, 10));
    Serial.write("\n");
   delay(1000);
}

[nerdralph]

Use avr-objdump -d to look at the asm. Until then you can't be so sure the compiler has done something wrong.

[Jordan-Szwejda]

It is out of my depth at this moment to analyse asm code. My statement was a pure speculation. I just reported this issue here with hope that I did something incorrect in my code and it would be quick for somebody experienced in this library to point me out my mistake. Unfortunately it looks like I opened a can of worms :( .

[Jordan-Szwejda]

I learned and analysed the ASM code of my program. I got some conclusions and please correct me if I am wrong.

1. My original program caused stack overflow what caused uncontrollable behaviour and restarts. After compiling the code I got following statistics:

Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=======   ]  73.4% (used 47 bytes from 64 bytes)
Flash: [=====     ]  49.0% (used 502 bytes from 1024 bytes)

So that there are only 17 bytes left for the stack which is shared with RAM. Unfortunately interrupt routine causes that a lot of variables is being stored on the stack (please take a look at "__vector_1" section in the attached file:

prg1.txt

There are 15 additional bytes taken from RAM by 'push' commands. So when the interruption occurred it caused stack overflow finally. It was my bug and the programme starts working stable when we save some RAM data by moving for example string constant to program memory (as it was pointed out by MCUdude - using F macro). I am attaching similar fix:

program.txt

2. Unfortunately I experienced also similar but slightly different problem when I moved back to my another program I have been working on. In short, the main idea of my another program is to count impulses on interrupt and sending statistics by using UART.

The main difference was inside the interrupt procedure which looked like:

static void interruptProc() 
{
  unsigned long curTime = millis();
  if((curTime-lastImpulseTime)>100) {
    ++ticks;
    lastImpulseTime = curTime;
  }
}

Here even if I saved a lot of RAM my program work unstable, data were corrupted and restarts occurred. I started analysing ASM code. And I have come to final conclusion that also here was problem with stack overflow. But the reason of this is that millis() implementation has a serious BUG. (and please correct me if I am wrong). Let's look at ASM code of millis():

000001d2 <_millis>:
 1d2:	ef e8       	ldi	r30, 0x8F	; 143
 1d4:	f8 94       	cli
 1d6:	81 91       	ld	r24, Z+
 1d8:	91 91       	ld	r25, Z+
 1da:	a1 91       	ld	r26, Z+
 1dc:	b1 91       	ld	r27, Z+
 1de:	18 95       	reti

There is RETI at the end of method. In my opinion it is not correct and there should be RET instead. Status of I register should be remembered (before CLI) and finally restored. RETI causes that interruptions are enabled again. When you have interruption pin with values which change very quickly then after millis() (which does SEI operation) interruption routine will be called before the current one is finished. As the consequence another 15 bytes of data will be stored on stack, and if such situation happens 2 or 3 times in row than stack overflow will occurr.

To fix the issue I wrote my own millis() operation which ends with RET instead of RETI and everything started working correctly.

So you must be very cautious and do not use current millis() implementation in your interrupt routines unless it is fixed.

Thanks everyone for help and I am waiting for anybody more experience in ASM on this platform to confirm my findings.

[nerdralph]

I wrote the millis code. I didn't expect anyone to be calling millis from within an interrupt since it gets updated from interrupt context. Since then I wrote a better version that doesn't require interrupts to be disabled at all: https://github.com/nerdralph/ArduinoShrink/blob/master/src/millis.S

[MCUdude]

@nerdralph what's great about the current MicroCore implementation you wrote is that it runs on the WDT, which means that it always runs at 128kHz regardless of the main clock, and Timer0 is free to configure as you like. Could the WDT millis code be rewritten to work with @Jordan-Szwejda's example?

[nerdralph]

The mod is very simple. I'm doing some pi stuff right now, so I probably won't have time to test the change today.

   ldi ZL, lo8(wdt_millis_counter)
  _reload:
    ld r24, Z
    ldd r25, Z+1
    ldd r26, Z+2
    ldd r27, Z+3
    ld r0, Z
    cp r0, r24
    brne _reload
    ret

[Jordan-Szwejda]

Looks clever. I will check this with my code.

[Jordan-Szwejda]

I did some testing and seems working fine with my code. Thanks.

[mcuee]

In this case, the issue can be closed.