Functions Not Executing

I recently created a project to interface to an 8-bit computer I made. The idea was that my Uno would act as an output device to the computer and drive a 4-digit 7-segment display. I used a 74HC252 shift register to control the display segments and used the Uno’s A0-A3 to control the digits (common cathode). All of the digital pins on the Uno are used for something, namely reading the data bus and one bit of the address bus. The computer already had a bar graph display as its output and the clock is active high. Once you see the code, you’ll be able to see what I’m doing.

Here is the code that currently works:

/*
   Bubble Display Info

   Common Cathode

   1.6V - 2V drop
   5mA/segment
   680 Ohm Resistors

   01 - Cathode 1
   02 - E
   03 - C
   04 - Cathode 3
   05 - DP
   06 - Cathode 4
   07 - G
   08 - D
   09 - F
   10 - Cathode 2
   11 - B
   12 - A
*/

// Interface Pins
#define ADD 13 // Connect to A0 on address bus. 1 = left byte; 0 = right byte
#define D7 12 // The following pins connect to D7 - D0 on data bus
#define D6 11
#define D5 10
#define D4 9
#define D3 8
#define D2 7
#define D1 6
#define D0 5
#define SRCLK 4 // Shift register clock
#define SRDAT 3 // Shift register data
#define CLK 2 // Attach an interrupt to this pin. It comes from decoded IO and WR (already drives CLK of output latch). Rising edge
#define SROE 1 // Shift register /OE
#define SRLAT 0 // Shift register latch

// Common Cathode Conections
#define C1 A0
#define C2 A1
#define C3 A2
#define C4 A3

// Gen Rules
#define method LSBFIRST

volatile byte lb = 0x00; // LB = digit 1 and 2 data; written to if ADD = 1
volatile byte rb = 0x00; // RB = digit 3 and 4 data; written to if ADD = 0
volatile byte flags = 0b00000000;
/* byte used for flags
   b0 = "update lb"
   b1 = "update rb"
   b2 - b7 = unused
*/

byte char1 = 0x00; // Character 1
byte char2 = 0x00; // Character 2
byte char3 = 0x00; // Character 3
byte char4 = 0x00; // Character 4

// The following array is a lookup table
byte pattern[] = {0b11111100, 0b01100000, 0b11011010, 0b11110010, 
                  0b01100110, 0b10110110, 0b10111110, 0b11100000, 
                  0b11111110, 0b11110110, 0b11101110, 0b00111110, 
                  0b10011100, 0b01111010, 0b10011110, 0b10001110};


void setup() {
  // Set up digital IO
  for (int i = 13; i > 4; i--) {
    pinMode(i, INPUT);
  }

  // control lines
  digitalWrite(SRCLK, 0);
  digitalWrite(SRDAT, 0);
  digitalWrite(SROE, 1);
  digitalWrite(SRLAT, 0);
  pinMode(SRCLK, OUTPUT);
  pinMode(SRDAT, OUTPUT);
  pinMode(SROE, OUTPUT);
  pinMode(SRLAT, OUTPUT);

  // character lines
  digitalWrite(C1, 1);
  digitalWrite(C2, 1);
  digitalWrite(C3, 1);
  digitalWrite(C4, 1);
  pinMode(C1, OUTPUT);
  pinMode(C2, OUTPUT);
  pinMode(C3, OUTPUT);
  pinMode(C4, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(CLK), ISR_Update_Val, RISING);
}

void loop() {
  // Put your main code here, to run repeatedly:
  if (flags == 0x01) {
    char1 = lb >> 4;
    char2 = lb & 0x0F;
    flags = flags & 0xFE;
  }
  if (flags == 0x02) {
    char3 = rb >> 4;
    char4 = rb & 0x0F;
    flags = flags & 0xFD;
  }

    // Write to the display
    // Digit 1
    shiftOut(SRDAT, SRCLK, method, pattern[char1]);
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C1, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char2]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C1, 1);

    // Digit 2
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C2, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char3]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C2, 1);

    // Digit 3
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C3, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char4]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C3, 1);

    //Digit 4
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C4, 0);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C4, 1);
  }


  void ISR_Update_Val() {
    // Read data, update LB or RB, set flag
    // Timing isn't as critical in practice as it sounds in theory (for this application anyway)

    if (digitalRead(ADD) == 0) {
      bitWrite(rb, 0, digitalRead(D0));
      bitWrite(rb, 1, digitalRead(D1));
      bitWrite(rb, 2, digitalRead(D2));
      bitWrite(rb, 3, digitalRead(D3));
      bitWrite(rb, 4, digitalRead(D4));
      bitWrite(rb, 5, digitalRead(D5));
      bitWrite(rb, 6, digitalRead(D6));
      bitWrite(rb, 7, digitalRead(D7));
      flags = flags | 0x02;
    }

    else {
      bitWrite(lb, 0, digitalRead(D0));
      bitWrite(lb, 1, digitalRead(D1));
      bitWrite(lb, 2, digitalRead(D2));
      bitWrite(lb, 3, digitalRead(D3));
      bitWrite(lb, 4, digitalRead(D4));
      bitWrite(lb, 5, digitalRead(D5));
      bitWrite(lb, 6, digitalRead(D6));
      bitWrite(lb, 7, digitalRead(D7));
      flags = flags | 0x01;
    }
  }

