Timer mystery

[PaulStoffregen]

Ah, ok then, problem solved.

I guess I misunderstood the code's purpose with msg #9 "The spacing between the two leading edges in the lower trace, or the duration of the pulse upper trace, should be roughly equal to the value of test_u2 in usecs, in all cases." Maybe that sentence was meant to say test_u1 rather than test_u2?

Anyway, the issue has now been fixed, at least on github. It will be in 1.58. You can put the line into your copy if you like. But you can easily avoid this issue with 1.57 or earlier by calling end() at the beginning of your interrupt function, as Defraster mentioned in msg #3.

[luni]

Impressively quick fix :-)

@DrM: BTW you don't need

#include <digitalWriteFast.h>

Teensies support digitalWriteFast out of the box

[DrM]

@defragster read my edit please, and read this carefully too.

Not clearing a pending interrupt before you re-enable interrupts is a bug and there is no robust solution other than masking the pending interrupt before you re-enable interrupts.

You can move end() as early as you like, as long as you can set the timer to give you another interrupt before that, you have the bug.



And it is not true that interrupt code can always be broken. Well written interrupt code does not get broken, ever.

[DrM]

@PaulStoffregen "Maybe that sentence was meant to say test_u1 rather than test_u2" - Yes, sorry for that.

[DrM]

@PaulStoffregen - apologies, I want to make this a separate post. I think it is very important.


Is there a similar issue in the interrupt attach/detach code?


All of the interrupt code for detach or end or whatever, should be masking the enable bit for any pending interrupts before re-enabling interrupts.


I usually leave the status bit so that I can check it, but clear the corresponding bit in the enable mask so that it does not generate the interrupt.

[PaulStoffregen]

Is there a similar issue in the interrupt attach/detach code?

Seems unlikely, since attachInterrupt() clears prior pending.

https://github.com/PaulStoffregen/co...terrupt.c#L120

I want to make this a separate post. I think it is very important.

Please start it with a test case I and anyone else can copy into Arduino and upload to a board to reproduce the problem. Code fragments aren't enough. It really needs to be a complete program.

[DrM]

@PaulStoffregen

But what does detach do if an interrupt arrived before or while it was working?

Does this do it ? gpio[IMR_INDEX] &= ~mask

(line 129, https://github.com/PaulStoffregen/co...terrupt.c#L129 )

[DrM]

@PaulStoffregen Never mind, dumb question. I see that disables the interrupt for the pin. Perfect.

[DrM]

Correction, disables the interrupt for the pin. That is what I was looking for. Perfect. Thank you

[DrM]

Okay, that was fun. Thank you for the fix.

[DrM]

@PaulStoffregen Oops, maybe. Is this atomic? If not, you have a re-entrancy bug. I don't really have time to construct a demonstration today, but I think it is pretty clear that it is so.

gpio[IMR_INDEX] &= ~mask;


From here, https://github.com/PaulStoffregen/co...terrupt.c#L129

[DrM]

Okay, I think my work around for the re-entrancy problem in detach, is to bracket detach in the main loop with noInterrupts() ... interrupts() and rely on interrupts being disabled on entry to the ISR.

But, I am little concerned about one thing, does calling delayMIcroseconds() in an ISR re-enable interrupts?

[joepasquariello]

Not having the .end() before the next interrupt was queued was the problem.

I'm not clear on the meaning of "interrupt PENDING". IntervalTimer.begin() sets the interrupt enable flag (TIE) and clears the interrupt flag (TFLG). When a timer expires, TFLG is set, and if TIE is also set, an interrupt will occur. If this happens while interrupts are disabled, the interrupt can be avoided by clearing TFLG, but not simply by clearing TIE? Is this correct? Does that mean that interrupt PENDING simply means (in this case) TIE=1 and TFLG=1, as opposed to the interrupt actually be "queued" somewhere so that it cannot be avoided even by clearing TFLG?

[DrM]

I think interrupt pending in this discussion means the status bit is set for the source but the enable mask is set to disable or global interrupts are disabled.

In other words, the condition is present but the "path" to cause the jump to the isr is not enabled at one or more steps along the way. It is not really a "path" since it is a set of enables in registers, like logic gates. I use the term path as a conceptual analogy.

In such a state, enabling the bit in the mask and enabling the global interrupt would result in an interrupt, provided it is level triggered and not edge triggered. Some controllers are edge triggered, some are level triggered, and some are selectable.

[DrM]

Re re-entrance in detach(),

I think it can be left as is, and the documentation can say it is not re-entrant. That is easier and more readily compatible with calling it from an isr (a very common use case). We the users can call noInterrupts() where needed.

The alternative to protect it internally, would mean saving and restoring the interrupt enable state, too fancy and unnecessary.

[PaulStoffregen]

I'm not clear on the meaning of "interrupt PENDING".

Sadly, NXP's documentation leaves quite a lot to be desired on this matter. So many finer details aren't clear (at least to me) and need to be investigated by experimentation.

... as opposed to the interrupt actually be "queued" somewhere so that it cannot be avoided even by clearing TFLG?

Yes, that's what happens on Teensy 3.x where the interrupt pending bit within the NVIC also comes into play. The NVIC is documented by ARM. But sadly there is no documentation (at least that I know about) which clearly explains the interaction between them.

[defragster]

...

But, I am little concerned about one thing, does calling delayMIcroseconds() in an ISR re-enable interrupts?

On Teensy 3.x any call to micros() - as done by delay() will leave with: __enable_irq();

This is NOT that case on Teensy 4.x with the code for micros() with atomic enforcing code to tolerate millis tick updates.

On Both T_3.x and T_4.x delayMicroseconds() is a raw wait loop based on F_CPU with no need to disable interrupts.

[PaulStoffregen]

In such a state, enabling the bit in the mask and enabling the global interrupt would result in an interrupt ...

ARM Cortex-M has nested priority-based interrupt, so when a pending interrupt can run is a little more complex than just a global interrupt enable bit.

[DrM]

@Defragster

Good to know. very helpful. Thank you.


@PaulStoffregen

Hi Paul, I would like to add a routine as follows, the intent is to resume interrupts after detach. Is this okay? Will it work and not conflict with anything else in the api? Thank you

void resumeInterrupts(uint8_t pin)
{
	if (pin >= CORE_NUM_DIGITAL) return;
	volatile uint32_t *gpio = portOutputRegister(pin);
	uint32_t mask = digitalPinToBitMask(pin);
        gpio[ISR_INDEX] = mask;  // clear pending
	gpio[IMR_INDEX] |= mask;  // re-enable
}

Aside, I want to use this to drop interrupts that occur between inner and outer, in the case where outer is run from an interrupt. Of course, for the detach from loop, I disable interrupts first..