DigitalWrite fails after analogWrite

[nseidle]

Suppose a user does an analogWrite to an output pin. Then they do a digitalWrite HIGH to the same pin. On Artemis as currently implemented, the pin will remain in PWM / analog output mode rather than going HIGH. However, the ATmega328 core will actually drive the pin high. This discrepancy leads to some user code not working correctly when users switch to Artemis.

I'm trying to figure out the best place to implement the fix. I believe we could do a .uFuncSel on a given pin inside digitalWrite:

But that would basically be a pinMode(xPin, OUTPUT); every time digitalWrite was called (not good).

I believe we can't easily read or decipher a pad's current funcsel. Correct me if I'm wrong. So I am slowly leaning towards a global array of 50 uint8_ts that keep track of how each pin/pad is setup (in, out, analogin, analogout, i2c, spi, pdm, uart, ?). This would replace and supersede the ap3_analog_configure_map (used to keep track of which pads are currently defined as analogInput).

I love how "this will be a simple fix" turns into a big deal...

[nseidle]

Code that fails to blink led (but should). Use on RedBoard Artemis:

int ledPin = 13;

void setup() {
  pinMode(ledPin, OUTPUT);
  analogWrite(ledPin, 255 / 10);
}

void loop() {
  digitalWrite(ledPin, HIGH);
  delay(1000);
  digitalWrite(ledPin, LOW);
  delay(1000);
}

[Wenn0101]

I think we can check the function selection.

Check out this branch.

How do you feel about checking the function select like this?
I am not sure how I feel about setting it to GPIO every time there is a digital write and the function isn't GPIO. I am trying to imagine situations where this might lead to confusion.

[Wenn0101]

I did some timing tests using this sketch.

int ledPin = A0;

void setup() {
  pinMode(ledPin, OUTPUT);
  Serial.begin(9600);
  Serial.println("Test Begin");
}

void loop() {
   Serial.println(millis());
  for(int i = 0; i< 100000 ; i++)
  {
  digitalWrite(ledPin, HIGH);
  digitalWrite(ledPin, LOW);
  }
  Serial.println(millis());

  delay(10000);
}

Here are the results for how long (ms as measured by millis()) each set of 100,000 digital write on/off took. This is just time added to perform the check, not switch the pin back to GPIO mode.

Baseline time - 114ms (.57 us per digital write)
Time with proposed changes(inline function) - 271ms (1.36 us per digital write)
Time with proposed changes(function) - 296ms (1.48 us per digital write)
Time where check is changed to checking a volatile flag (closer to proposed solution above) - 214 ms (1.07 us per digital write)

I modified the last one to force the pinMode change every time and it took - 1642 ms (8.21 us per digital write)

@nseidle How do you feel about these timings?

[oclyke]

Just adding a note here to link this with PR #148

Waiting to merge that PR until we:
a) agree that this is a desirable portion of the Arduino API to follow (detrimental possibility of de-confinguring pins when calling digitalWrite on them)
b) decide which of the proposed methods to follow (fastest? least overhead?)

[nseidle]

I'm split. I see the need to be as much 'Arduino compatible' as possible but this 'bug' is a pretty fringe case that no one has reported as a problem in many months. I really like the fast/pure digitalWrites we currently have. I don't think doubling or tripling that is not worth the bubble wrap of preventing the user from doing something they shouldn't really be doing anyway (you should define the pin func, then use it that way).

@Wenn0101 - Thanks for writing the fix! Regardless of bug decision, I think we should keep the ap3_get_funct_sel() method in the core for future use.

I vote we close this as 'not going to fix'. @oclyke?