Why doesn't my code work in a function, yet works inline?

Question

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?

Majenko · Accepted Answer · 2015-05-10 12:50:09Z

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.

Edgar Bonet · Accepted Answer · 2015-05-10 12:02:41Z

5

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

answered May 10, 2015 at 12:02

Edgar Bonet

45.2k4 gold badges42 silver badges81 bronze badges

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
– Joris Groosman

2015-05-10 12:45:05 +00:00
Commented May 10, 2015 at 12:45
only the ports, and as @Majenko said, you should pass them by reference.

Edgar Bonet
– Edgar Bonet

2015-05-10 12:57:14 +00:00
Commented May 10, 2015 at 12:57

Add a comment |

Stack Exchange Network

Why doesn't my code work in a function, yet works inline?

2 Answers 2

Your Answer

Hot Network Questions

Why doesn't my code work in a function, yet works inline?

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions