5

I'm trying to write a faster shiftOut function, which does not use the slow digitalWrite. In my code I have the original shiftOut, the new shiftOutFast, and the same as shiftOutFast, but inline. The three blocks are separated by delays to tell them apart on my scope. Here's the code:

#define CLR(x,y) (x &= (~(1 << y)) )
#define SET(x,y) (x |= (1 << y) )

void shiftOutFast(uint8_t dataPort, uint8_t dataBit, uint8_t clkPort, 
                  uint8_t clkBit, uint8_t bitOrder, uint8_t val) {
  for (uint8_t i = 0; i < 8; i++)  {
    if (bitOrder == LSBFIRST) {
      if (val & 0x01) {
        SET(dataPort, dataBit);
      } else {
        CLR(dataPort, dataBit);
      }
      val >>= 1;
    } else {    
      if (val & 0x80) {
        SET(dataPort, dataBit);
      } else {
        CLR(dataPort, dataBit);
      }
      val <<= 1;
    }   

    CLR(clkPort, clkBit);
    SET(clkPort, clkBit);
  }
}


#define dataPin 4
#define clockPin 5
#define dataPort PORTD
#define dataBit PORTD4
#define clockPort PORTD
#define clockBit PORTD5

void setup() {
  pinMode(dataPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
}

void loop() {
  shiftOut(dataPin, clockPin, LSBFIRST, 0x55);
  delay(1);

  shiftOutFast(dataPort, dataBit, clockPort, clockBit, LSBFIRST, 0x5C);
  delay(1);  

  bool bitOrder = LSBFIRST;
  uint8_t val = 0x5C; 

  for (uint8_t i = 0; i < 8; i++)  {
    if (bitOrder == LSBFIRST) {
      if (val & 0x01) {
        SET(dataPort, dataBit);
      } else {
        CLR(dataPort, dataBit);
      }
      val >>= 1;
    } else {    
      if (val & 0x80) {
        SET(dataPort, dataBit);
      } else {
        CLR(dataPort, dataBit);
      }
      val <<= 1;
    }   

    CLR(clockPort, clockBit);
    SET(clockPort, clockBit);
  }
  delay(1);  
}

The problem is that calling shiftOutFast doesn't do zilch. Absolutely nothing. It doesn't seem to be the logic, because when I do the same inline it works. Any ideas?

11

The problem is that you are passing the value of the ports to the function, not the address of the ports.

You need to either pass "by reference" or as a pointer, so that modifying the port variable modifies the actual port variable not the copy of the value that you pass.

The simplest way is to modify your function to be:

void shiftOutFast(volatile uint8_t &dataPort, uint8_t dataBit,
                  volatile uint8_t &clkPort, uint8_t clkBit, 
                  uint8_t bitOrder, uint8_t val) {
....
}

That way, instead of passing whatever is in the port at the time to the function (pass by value) it instead creates a new variable which is located at the same address as the variable that you are passing (pass by reference) so modifications of one variable modifies the other.

Note that only the "port" variables need to be passed by reference, all the others remain as pass by value. Also they should be, as Edgar says, flagged as volatile.

6

You should declare volatile the port parameters of your function. Otherwise, to the compiler, their write accesses seem useless, and they get optimized out.

  • You mean void shiftOutFast(volatile uint8_t dataPort, volatile uint8_t dataBit, volatile uint8_t clkPort, etc..? I tried it but it doesn't solve the problem. – Joris Groosman May 10 '15 at 12:45
  • only the ports, and as @Majenko said, you should pass them by reference. – Edgar Bonet May 10 '15 at 12:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.