Adding digitalToggle to core?

[RobTillaart]

Description

The Arduino core has a digitalWrite() and a digitalRead() function. In many sketches there is a need to toggle a pin, either for a blinking LED or a clock pin for swSPI etc.

There are two typical ways to invert a pin - see code below

  // using a state holding the value of the pin
  digitalWrite(pin, state);
  state = 1 - state;

  // read pin, invert and write back
  digitalWrite(pin, !digitalRead(pin));

The latter one is slower as it redo a lot of "pin math", so I implemented a version of digitalToggle() for AVR. Attached test sketch shows the gain compared to the two methods.

Results test on UNO

Time us		1000 calls
Reference:	7392	// read invert write
      Var:	4784	// use state var
   Toggle:	3964	// use digitalToggle returning state
   Toggle:	3520	// use digitalToggle returning NO state

The gain of toggle returning state is 46% resp 16% The gain of toggle returning NO state is 52% resp 26%

Imho these gains are interesting, esp for clocking data

Implementation for AVR

Arduino.h

uint8_t digitalToggle(uint8_t pin);     // returns the new state of the pin

wiring_digital.c

uint8_t digitalToggle(uint8_t pin)
{
  uint8_t port = digitalPinToPort(pin);
  volatile uint8_t *out;

  if (port == NOT_A_PIN) return 0;
  uint8_t timer = digitalPinToTimer(pin);
  if (timer != NOT_ON_TIMER) turnOffPWM(timer);

  uint8_t bit = digitalPinToBitMask(pin);

  out = portOutputRegister(port);
  uint8_t oldSREG = SREG;
  cli();
  *out ^= bit;      // invert bit
  SREG = oldSREG;
  return ((*out & bit) != 0);
}

Note: a no state returning version is straightforward given the above code.

Test sketch

digitalToggle.zip

uint32_t start, Tref, Tref2, Tnew;
const uint8_t pin = 13;

uint8_t state = LOW;

void setup()
{
  Serial.begin(115200);
  Serial.println();
  Serial.println(__FILE__);

  pinMode(pin, OUTPUT);
  digitalWrite(pin, LOW);

  start = micros();
  for (int i = 0; i < 1000; i++) digitalWrite(pin, !digitalRead(pin));
  Tref = micros() - start;

  start = micros();
  for (int i = 0; i < 1000; i++)
  {
    digitalWrite(pin, state);
    state = 1 - state;
  }
  Tref2 = micros() - start;

  start = micros();
  for (int i = 0; i < 1000; i++) digitalToggle(pin);
  Tnew = micros() - start;

  Serial.print("Reference:\t");
  Serial.println(Tref);
  Serial.print("      Var:\t");
  Serial.println(Tref2);
  Serial.print("   Toggle:\t");
  Serial.println(Tnew);
  Serial.print("     Gain:\t");
  Serial.println(Tref - Tnew);
  Serial.print("     Perc:\t");
  Serial.println(100.0 - (100.0 * Tnew) / Tref, 1);

  pinMode(13, OUTPUT);
}

void loop()
{
  static int cnt = 0;
  if (cnt == 60)
  {
    cnt = 0;
    Serial.println();
  }
  cnt++;
  // digitalToggle(pin);
  int x = digitalToggle(pin);
  Serial.print(x);

  delay(1000);
}

[matthijskooijman]

Sounds like a good additoin. I think it belongs to the ArduinoCore-API repo, though, so I'm moving it there.

As for AVR, IIRC you can also toggle a pin by writing a 1 to the PINx register, which would be even (slightly) faster.

[RobTillaart]

Implementation for ESP32

For ESP32 platform I have not yet tested code.
No gain expected here, although there is one (1 << pin) and one if () less on average

esp32-hal-gpio.h

int digitalToggle(uint8_t pin);

esp32-hal-gpio.c

extern int IRAM_ATTR __digitalToggle(uint8_t pin)
{
  uint32_t val = 0;
  uint32_t mask = 0x01;
  
  if (pin < 32) 
  {
    mask = (0x01 << pin);
    val =  (GPIO.in & mask) != 0;
    if (val) GPIO.out_w1tc = ((uint32_t) mask);
    else     GPIO.out_w1ts = ((uint32_t) mask);
    return (val == 0);
  } 
  if( pin < 34)
  {
    mask = (0x01 << (pin - 32));
    val = (GPIO.in1.val & mask) != 0;
    if (val) GPIO.out1_w1tc.val = ((uint32_t) mask);
    else     GPIO.out1_w1ts.val = ((uint32_t) mask);
    return (val == 0);
  }
  return 0;
}

extern int digitalToggle(uint8_t pin) __attribute__ ((weak, alias("__digitalToggle")));

[edgar-bonet]

Nice idea. Here is a small optimization opportunity:

  return ((*out & bit) != 0);

The compiler should issue a “load” instruction for *out, in order to read the port register, although its contents is known, as it has just been written. You can save this memory access by explicitly caching that contents:

  uint8_t state = *out;
  state ^= bit;      // invert bit
  *out = state;
  SREG = oldSREG;
  return ((state & bit) != 0);

I would expect this to save two CPU cycles, although I did not test. The compiler cannot perform this optimization itself because the volatile keyword explicitly forbids it.

[Perehama]

That was my first contribution here on github, as I just had the same idea others have had. I came here from the Arduino forums. My only contribution is to make it a void return so it operates otherwise like digitalWrite(). There is an AVR specific version that would write a 1 to the portInputRegister(port) as per Atmel, writing a 1 to PINx will toggle that pin, but the XOR operation is portable to other microcontrollers.

void digitalToggle(uint8_t pin) {
  uint8_t timer = digitalPinToTimer(pin);
  uint8_t bit = digitalPinToBitMask(pin);
  uint8_t port = digitalPinToPort(pin);
  volatile uint8_t *out;
  if (port == NOT_A_PIN) return;
  if (timer != NOT_ON_TIMER) turnOffPWM(timer);
  out = portOutputRegister(port);
  uint8_t oldSREG = SREG;
  cli();
  *out ^= bit;
  SREG = oldSREG;
}