Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

digitalWrite & pinMode unsafe from interrupts [imported] #146

Closed
cmaglie opened this issue Nov 15, 2012 · 0 comments
Closed

digitalWrite & pinMode unsafe from interrupts [imported] #146

cmaglie opened this issue Nov 15, 2012 · 0 comments

Comments

@cmaglie
Copy link
Member

cmaglie commented Nov 15, 2012

This is Issue 146 moved from a Google Code project.
Added by 2009-11-23T10:55:56.000Z by paul.sto...@gmail.com.
Please review that bug for more context and additional comments, but update this bug.
Closed (Fixed).

Original labels: Type-Defect, Priority-Medium, Component-Core, Milestone-0019

Original description

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.

@cmaglie cmaglie closed this as completed Nov 15, 2012
tbowmo pushed a commit to tbowmo/Arduino that referenced this issue Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant