Critique my code

Projects General Guidance
1

Hi. Let me preface that I am still learning how to code the Arduino platform. Someday this project will turn into a nixie clock once I truly learn how my shift register works, learn to code properly and learn to use SPI. My plan is to get the clock digit to display on these nixie tubes using an array. Right now I am driving three nixie tubes off of one HV5622 shift register. It's working as I wanted it to scrolling through each digit of the three nixie tubes. I looked around on how best to use hex and fell flat on my research. So instead I just reckoned what the 2^n power was then converted to hex using a hex calculator. I thought such large numbers would definitely include alpha characters. To my surprise the conversion each time yielded a number format.

I'd like to be able to make the array more readable if someone was perusing the code. I intend on posting it up someday to share with the community. Pointers, links, guidance and constructive criticism appreciated.

#define NUM_LEADS    31
int clockPin = 0;
int latchPin = 1;
int dataPin = 2;
long sequence[31] = {
//[ 1(2^0), 2(2^1), 3(2^2), 4(2^3), 5(2^4), 6(2^5), 7(2^6), 8(2^7), 9(2^8), 0(2^9)]
     0x1,    0x2,    0x4,    0x8,   0x10,   0x20,   0x40,   0x80,   0x100,   0x200,
//[  2^10 ,  2^11 ,  2^12  ,  2^13  ,  2^14 ,   2^15 ,   2^16  ,   2^17  ,   2^18  ,   2^19 ]
     0x400,  0x800,  0x1000,  0x2000, 0x4000,  0x8000,  0x10000,  0x20000,  0x40000, 0x80000,
     0x100000,   0x200000,  0x400000,  0x800000,  0x1000000,  0x2000000,  0x4000000, 0x8000000, 0x10000000, 0x20000000,
     0x40000000};


void setup() {
  pinMode(clockPin, OUTPUT);
  pinMode(latchPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
}

void loop() {
  for (int i = 0; i < NUM_LEADS; i++)
  {
    digitalWrite(latchPin, LOW);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence[i] >> 24);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence[i] >> 16);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence[i] >> 8);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence[i]);
    digitalWrite(latchPin, HIGH);
    delay(80);
  }
}
2

The ^ operator does not raise a number to a power in C, rather it does a bitwise exclusive or of the two values

Can you give examples of the problems that had with Hex values ?

3 
long sequence[31] = {
//[ 1(2^0), 2(2^1), 3(2^2), 4(2^3), 5(2^4), 6(2^5), 7(2^6), 8(2^7), 9(2^8), 0(2^9)]
     0x1,    0x2,    0x4,    0x8,   0x10,   0x20,   0x40,   0x80,   0x100,   0x200,
//[  2^10 ,  2^11 ,  2^12  ,  2^13  ,  2^14 ,   2^15 ,   2^16  ,   2^17  ,   2^18  ,   2^19 ]
     0x400,  0x800,  0x1000,  0x2000, 0x4000,  0x8000,  0x10000,  0x20000,  0x40000, 0x80000,
     0x100000,   0x200000,  0x400000,  0x800000,  0x1000000,  0x2000000,  0x4000000, 0x8000000, 0x10000000, 0x20000000,
     0x40000000};

The problem is I don't understand what that is meant to be. You seem to have put some array elements inside [], some not inside any kind of bracket and some correctly in {}

Do you mean:
long sequence[31] = {0x400,  0x800,  0x1000,  0x2000, 0x4000,  0x8000,  0x10000,  0x20000,  0x40000, 0x80000, 0x100000,  0x200000,  0x400000,  0x800000,  0x1000000,  0x2000000,  0x4000000, 0x8000000, 0x10000000, 0x20000000, 0x40000000};?
I find that perfectly readable.

I thought such large numbers would definitely include alpha characters. To my surprise the conversion each time yielded a number format.

Why when you only set 1 bit in each nibble?

4

PerryBebbington:

long sequence[31] = {

//[ 1(2^0), 2(2^1), 3(2^2), 4(2^3), 5(2^4), 6(2^5), 7(2^6), 8(2^7), 9(2^8), 0(2^9)]
0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80, 0x100, 0x200,
//[ 2^10 , 2^11 , 2^12 , 2^13 , 2^14 , 2^15 , 2^16 , 2^17 , 2^18 , 2^19 ]
0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, 0x40000, 0x80000,
0x100000, 0x200000, 0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000, 0x8000000, 0x10000000, 0x20000000,
0x40000000};



The problem is I don't understand what that is meant to be. You seem to have put some array elements inside []

They're comments

5

TheMemberFormerlyKnownAsAWOL:
They're comments

Ah! Of course they are!
Ok, so I find it easier to read without the comments.

6

Why are you using 31 outputs, when 3 nixie tubes would have 30 leads?

In this case, the array is totally unnecessary, and isn't needed for converting a number to the correct pattern for a nixie tube output either, provided you have the nixie digits wired sequentially on the shift register.

This should produce the same output as your code, without the array.

