Does this shiftout code look correct?

Projects Programming
1

I'm trying to make a tachometer that measures the time between the rising edge of a square wave and shifts out the results to three shift registers. The 24 leds are supposed to light up in sequence like a bar graph.

After the RPM is divided by 250 (250 x 24 leds = 6000 RPM redline), the result is used to access an array containing hex values to be shifted out.

It compiles but I don't have a way to test it (Need to etch the board), I'm wondering if this looks correct to you guys, particularly the bitshifting to the second and third registers. Can you use bitshift with hex values, and am I using it correctly to output to a 24 led bargraph?

boolean Cycle = false;                  // set to 0 for PulseStartTime and set to 1 for each PulseEndTime
unsigned long PulseStartTime;   // Saves Start of pulse in ms
unsigned long PulseEndTime;     // Saves End of pulse in ms
unsigned long PulseTime;        // Stores dif between start and stop of pulse
unsigned long RPM = 0;          // RPM to ouptut (30*1000/PulseTime)
int shiftarray[25] = {0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F, 0xFF, 0x1FF, 0x3FF, 0x7FF, 0xFFF, 0x1FFF, 0x3FFF, 0x7FFF, 0xFFFF, 0x1FFFF, 0x3FFFF, 0x7FFFF, 0xFFFFF, 0x1FFFFF, 0x3FFFFF, 0x7FFFFF, 0xFFFFFF};  
int latchPin = 8; //Shift register latch pin
int clockPin = 12; //Shift register clock pin
int dataPin = 11; //Shift register data pin
int ledData; //Value to access the array with

void setup()
{
 pinMode(latchPin, OUTPUT);
 pinMode(clockPin, OUTPUT);
 pinMode(dataPin, OUTPUT);
 attachInterrupt(0, RPMPulse, RISING); // Attaches interrupt to Digital Pin 2
}

void RPMPulse()
{
  if (Cycle)                // Check to see if start pulse
  {
    PulseStartTime = millis();  // stores start time (TEST GOING TO MICROS)
    Cycle = true;           // sets counter for start of pulse
  }
  else
  {
    detachInterrupt(0);         // Turns off inturrupt for calculations
    PulseEndTime = millis();    // stores end time
    Cycle = false;                  // resets counter for pulse cycle
    calcRPM();                  // call to calculate pulse time
  }
}

void calcRPM()
{
  PulseTime = PulseEndTime - PulseStartTime; // Gets pulse duration
  RPM = 30*1000/PulseTime*2;                 // Calculates RPM
  attachInterrupt(0, RPMPulse, RISING);      // re-attaches interrupt to Digi Pin 2
}

void loop()
{
  ledData = RPM/250; //Should divide RPM by 250 (On the gauge, one led is 250 RPM). Keep in mind any float values get tossed (499rpm would end up as 250)
  shiftOut(dataPin, clockPin, LSBFIRST, shiftarray[ledData]); 
  shiftOut(dataPin, clockPin, LSBFIRST, (shiftarray[ledData] >> 8));
  shiftOut(dataPin, clockPin, LSBFIRST, (shiftarray[ledData] >> 16));
  delay(1000);                  // 1 sec delay for debugging
}
2

Hello,

No, it won't work, your shiftarray is of type int (16 bits), but you try to store 24bits numbers in there.

There is a much easier solution, that doesn't require an array. For example, to make it easier to understand, let's say you have 8 LEDs, and the engine's red line is 8 RPM :).

At 1 RPM you want to write the shift registers with binary value 0b10000000
At 2 RPM you want to write the shift registers with binary value 0b11000000

Etc.

This value can be calculated very easily:

uint8_t rpm = 4; // for example

uint8_t data = 256 - ( 1 << (8 - rpm) );

With this code, data's value would be 0b11110000 (or 240 in decimal), so you would just shiftOut this value to see the 4 first LEDs on and the 4 last LEDs off

Now for 24 bits it's a bit trickier, but give it a try :wink:

3

That looks like a much better solution. And right as I was about to say I don't understand how to convert it to work with 24 bits, I realized my attempts failed because the decimal number needs to be one more than the binary equivalent of 24 bits being "1".

This is what I came up with, as far as I can tell it works:

uint8_t rpm = 4;

uint32_t data = 16777216 - ( 1 << (24 - rpm) );

Then it's just a matter of three shiftouts, two of them bitshifting by 8 and 16.

Curious though, it took me a little while to see how that math works. Tricky, How did you come up with it?

4

Well I don't really know how to explain but

1 << x is the same as 2^x

So for example 1 << 4 = 16, in binary that is 0b10000
0b100000000 (256) - 0b10000 = 0b11110000

Note that you can do it even easier if you don't care about bits order being "normal" (such that 1 RPM = 0b00000001, etc) by doing (1 << x) - 1... which may be what you want, since your array's values are in that order :wink:

(1 << 4) - 1 = 0b00001111

5

AHA! Yes!

Thank you very much. And thanks for making me figure it out instead of giving me the answer. It looks like this will even fit on an attiny85 when I'm done :slight_smile:

If you're wondering, I'm making a more modern instrument cluster for a 1985 Saab 900. Tach, speedo, boost, fuel and temp along with a 16x4 information display. Everything is modeled in Autocad Inventor and I'm about ready to etch the boards.

6

This section also has a problem. Cycle never gets toggled, if it's already true you set it to true, if it's already false you set it to false.

Also it will not work as you've failed to declare pulsStartTime and Cycle as volatile (so any changes you make within the ISR will be lost.

  if (Cycle)                // Check to see if start pulse
  {
    PulseStartTime = millis();  // stores start time (TEST GOING TO MICROS)
    Cycle = true;           // sets counter for start of pulse
  }
  else
  {
    detachInterrupt(0);         // Turns off inturrupt for calculations
    PulseEndTime = millis();    // stores end time
    Cycle = false;                  // resets counter for pulse cycle
    calcRPM();                  // call to calculate pulse time
  }
7

Whoops, swapped the true and false.

This better?

volatile boolean Cycle = true;                  // set to 0 for PulseStartTime and set to 1 for each PulseEndTime
volatile unsigned long PulseStartTime;   // Saves Start of pulse in ms
volatile unsigned long PulseEndTime;     // Saves End of pulse in ms

void RPMPulse()
{
  if (Cycle)                // Check to see if start pulse
  {
    PulseStartTime = micros();  // stores start time (TEST GOING TO MICROS)
    Cycle = false;           // sets counter for start of pulse
  }
  else
  {
    detachInterrupt(0);         // Turns off inturrupt for calculations
    PulseEndTime = micros();    // stores end time
    Cycle = true;                  // resets counter for pulse cycle
    calcRPM();                  // call to calculate pulse time
  }
}