1

This is my first manually typed code. I'm sure there may be some glaring "schoolboy errors" in it, so wondered if people could give it the 'once over' In particular, I have never used C++ arrays, storing binary data within, and then trying to read them bit by bit. In perl, much easier; you would simply use a multi-dimensional something like this:

@code1=('10101010', '01010101');
$bit=$code1[0][0] (..through to..) code1[0][7]
$bit=$code1[1][0] ... code1[1][7]

So this is my Arduino code. Welcome to suggestions.

int pinArray[]={2,3,4,5,6,7,8,9};
int code1[]={B10101010, B01010101,B10101010, B01010101,B10101010, B01010101};
int count=0;
int split=0;    // Counter for the bit loop
int led;    // Store bit data
byte x;     // Used to store the binary data in code1

void setup(){
    for (count=0; count<8; count++){
    pinMode(pinArray[count],OUTPUT); // Sets pinMode as output on pinArray pins
    ]
}

void loop(){
    for (count=0; count<6; count++){
    x=code1[count];
    ledDisplay(x);
    }
}

void ledDisplay(){
    for (split=7; split>=0; split--){
    led=bitRead(x,split);
        if (led == 1){
        digitalWrite(split+2,HIGH);
        }
        else{
        digitalWrite(split+2,LOW);
        }
    }
}

Have just modified split as realised bit7 is the leftmost digit. In split, I am adding 2 to the value. Does this need to be enclosed in brackets,ie (split+2) ?

  • Your indents are confusing. Press Ctrl+T – Jot Jun 18 '19 at 10:01
  • When I write my code in text editor, I use the tab key to indent code, so that the "for" opening / closing brackets etc are in the same column, (which seems to view correctly in my desktop browser - maybe perhaps incorrect on phone - so will use CRTL+t in future when posting. (I know CODE1 doesn't follow the same convention, but then I try to keep open/close on same line so it doesn't 'run into' the next array, bearing in mind I will have 14 such arrays) This isn't meant to be 'nasty' to you as I do appreciate your taking the time to be helpful, just showing my personal 'naming convention' – Cristofayre Jun 18 '19 at 17:03
  • 1
    Ctrl-T is just a quick way to get the code correctly indented in whatever is the IDE's default style. You don't have to use it though. You can use whatever indentation style you like if you handle it manually. Alternatively, you can configure the IDE for your favorite style. Just make sure to adopt one indent style and apply it consistently. – Edgar Bonet Jun 18 '19 at 19:06
2

There are only two errors I can spot in your program:

  1. A block of code should be delimited by curly braces:

    for (count=0; count<8; count++){
        pinMode(pinArray[count],OUTPUT);
    }  // ← ‘}’ instead of ‘]’
    
  2. If a function expects to be passed arguments, these should be declared as parameters in the function prototype:

    void ledDisplay(byte x){  // ← mind the “byte x”
        // ...
    }
    

Other than that, here a few suggestions to make the code look nicer, and hopefully more maintainable:

  • indent consistently
  • use const to qualify constants
  • use named constants instead of magic numbers
  • keep variables local if possible
  • note that an int that is either 0 or 1 is equivalent to a boolean
  • use pinArray[] for accessing the pins

Applying those suggestions, I get the following:

const int PIN_COUNT = 8;
const int pinArray[PIN_COUNT] = { 2, 3, 4, 5, 6, 7, 8, 9 };
const int CODE_COUNT = 6;
const int code1[CODE_COUNT] = {
    B10101010, B01010101, B10101010, B01010101, B10101010, B01010101
};

void setup() {
    for (int count = 0; count < PIN_COUNT; count++) {
        pinMode(pinArray[count], OUTPUT);
    }
}

void loop() {
    for (int count = 0; count < CODE_COUNT; count++) {
        ledDisplay(code1[count]);
    }
}

void ledDisplay(byte x) {
    for (int split = 7; split >= 0; split--) {
        if (bitRead(x, split)) {
            digitalWrite(pinArray[split], HIGH);
        } else {
            digitalWrite(pinArray[split], LOW);
        }
    }
}
| improve this answer | |
  • For once, I honestly wish I had the ability to upvote a reply. You have given me very constructive advice, rather than just a pointer here and there. (The "]" was just a typo,but thanks for pointing it out) I like too the boolean comment,which will save an extra few bytes. I shall now research "const" to see what that does exactly ... Again, I honestly am very grateful for your indepth feedback – Cristofayre Jun 18 '19 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.