It looks a little messy, but I had to format it like so because my initial code (below) failed to work. I found a few wiring issues, which I fixed, but it still failed to run. I couldn’t figure out why until I put my DMM to the circuit. It looked to me like the shift register was getting initialized with the initial 00’s but it wasn’t doing anything else. My DMM has a frequency tester as well as a duty cycle checker. Using these, I determined that the Uno was not sending anything out to, well, anything; the shift register nor the display. I wired one of the display cathodes directly to ground (removed the connection to the Uno) and got nothing out. I then did the same with the shift register’s /OE and it put out a 0. It started to seem like my functions were not being executed. I went into my code and cut out the functions and pasted them into the main loop and suddenly it started working flawlessly.

Note that this code has been slightly optimized to use one less byte of RAM. The code I originally used more like the first one but in the format of this one.

(I was asked to attach the code instead of replying with it. io7[…]2.ino is the version posted in this post. The other one is the one that’s not working)

io7segdisp.ino (4.48 KB)

io7segdisp2.ino (4.23 KB)

Once you see the code, you'll be able to see what I'm doing.

Not really. What does this opaque code do?

  if (flags == 0x01) {
    char1 = inByte >> 4;
    char2 = inByte & 0x0F;
    flags = flags & 0xFE;
  }

aarg:
What does this opaque code do?

  if (flags == 0x01) {

char1 = inByte >> 4;
    char2 = inByte & 0x0F;
    flags = flags & 0xFE;
  }

Exactly what it looks like. It takes a byte of data and stores the upper nibble as one variable and then the lower nibble as the next variable, then resets the "flag" variable (well, it only resets the flag for its own section. I did this for if I decide to add more stuff in the future).
(this forced 5 min wait between being able to reply sucks)

free-bee:
Exactly what it looks like. It takes a byte of data and stores the upper nibble as one variable and then the lower nibble as the next variable, then resets the "flag" variable.
(this forced 5 min wait between being able to reply sucks)

I can read C! I mean, why does it do that? What is the purpose, what useful work is it doing?

Also, you say it works flawlessly. Does that mean the problem is solved?

It takes a full byte of data and breaks it into two nibbles to use as the key in a lookup array containing bit patterns sent to the display. You did read the full code, yes? It should be fairly easy to follow along.

With THAT version, yes, but it's not the version giving me problems. It's not even the only code that has given me problems. Literally any time I try to use functions, they aren't performed.

/*
   Bubble Display Info

   Common Cathode

   1.6V - 2V drop
   5mA/segment
   680 Ohm Resistors

   01 - Cathode 1
   02 - E
   03 - C
   04 - Cathode 3
   05 - DP
   06 - Cathode 4
   07 - G
   08 - D
   09 - F
   10 - Cathode 2
   11 - B
   12 - A
*/

// Interface Pins
#define ADD 13 // Connect to A0 on address bus. 1 = left byte; 0 = right byte
#define D7 12 // The following pins connect to D7 - D0 on data bus
#define D6 11
#define D5 10
#define D4 9
#define D3 8
#define D2 7
#define D1 6
#define D0 5
#define SRCLK 4 // Shift register clock
#define SRDAT 3 // Shift register data
#define CLK 2 // Attach an interrupt to this pin. It comes from decoded IO and WR (already drives CLK of output latch). Rising edge
#define SROE 1 // Shift register /OE
#define SRLAT 0 // Shift register latch

// Common Cathode Conections
#define C1 A0
#define C2 A1
#define C3 A2
#define C4 A3

// Gen Rules
#define method LSBFIRST

volatile byte lb = 0x00; // LB = digit 1 and 2 data; written to if ADD = 1
volatile byte rb = 0x00; // RB = digit 3 and 4 data; written to if ADD = 0
volatile byte flags = 0b00000000;
/* byte used for flags
   b0 = "update lb"
   b1 = "update rb"
   b2 - b7 = unused
*/

byte char1 = 0x00; // Character 1
byte char2 = 0x00; // Character 2
byte char3 = 0x00; // Character 3
byte char4 = 0x00; // Character 4

// The following array is a lookup table
byte pattern[] = {0b11111100, 0b01100000, 0b11011010, 0b11110010,
                  0b01100110, 0b10110110, 0b10111110, 0b11100000,
                  0b11111110, 0b11110110, 0b11101110, 0b00111110,
                  0b10011100, 0b01111010, 0b10011110, 0b10001110};


