Can someone please check my coding for using 2*595 shift registers?

I have been working on using 595 shift registers. I have now linked 2 together, but I am not sure if I am using excessive code. I have taken code from this site and finally slimmed it down to just place 2 numbers, to operate the outputs of both register:

https://docs.arduino.cc/tutorials/communication/guide-to-shift-out/

I have wired mine as per the 2 registers linked:

The main issue is the coding, which has been borrowed from that website I mentioned:

/*
  Extant4Life: A very basic input to set the outputs of each shift reg
  Total 595s is 2. Further analysis will be needed to add a 3rd or more
*/

//Pin connected to ST_CP of 74HC595
int latchPin = 8;
//Pin connected to SH_CP of 74HC595
int clockPin = 12;
////Pin connected to DS of 74HC595
int dataPin = 11;

void setup() {
  //set pins to output because they are addressed in the main loop
  pinMode(latchPin, OUTPUT);
}
void loop() {
  //... just sets both 595s
  setBothvalues(140, 106);
}

//First value sets the first 595, then second number sets the second 595
void setBothvalues(int shiftReg1Val, int shiftReg2Val ) {
  digitalWrite(latchPin, 0);
  shiftOut(dataPin, clockPin, shiftReg2Val);
  shiftOut(dataPin, clockPin, shiftReg1Val);
  digitalWrite(latchPin, 1);
}

// the heart of the program (original comment from coder)
//NOTE: this is the original code from a site, with mostly their annotations
//Some annotations from me due to experimentation
void shiftOut(int myDataPin, int myClockPin, byte myDataOut) {
  // This shifts 8 bits out MSB first,
  //on the rising edge of the clock,
  //clock idles low
  //internal function setup
  int i = 0;
  int pinState = 0;
  pinMode(myClockPin, OUTPUT);
  pinMode(myDataPin, OUTPUT);
  //clear everything out just in case to
  //prepare shift register for bit shifting
  digitalWrite(myDataPin, 0);
  digitalWrite(myClockPin, 0);
  //for each bit in the byte myDataOut�
  //NOTICE THAT WE ARE COUNTING DOWN in our for loop
  //This means that %00000001 or "1" will go through such
  //that it will be pin Q0 that lights.
  for (i = 7; i >= 0; i--) {//... was i = 7
    digitalWrite(myClockPin, 0);//... was 0!
    //if the value passed to myDataOut and a bitmask result
    // true then... so if we are at i=6 and our value is
    // %11010100 it would the code compares it to %01000000
    // and proceeds to set pinState to 1.
    if (myDataOut & (1 << i)) {//... was 1 << i
      pinState = 1;//... was 1!
    } else {
      pinState = 0;//... was 0!
    }
    //Sets the pin to HIGH or LOW depending on pinState
    digitalWrite(myDataPin, pinState);
    //register shifts bits on upstroke of clock pin
    digitalWrite(myClockPin, 1);
    //zero the data pin after shift to prevent bleed through
    digitalWrite(myDataPin, 0);
  }
  //stop shifting
  digitalWrite(myClockPin, 0);//... apparently you need this!!
}

I was mainly wondering about the shiftOut() function that they created. It works perfectly fine btw. I was concerned if there was any excess code that I have included that is unneeded. Any advice is appreciated.

Additionally, I noticed that shiftOut() does not actually need to be declared in the Arduino IDE. Apparently they have modified it, with additional function.

  • Always show us a good schematic of your proposed circuit.
    Show us good images of your ‘actual’ wiring.

The exact wiring is what I posted in the image. It works as expected. I maybe should have put this into programming instead of general electronics.

If you really want to see the wiring, it looks something like this:

That will be more appealing, and clearer than an image of my wiring. :squinting_face_with_tongue:

I will be honest, my question is bad, and really asking for too much from people. It is too open ended I do not think it a fair question, so it is best if I just work stuff out for myself, rather than ask people to hunt through the code.

I do not have permission to delete the question myself, but by all means, if an admin wants to, then it is no big deal.

  • We are sure you are sure your wiring matches the wiring diagram. Unfortunately we often see too many times where this is not the case.
  • We always invite users to follow the forum’s posting guidelines.
  • Only when everyone is brought up to speed can volunteers make informative suggestions for a fix to your problem.
  • Please read the posting guidelines.

Your wiring doesn’t need to look like spaghetti, neaten it up.

It does, but it does not matter tbh. I just posted above that my question is not a fair question anyway, so I have no problem if the thread is deleted.

I just need to work through it myself. Thank you for your time anyway. I should not have posted the thread in the first place :rofl:

Oh that is great. Thank you. I will look into that instead of reinventing the wheel then. Good call Larry.

You are because most of your code is this function:

But there's a standard Arduino function for that:

https://docs.arduino.cc/language-reference/en/functions/advanced-io/shiftOut/

Yes, I did notice that Paul, because on other test projects I didn’t even need to create the function. As you say, it is actually already there and does not need to be created, and can just be declared in the Arduino IDE.

The probability is that, the function I am using in my code, should even be named differently. In that way it differentiates it from the existing function.

I actually like Larry’s suggestion above to use the SPI library. No need to reinvent the wheel, as the saying goes.

No, giving it a different name doesn't justify having it in your code, because there is a standard function that performs the same... function!

It's possible that the code you found pre-dates the introduction of the standard Arduino function. So if you use that old code as-is, leave it as-is, because it's been tested and works. But if you are creating new code, you will have to test that new code, so it's better to minimise the amount of code you need to test by taking advantage of the standard function.