bperrybap:
That doesn't look like what I suggested as that is still doing lots of runtime calculations on bit masks.
If you want fast, you can't do that. You should calculate as much as you can up front so it is only done once.
You should check for a constant bit in the value (either bit 7 or 0) and then set the data pin either high or low based on that.
Then shift out the data bit in the shift register by bumping the clock pin.
I'm also not sure how that code can work as you shift the data the opposite direction before you start looping which will cause you to loose a bit on the end.
Also, this may not be doing what you might think:
if(*port == PORTB) j = 8;
if(*port == PORTC) j = 14;
if(*port == PORTD) j = 0;
These are comparing the contents of two port registers vs the addresses of the port registers.
That is not a good way of checking since it takes more overhead to actually the read the contents and the compare them vs just comparing the addresses. Also, the contents of the registers could be changing if they are attached to external signal inputs so the tests might actually fail.
Compares like that should compare the address of the port register vs the contents of the port register.
i.e.
if(port == &PORTB) j = 8;
etc...
Here is some sample code that I did from an LCD library. (It is from fm's newLiquidCrystal library)
It is extremely fast as it optimizes down to minimal instructions.
It uses the AVR libC ATOMIC block code for the atomic register updates, while allow interrupts to sneak in between bits should they occur.
This offers fast output while minimizing ISR latency.
This code was further optimized by using some storage in the device object to precalculate some of the register pointers and bits like the clock port and clock bit, and data register and data bit.
(The actual module can be found here: https://bitbucket.org/fmalpartida/new-liquidcrystal/src/integration/FastIO.cpp )
void fio_shiftOut (fio_register dataRegister, fio_bit dataBit,
fio_register clockRegister, fio_bit clockBit,
uint8_t value, uint8_t bitOrder)
{
int8_t i;
if(bitOrder == LSBFIRST)
{
for(i = 0; i < 8; i++)
{
ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
{
if(value & 1)
{
fio_digitalWrite_HIGH (dataRegister, dataBit);
}
else
{
fio_digitalWrite_LOW (dataRegister, dataBit);
}
value >>= 1;
fio_digitalWrite_HIGH (clockRegister, clockBit);
fio_digitalWrite_LOW (clockRegister, clockBit);
}
}
}
else
{
for(i = 0; i < 8; i++)
{
ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
{
if(value & 0x80)
{
fio_digitalWrite_HIGH (dataRegister, dataBit);
}
else
{
fio_digitalWrite_LOW (dataRegister, dataBit);
}
value <<= 1;
fio_digitalWrite_HIGH (clockRegister, clockBit);
fio_digitalWrite_LOW (clockRegister, clockBit);
}
}
}
}
Note that there is no runtime bit mask calculations in the loop.
A single bit in the value is tested, and the Arduino pin connected to the data pin of the shift register is set to high or low.
Then the clock is strobed.
All the register locations and bits within the registers are pre-calculated.
In this case they are done prior to calling fio_shiftOut() for maximum performance but you also can do it up front in the actual shiftout() function and still achieve a large win.
--- bill
Hi,
I adjusted several things in the shift_out function and this is how I ended up now:
#define digitalWrite_LOW(reg, bit) *reg &= ~bit
#define digitalWrite_HIGH(reg, bit) *reg |= bit
shift_out (volatile uint8_t *port,
uint8_t dataPin,
uint8_t clockPin,
uint8_t latchPin,
uint8_t Bitorder,
uint8_t val)
{
uint8_t i;
if(Bitorder == MSBFIRST)
{
for (i = 0; i < 8; i++)
{
ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
{
(!!(val & 0x80))?(digitalWrite_HIGH(port, dataPin)):(digitalWrite_LOW(port, dataPin)); // shift out
digitalWrite_HIGH(port, clockPin); // toggle the clock pin
digitalWrite_LOW(port, clockPin);
//_delay_us(xxx);
val <<= 1;
}
}
}
else
{
for (i = 0; i < 8; i++)
{
ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
{
(!!(val & 1))?(digitalWrite_HIGH(port, dataPin)):(digitalWrite_LOW(port, dataPin)); // shift out
digitalWrite_HIGH(port, clockPin); // toggle the clock pin
digitalWrite_LOW(port, clockPin);
//_delay_us(xxx);
val >>= 1;
}
}
}
ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
{
digitalWrite_HIGH(port, latchPin); //toggle the latch pin
digitalWrite_LOW(port, latchPin);
}
}
Scraped the calculation of the bit positions (with the variable j) again and do now just pass the bit positions according to the port in the arduino code directly. Also made use of the atomic blocks and fixed the shifting of my value, which I shifted into the wrong direction at first. I might further improve it to use seperate ports for the 3 pins if needed.
I ran the output of the shift register through an oscilloscope again and it looks neat. For one byte to be output the new shiftout function requires a little less than 10 µs.
Thanks again for your time and detailed answer bill, it helped me out a LOT!
Winterwurst