void setup() {
 
Serial.begin(115200);
 // Set up digital IO
  for (int i = 13; i > 4; i--) {
    pinMode(i, INPUT);
  }

  // control lines
  digitalWrite(SRCLK, 0);
  digitalWrite(SRDAT, 0);
  digitalWrite(SROE, 1);
  digitalWrite(SRLAT, 0);
  pinMode(SRCLK, OUTPUT);
  pinMode(SRDAT, OUTPUT);
  pinMode(SROE, OUTPUT);
  pinMode(SRLAT, OUTPUT);

  // character lines
  digitalWrite(C1, 1);
  digitalWrite(C2, 1);
  digitalWrite(C3, 1);
  digitalWrite(C4, 1);
  pinMode(C1, OUTPUT);
  pinMode(C2, OUTPUT);
  pinMode(C3, OUTPUT);
  pinMode(C4, OUTPUT);
  attachInterrupt(digitalPinToInterrupt(CLK), ISR_Update_Val, RISING);
}

void loop() {
dudllySquat();
  // Put your main code here, to run repeatedly:
  if (flags == 0x01) {
    char1 = lb >> 4;
    char2 = lb & 0x0F;
    flags = flags & 0xFE;
  }
  if (flags == 0x02) {
    char3 = rb >> 4;
    char4 = rb & 0x0F;
    flags = flags & 0xFD;
  }

    // Write to the display
    // Digit 1
    shiftOut(SRDAT, SRCLK, method, pattern[char1]);
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C1, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char2]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C1, 1);

    // Digit 2
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C2, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char3]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C2, 1);

    // Digit 3
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C3, 0);
    shiftOut(SRDAT, SRCLK, method, pattern[char4]);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C3, 1);

    //Digit 4
    digitalWrite(SRLAT, 1);
    digitalWrite(SRLAT, 0);
    digitalWrite(SROE, 0); digitalWrite(C4, 0);
    delay(1);
    digitalWrite(SROE, 1); digitalWrite(C4, 1);
  }


  void ISR_Update_Val() {
    // Read data, update LB or RB, set flag
    // Timing isn't as critical in practice as it sounds in theory (for this application anyway)

    if (digitalRead(ADD) == 0) {
      bitWrite(rb, 0, digitalRead(D0));
      bitWrite(rb, 1, digitalRead(D1));
      bitWrite(rb, 2, digitalRead(D2));
      bitWrite(rb, 3, digitalRead(D3));
      bitWrite(rb, 4, digitalRead(D4));
      bitWrite(rb, 5, digitalRead(D5));
      bitWrite(rb, 6, digitalRead(D6));
      bitWrite(rb, 7, digitalRead(D7));
      flags = flags | 0x02;
    }

    else {
      bitWrite(lb, 0, digitalRead(D0));
      bitWrite(lb, 1, digitalRead(D1));
      bitWrite(lb, 2, digitalRead(D2));
      bitWrite(lb, 3, digitalRead(D3));
      bitWrite(lb, 4, digitalRead(D4));
      bitWrite(lb, 5, digitalRead(D5));
      bitWrite(lb, 6, digitalRead(D6));
      bitWrite(lb, 7, digitalRead(D7));
      flags = flags | 0x01;
    }
  }

void dudllySquat()
{
Serial.println("lfkgnrogneogngoefnvdnftgorengfojopreijgvw[peof");
}

Upload, open monitor set to 115200.
This does not work; as in things print on screen?

Are not setup() and loop() functions, do they work?

Idahowalker:

Upload, open monitor set to 115200.
This does not work; as in things print on screen?

Are not setup() and loop() functions, do they work?

I haven’t tried it yet, but I expected that to not compile because the compiler complained constantly at me when I tried to call functions like that. Something about it not being declared. I moved the functions around, putting it between setup() and loop() and I even put it before. I had () at the end of how I was trying to call it, much like you did, but it still complained. I deleted the () and it happily compiled…but didn’t run. But before I try your edit, I’ll need to free up the digital pins D0 and D1, which’ll be easy enough.

Setup and loop work fine. It’s my custom functions it’s not happy with.

the compiler complained constantly at me

A cut and paste of the actual compiler log would be more useful here.

C syntax demands parentheses after a function name when you call it.

void loop() {
  // Put your main code here, to run repeatedly:
  if (flags != 0x00) {
    flagCheck;
  }

  displayWrite;
}

must be

void loop() {
  // Put your main code here, to run repeatedly:
  if (flags != 0x00) {
    flagCheck();
  }

  displayWrite();
}

(soapbox) Your tekky coding style makes it hard for other people to read. It's fine as long as you can produce perfectly running code that nobody else will ever have to work on. Usually, such code is also hard for the coder to read when things get at all complicated.(/soapbox)

I set the boards manager to Uno, the code checks out for me. I'd run it but I do not have an Uno. Perhaps you needs be set the board manager to the proper board you are using, which is a?

"(this forced 5 min wait between being able to reply sucks)"

Yes, but we couldn't keep up with banning spammers and deleting their posts across multiple forums without it.

One thing apart from the many things I didn't like about Forth, that I did like, was the ability to call a function without any syntactic crapola around it, you could just say

flagCheck

So I sympathize. I believe the C syntax was designed to make it easy for the machine to parse (by being extremely literal and not expecting too many things to be decided by context).

aarg:
One thing apart from the many things I didn't like about Forth, that I did like, was the ability to call a function without any syntactic crapola around it, you could just say

Oi! I coded with Forth, good times.