Mfcttrf

I have an atmega328p that checks if a button was pressed via pin change interrupts. Now, I want to turn on an led for 200ms.
Can i just turn the led on, wait 200ms and turn it back off in the ISR like so?

ISR(PCINT1_vect)

 if(PINB & 0b1)
 
 PORT = 0b10;
 _delay_ms(200);
 PORT = 0;
 

In a few forum posts on AVRfreaks, I've read that you shouldn't spend much time in an ISR, but I've never seen any exact numbers. I sadly can't find those posts anymore, so I cant link them. As far as I can remember, they all said, that if you spent to much time in the ISR, the µC might crash.
Is that true? And if so, is there an exact time limit after that this might happen?

If nothing else is running in the MCU, then you are free to take as long as you like in the ISR. BUT, this is a bad habit to get into, and it means that if you want to do anything else, you'll likely have to rework the code.

A particular case is if the MCU is using a serial library, that expects interrupts to be working often enough to service individual characters received. At 115200 baud (a high serial speed often used to minimise download time), there is less than 100uS between characters. If you block the interrupts for longer than that, you risk losing input characters.

As a general rule, do the absolute minimum in an ISR. In your application, a reasonable design would be to have an interrupt every mS, which increments and checks a counter value. I'm sure you can work out some suitable logic to set and test the counter to get 200mS between turn on and turn off events.

While the practice is to allocate the minimum possible execution cycles inside an interruption, and beside other general hardware specifications, there are not technical limitations for increasing them, if there are not any other interruption to be executed.

At attachInterrupt() Arduino Reference:

Generally, an ISR should be as short and fast as possible. If your
sketch uses multiple ISRs, only one can run at a time, other
interrupts will be executed after the current one finishes in an order
that depends on the priority they have. millis() relies on interrupts
to count, so it will never increment inside an ISR. Since delay()
requires interrupts to work, it will not work if called inside an ISR.
micros() works initially but will start behaving erratically after 1-2
ms. delayMicroseconds() does not use any counter, so it will work as
normal.

Having 25 possible interruptions in this processor family, it is encouraged to deal with them like punctual events, for allowing other interruptions to happen.

In the worst case, an ISR can run until the next interrupt of the same type occurs again.

But in general, it's poor design practice to spend more time in an ISR than absolutely necessary, because it prevents all other code from running at all. This is a big issue for anything other than trivial programs.

You have a single-core, single-threaded CPU. This means that at any given time, it's doing exactly one thing. If your application requires it to do several things, then the code has to be designed to switch between all of them. This can be as trivial or as complex as you like.

Normally, the CPU putters around the main loop, doing whatever is in there, and that's all well and good until an interrupt occurs. The interrupt hardware basically forces a function call to the appropriate ISR, even though there's no call instruction for it in the main loop. This unpredictability is where most of the rules come from for writing ISR's.

Whatever time you spend in an ISR is time that the main loop is paused, waiting for the ISR to return. If the main loop is the only one to reset an active watchdog timer (very good practice), then the watchdog will not be reset during that time. If the watchdog times out, it gives you a hard reset. Just like the external reset, but with different flags that you can check on startup. This is probably the "crash" that you heard of.

It's very good practice to use the watchdog, and to only reset it once each trip around the main loop. This forces you to write code that stays responsive. If you need to wait for something, you can set up an event (timer finished, next character received, etc.), and move on. Check periodically for that event or set another interrupt for its completion, and get back to it then. Meanwhile, you continue with whatever else you were doing.

My main structure is typically something like this:

#include "module1.h"
#include "module2.h"

void main(void)

 //overall
 //chip
 //setup

 mod1_init();
 mod2_init();

 //clear interrupt flags
 //global interrupt enable

 while(1)
 
 //clear watchdog

 mod1_run();
 mod2_run();
 

And my modules are like this:

void modX_init(void)

 //hardware and variable init for this module only
 //don't use interrupts if polling is good enough


void modX_run(void)

 if (POLLED_INTERRUPT_FLAG)
 
 POLLED_INTERRUPT_FLAG = 0;

 //non-blocking "ISR" code
 


void ISR modX_ISR(void)

 //okay, this does require an *immediate* response
 //spend the absolute minimum time here and get out

The function signatures don't have to be void, but most of them are. Sometimes I'll have some broad timing in one module that is also used by another, and it's handy to use the return value of one modX_run() and the arguments of another (or some basic logic) to make that connection. For example:

if (DMX_run()) //includes its own timing, and returns true at the start of each 30Hz interval, otherwise false

 I2C_start(); //I2C frames are sync'ed to DMX

I2C_run(); //once started, an I2C frame runs freely until finished

If you study the datasheet, you may also find that the hardware peripherals can be massaged to do what you want without any CPU intervention at all.

Output pulse generation, for example, is a common one. Turn it on, set the peripheral to turn it off some time later, and forget about it. It's typically in the same general area as PWM.