void loop() {
  for (int i = 0; i < NUM_LEADS; i++)
  {
    long sequence = 1L << i; //shifts a 1 into the bit position corresponding to the value of i
    digitalWrite(latchPin, LOW);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 24);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 16);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 8);
    shiftOut(dataPin, clockPin, MSBFIRST, sequence);
    digitalWrite(latchPin, HIGH);
    delay(80);
  }
}
7

So I should have said you mean this:

long sequence[31] = {0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80, 0x100, 0x200, 0x400, 0x800, 0x1000, 0x2000, 0x4000, 0x8000, 0x10000, 0x20000, 0x40000, 0x80000, 0x100000, 0x200000,  0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000, 0x8000000, 0x10000000, 0x20000000, 0x40000000};

Yes?
Clear to me without comments. The comments confused me and seemed to be a complicated way of saying something simple.

8

UKHeliBob:
The ^ operator does not raise a number to a power in C, rather it does a bitwise exclusive or of the two values

Can you give examples of the problems that had with Hex values ?

I am not using the ^ operator, that is commented out because that is personal notes. I was equating the number 1 on nixie tube 1 to what the binary number is ie. Nixie digit 1 is B00000001, or 2^0. So thanks for the insight that makes it confusing. I'll need to add notes to signify that in a future sketch.

PerryBebbington:

long sequence[31] = {

//[ 1(2^0), 2(2^1), 3(2^2), 4(2^3), 5(2^4), 6(2^5), 7(2^6), 8(2^7), 9(2^8), 0(2^9)]
    0x1,    0x2,    0x4,    0x8,  0x10,  0x20,  0x40,  0x80,  0x100,  0x200,
//[  2^10 ,  2^11 ,  2^12  ,  2^13  ,  2^14 ,  2^15 ,  2^16  ,  2^17  ,  2^18  ,  2^19 ]
    0x400,  0x800,  0x1000,  0x2000, 0x4000,  0x8000,  0x10000,  0x20000,  0x40000, 0x80000,
    0x100000,  0x200000,  0x400000,  0x800000,  0x1000000,  0x2000000,  0x4000000, 0x8000000, 0x10000000, 0x20000000,
    0x40000000};



The problem is I don't understand what that is meant to be. You seem to have put some array elements inside [], some not inside any kind of bracket and some correctly in {}

Do you mean:

long sequence[31] = {0x400,  0x800,  0x1000,  0x2000, 0x4000,  0x8000,  0x10000,  0x20000,  0x40000, 0x80000, 0x100000,  0x200000,  0x400000,  0x800000,  0x1000000,  0x2000000,  0x4000000, 0x8000000, 0x10000000, 0x20000000, 0x40000000};



?
I find that perfectly readable.
Why when you only set 1 bit in each nibble?

The same response as to Helibob to your question about the [], these were personal notes. I will clarify in my future sketch or get rid of them altogether. I was making myself a commented out table.

Now that makes sense about the nibble. I was hoping to make the array more concise (not terse) perhaps it is what it is.

Is it possible to write hex value 0x40000000 in a shorter form that I am missing? Say octal or binary format?

9

The same response as to Helibob to your question about the [], these were personal notes. I will clarify in my future sketch or get rid of them altogether. I was making myself a commented out table.

I understand that since AWOL pointed out what should have been obvious to me all along. My preference is for comments in code to be after the ; at the end of the line, not on a separate line. I stress my preference, what your preference is is up to you.

Is it possible to write hex value 0x40000000 in a shorter form that I am missing? Say octal or binary format?

I feel that you can work that out for yourself, try it and see.

10

david_2018:
Why are you using 31 outputs, when 3 nixie tubes would have 30 leads?

In this case, the array is totally unnecessary, and isn't needed for converting a number to the correct pattern for a nixie tube output either, provided you have the nixie digits wired sequentially on the shift register.

This should produce the same output as your code, without the array.

void loop() {

for (int i = 0; i < NUM_LEADS; i++)
 {
   long sequence = 1L << i; //shifts a 1 into the bit position corresponding to the value of i
   digitalWrite(latchPin, LOW);
   shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 24);
   shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 16);
   shiftOut(dataPin, clockPin, MSBFIRST, sequence >> 8);
   shiftOut(dataPin, clockPin, MSBFIRST, sequence);
   digitalWrite(latchPin, HIGH);
   delay(80);
 }
}

I am using 31 in this as when I would set it to 30, I'd lose the last zero on the last nixie tube.

I have written a sketch that does what you provided and it works great too. Your code is definitely short and to the point. Far more so than mine. I am going to analyze your code and learn from it.
The reason for the array is when I want the Arduino to call a single digit from each nixie tube as it's running as a clock. Again this sketch is me learning how to program. By playing with an array, playing with shiftOuts only, and playing around by moving the one through the register is how I am coming to learn how these things work. I am forming new neural pathways I do hope by struggling through why things don't work, and when they do I certainly feel a sense of victory.