digitalWrite & pinMode unsafe from interrupts

[GoogleCodeExporter]

If digitalWrite is used from any interrupt routine, the results can be
overwritten by digitalWrite running from the main program.  The two writes
do NOT need to be to the same pin, only to pins that share the same 8 bit
register.

For example, if the Servo library is updating pin 2 from its interrupt
routine and the main program is writing to pin 6, the write to pin 6 will
interfere with the write to pin 2 if the interrupt occurs while
digitalWrite from the main program is modifying the register.  The code
"*out |= bit" is implement using 3 instructions: load, or, store.  If the
main program has loaded the value, then the interrupt occurs, the interrupt
will change pin 2, but immediately upon return to the main program, the
change to pin 6 will cause the stale value of pin 2 to be written back. 
Very bad.

Interrupts need to be disabled while performing a read-modify-write
operation on a I/O register via a non-const pointer.

For digitalWrite()

        // disable interrupts while changing output register
        if (val == LOW) {
                uint8_t oldSREG = SREG;
                cli();
                *out &= ~bit;
                SREG = oldSREG;
        } else {
                uint8_t oldSREG = SREG;
                cli();
                *out |= bit;
                SREG = oldSREG;
        }

For pinMode()

        // disable interrupts while changing mode register
        if (mode == INPUT) {
                uint8_t oldSREG = SREG;
                cli();
                *reg &= ~bit;
                SREG = oldSREG;
        } else {
                uint8_t oldSREG = SREG;
                cli();
                *reg |= bit;
                SREG = oldSREG;
        }

Also, inside digitalWrite(), the PWM disable should be done after the
output regsiter is modified.  Between the PWM disable and the write to the
output register, the pin will output whatever the old value of the output
register was, and that time can be lengthened if an interrupt occurs.

For example:

digitalWrite(5, HIGH);
delay(100);
analogWrite(5, 2);
delay(500);
digitalWrite(5, LOW);

When the 1% duty PWM is disabled by the last digitalWrite, the pin will be
driven HIGH for a brief time before it is written LOW.  If an interrupt
occurs, a substantially long HIGH pulse could occur, when the user expects
a 1% duty cycle to become LOW.  The output register should be updated
before PWM is disabled so when the pin returns to the control of the output
register it will have the correct value.

Original issue reported on code.google.com by paul.sto...@gmail.com on 23 Nov 2009 at 10:55

[GoogleCodeExporter]

This bug seems to be sitting a long time without being accepted.
The bug was pretty well defined.
And a 12 line code fix was even offered.

(although I think it would be better to use the ATOMIC macros to 
guarantee the gcc optimizer doesn't re-order the status register manipulation)

This issue is showing how the routines in wiring_digital.c do not work properly
because the method of programming i/o pins does not work (isn't atomic)
on RISC processors without masking interrupts.

Not fixing this bug, means that sketches that have interrupt routines that use 
the
digital i/o functions will have hard to track down flakiness.

For example sketches that use the servo library will potentially stomp on the 
servo
libraries i/o pins when programing their other i/o pins that are unrelated to 
the
servo library's i/o pins.

Original comment by bperry...@gmail.com on 2 Feb 2010 at 2:12

[GoogleCodeExporter]

It seems to me that this is an example of the issue in the wild and an example 
of how
hard to track down the problem will always be:
http://forums.adafruit.com/viewtopic.php?f=31&t=7594

Original comment by johnrain...@gmail.com on 7 Mar 2010 at 2:49

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Another example of this bug: 
http://code.google.com/p/arduino/issues/detail?id=170&sort=-id

Once understood, this bug is not difficult to work around, but it's damn 
difficult to
track down in the first place.

Original comment by delsqua...@gmail.com on 22 Mar 2010 at 7:42

[GoogleCodeExporter]

Original comment by dmel...@gmail.com on 6 May 2010 at 6:04

• Changed state: Accepted
• Added labels: Component-Core, Milestone-0019

[GoogleCodeExporter]

I have noticed this as well in my (fairly) simple Pan-Tilt application.  I am 
using 
the liquidcrystal library as well as the servo library to control and display 
the 
pan/tilt position of the servos...

If I uncomment the LCD update code, the servo is rock solid.  When I run the 
update 
code, the servo will jerk (both axis) every couple of seconds.  I am using if 
(micros() % 10 == 0) to time the updates and the longer the delay, the less 
often the 
jerking occurs (every 1-3 seconds when no delay is present).

Attached is my current code.  I thought I was going crazy, and re-designed my 
circuit 
from scratch (including a secondary 5v regulator circuit), and for a relative 
beginner, it has been a tough couple days ;)

Original comment by nic...@gmail.com on 23 May 2010 at 7:31

Attachments:

• Pan_Tilt_Final.pde

[GoogleCodeExporter]

I made the changes Paul suggested. If you've been having problems with this, 
can you try the latest SVN revision and see if it helps?

Original comment by dmel...@gmail.com on 12 Jun 2010 at 8:33

• Changed state: Fixed

[GoogleCodeExporter]

Took me a while to get the build process completed, but after testing it on my 
pan-tilt servo appliction, and letting it sit for about 10 minutes, I didn't 
notice a single hiccup in the servos whatsoever.  Thanks so much for taking 
care of this long-standing bug!

Original comment by nic...@gmail.com on 13 Jun 2010 at 7:20

[GoogleCodeExporter]

Thanks for testing and sorry for the delay in getting this fixed.

Original comment by dmel...@gmail.com on 13 Jun 2010 at 7:22

[GoogleCodeExporter]

Hi, I am a noob. Do I have to recompile Arduino from the svn source in order to 
implement this servo jitter fix? Or is there a compiled version somewhere. 
Thanks (sensev at hotmail com)

Original comment by R.Seveel...@gmail.com on 21 Jun 2010 at 2:03

[GoogleCodeExporter]

Issue 170 has been merged into this issue.

Original comment by c.mag...@arduino.cc on 3 Jul 2015 at 1:33