Cannot blink pins 7 through 5 (NEWBIE)

HazardsMind:
Try analogWrite, you might have burned out your arduino.

I'll give that a shot. All the other pins continue to blink fine.

PeterH:
I haven't looked into the logic you're using to control the writes, but at first glance it's complex enough that I'm not certain whether you are actually doing a digitalWrite(HIGH) on the relevant pins. Write a sketch that does nothing but set the mode to output and set the pins HIGH, disconnect all the external wiring and just connect one LED with a current limiting resistor to each output pin in turn, with the other leg of the LED grounded.

All the pins work except for pins 5 through 7. It should do a digitalWrite(PIN#, HIGH). Its not that big. It may be easier to read on the github page.

I did try to blink those pins using the example file Blink.ino though I did not disconnect everything so perhaps I will try that.

Alright, there are numerous problems with your code.

  1. You have absolutely no code driving the Hours pins (5,6,7). Looks like they are commented out.

  2. In your loop() you have two different functions. Although they are both called blink_binary_number, they are overloaded.

  3. do not assign a variable in your parameters list of a method, whether it works or not, its a convention. You just don't do that.

  4. Your digital write is funky, when you call your method, you are sending a LOW with it. The problem is, in your blink_binary_number method, it doesn't do anything unless the pin is already HIGH. So even if you sent the method a HIGH, if the pin wasn't already HIGH, it wouldn't do a single thing.

Your blink_binary_number method is underdeveloped. You cannot do what you are trying to do with just four lines of code.

crosson:
All the pins work except for pins 5 through 7. It should do a digitalWrite(PIN#, HIGH). Its not that big. It may be easier to read on the github page.

It's not a question of size. You are assuming that your sketch does what you want. I'm saying don't make that assumption; write a sketch that just turns that output HIGH in the simplest possible way. You don't need github or anything else to write that. Just set the pin mode and set the pin high. Eliminate all the superfluous hardware and just test one specific output with your LED and current limiting resistor. Don't allow any other complexity at all into your test system.

What is "boolean vol=HIGH" doing?

HazardsMind:
Try analogWrite, you might have burned out your arduino.

I plan on trying this tonight but it seems weird to me that I would need to do this. Aren't pins 5-7 digital?

HazardsMind:
What is "boolean vol=HIGH" doing?

Perhaps I am not doing this in the most appropriate way. To blink my leds I simply copied the methods demonstrated in the blink tutorial file, blink.ino. This can be loaded from within the Arduino IDE from File -> Examples -> Basics.

The method used is digitalwrite. My function just assumes a default variable of const HIGH.

In retrospect I can see how it degrades the readability of the program. I don't gain anything by having the default value here. I will probably remove this. Actually in thinking about it and based on some other points here I'll rewrite blink_binary_number to address all leds. I will turn off LEDS that should be off and turn on leds that should be on. In this way I can call the function once per loop rather than twice(Once to turn off and again to turn off).

Does that make sense?

Back to the problem discovered in the OP I can use the blink sketch to blink pins 5 through 7. This arduino board is brand new. Is it possible I did something wrong?

Pins 5 & 6 are PWM so they will work with analogWrite. 7 is digital. I found out that I burnt out my Nano, and my pins would only go high when I did analogWrite(pin, 255) and not digitalWrite(pin,HIGH).

funkyguy4000:
Alright, there are numerous problems with your code.
2) In your loop() you have two different functions. Although they are both called blink_binary_number, they are overloaded.

I don't follow. My blink_binary_number is defined once. It is not overloaded. I do call the function several times however.

funkyguy4000:
3) do not assign a variable in your parameters list of a method, whether it works or not, its a convention. You just don't do that.

So defaults, by convention, are generally frowned on? I did try to search for Arduino sketch conventions but didn't pull anything related to it. I think in this case I am going to remove the default assignment since it does not add to the program in anyway and takes away the readability. Though based on your comment it is not clear why the Arduino community adopts avoiding defaults by convention.

These points posted do not address why pins 5 through 7 won't blink.

funkyguy4000:
4) Your digital write is funky, when you call your method, you are sending a LOW with it. The problem is, in your blink_binary_number method, it doesn't do anything unless the pin is already HIGH. So even if you sent the method a HIGH, if the pin wasn't already HIGH, it wouldn't do a single thing.

I don't follow. The method as is currently does not behave this way. Keep in mind I have spent all of about 2 hours with Arduino sketching so it is possible I am not understanding it but I don't think that method is behaving as your describe above.

For my first hello world sketch I used this method to create a binary counter that would count up to 31(I only had 5 pins at the time). Unfortunately I did not use pins 5-7 so I don't know if I would have the same problem or not.

Anyhow the blink_binary_number worked as intended. I got some fantastic feedback from Nick Gammon when setting it up. Here is a video of the counter in action. Here is the actual sketch itself. So we know the function is working to an extent.

funkyguy4000:
Your blink_binary_number method is underdeveloped.

Yes I think you are right. Is it wise to send a LOW to pins that are already turned off? Is there any harm in this? If not I could send LOW to each PIN who represents the binary 0 in the binary array and I could send HIGH to each pin that represents the binary 1. When the number changes the function will just turn off the pins that need to be off and turn on the pins that need to be on. I would only need to call the function once per loop.

funkyguy4000:
You cannot do what you are trying to do with just four lines of code.

What am I trying to do with just 4 lines of code? Do tell. :slight_smile:

HazardsMind:
Pins 5 & 6 are PWM so they will work with analogWrite. 7 is digital. I found out that I burnt out my Nano, and my pins would only go high when I did analogWrite(pin, 255) and not digitalWrite(pin,HIGH).

Ok I will definitely try this tonight. I am beginning to wonder if I did this. Fortunately I do have another uno I can test on as well. Thanks HazardsMind I'll keep you posted.

PeterH:
You are assuming that your sketch does what you want. I'm saying don't make that assumption;

Well presently I am not sure what is going on. Hence the post here. I suspected something wrong with my sketch.

PeterH:
write a sketch that just turns that output HIGH in the simplest possible way.

Right. Which is why I tried using the blink example sketch. You may have missed this but the second post mentions that I tried this.

PeterH:
You don't need github or anything else to write that.

I don't follow. Github isn't used for writing sketches.

PeterH:
Eliminate all the superfluous hardware and just test one specific output with your LED and current limiting resistor. Don't allow any other complexity at all into your test system.

This is a good point. I plan on trying this and Hazards notes tonight when I get a chance. Thanks for the feedback Peter.

Okay well to really help you, because your code is atrocious and we still haven't gotten an update, tell us what your project is.

Are you just trying to make a binary clock? Because there is a plethora of code for that out there already.

HazardsMind:
...

PeterH:
...

I am troubleshooting this now. I got pins 5-7 blink after disconnecting everything. I'll keep you guys posted. Thanks again for the help.

I got all 3 pin, 5-7, to blink individually. I think I made some wiring mistakes. Apparently some of my LEDS are 3.0-3.2V and they didn't work on the digital pins. I'll continue in the morning with more information. Thanks guys.

I've made some wonderful progress. I have all my pins blinking and my binary counting for the hour leds and minute leds are working great. I've cleaned up my blink_to_binary function a bit so that it only needs to be called once per array of pins.

#define NUM_ITEMS(arg) ((unsigned int) (sizeof (arg) / sizeof (arg [0])))

int const MINUTES[] = {13, 12, 11, 10, 9, 8};
int const HOURS[] = {7, 6, 5, 4};
int const delayms = 500;
int hours = 1;

void setup() {                
  for(int led = 4; led < 14; led++){
    pinMode(led, OUTPUT);
  }
  Serial.begin(9600);
}

int array_size(int const a[]) {
  return (sizeof(a)/sizeof(int));  
}

void blink_binary_number(int number, int const pin_array[], int a_size){
  for(int led = 0; led < a_size; led++){
    if (bitRead(number, led) == 1){
      digitalWrite(pin_array[led], HIGH);
    } else {
     digitalWrite(pin_array[led], LOW); 
    }
  }
}

void loop() {
  for(int x = 0; x < 60; x++){
   
    Serial.print("Minute: ");
    Serial.println(x);
    Serial.print("Hour: ");
    Serial.println(hours);
    Serial.println("------");   
   
    blink_binary_number(hours, HOURS, 4);
    blink_binary_number(x, MINUTES, 6);
   
    if (x == 59) {
      hours++;  
    }
   
    if (hours > 12) {
      hours = 0; 
    }
    delay(delayms);
  }
}

I did run into a snag with catching array size. I wanted my bit_to_binary function to be able to handle any array size. I also don't want it trying to access the next element in an array if it doesn't exist so I only want to count up to the array size.

I made two array_size functions to accomplish this. NUM_ITEMS and array_size. If I run these inside of my blink_binary_number the array size is always one.

I want to do something like this

void blink_binary_number(int number, int const pin_array[]){
  a_size = array_size(pin_array);

  for(int led = 0; led < a_size; led++){
    if (bitRead(number, led) == 1){
      digitalWrite(pin_array[led], HIGH);
    } else {
     digitalWrite(pin_array[led], LOW); 
    }
  }
}
void loop() {
  blink_binary_number(number, HOURS_ARRAY);
}

However every time I run this method inside of the blink_binary_number HOURS_ARRAY always returns 1. If I run it from within void loop it correctly returns a length of 4. For now I just pass the array size into the function but it would be nice if the function could just handle this for me.

I know this is probably due to a lack of understanding in what is going with hour the array is referenced. Can someone help me understand this?

I'll send pictures and perhaps a video of what I have so far in a bit.

What does a_size return?

You're running into the difference between an array and a pointer.

When you deal directly with the array variable, the compiler knows its size.

When you are only dealing with a reference to the array, the compiler knows the element type and the address of the first element but does not know how long the array is. Hence for all practical purposes the value you have is just a pointer to the first element in the array, and it is up to you to provide some other way to know how long the array is if your code requires that. For example, it's common to pass an array around as a pointer and a length.

That makes perfect sense. It has been a very long time since I've coded in c++ but some of this is coming back. Do you think the way I have it now is acceptable?

Alright well I have a working clock. In my excitement to get it all working I may have mangled my code a bit. I had to add a crude setup process so you could set the right time. Still it is wonderful that I got it up and running.

Now I can refine it and improve it. Since this is the first real thing I've made with Arduino it has spiked a load of questions regarding electronics. Some of resistors I used were guess work. I'll probably post some followup questions in the electronics forum.

Below is what I have so far in my code.

//#define NUM_ITEMS(arg) ((unsigned int) (sizeof (arg) / sizeof (arg [0])))

int const MINUTES[] = {13, 12, 11, 10, 9, 8};
int const HOURS[] = {7, 6, 5, 4};
int hours = 1;
int minute = 0;
int setup_time = 10000;

void setup() {                
  for(int led = 4; led < 14; led++){
    pinMode(led, OUTPUT);
  }
  //Serial.begin(9600);
  pinMode(3, INPUT);
}

//int array_size(int const a[]) {
//  return (sizeof(a)/sizeof(int));  
//}

void blink_binary_number(int number, int const pin_array[], int a_size){
  for(int led = 0; led < a_size; led++){
    if (bitRead(number, led) == 1){
      digitalWrite(pin_array[led], HIGH);
    } else {
     digitalWrite(pin_array[led], LOW); 
    }
  }
}

void loop() {
  while (setup_time > 0) {
    if (digitalRead(3) == HIGH) {
//      Serial.print("Minute: ");
//      Serial.println(minute);
//      Serial.print("Hour: ");
//      Serial.println(hours);
//      Serial.println("------");        
      blink_binary_number(hours, HOURS, 4);
      blink_binary_number(minute, MINUTES, 6);

      //Incriment time   
      minute++;
      if (minute == 60) {
        minute = 0;
        hours++;  
      }
     
      if (hours > 12) {
        hours = 1; 
      }
      delay(150);
      setup_time = 10000;
    } else {
     delay(10); 
     setup_time = setup_time - 10;
    }
  }
  
  
//  Serial.print("Minute: ");
//  Serial.println(minute);
//  Serial.print("Hour: ");
//  Serial.println(hours);
//  Serial.println("------");   
 
  blink_binary_number(hours, HOURS, 4);
  blink_binary_number(minute, MINUTES, 6);

 //Incriment time   
  minute++;
  if (minute == 60) {
    minute = 0;
    hours++;  
  }
 
  if (hours > 12) {
    hours = 1; 
  }
  delay(60000);
}

Uploading a video now.

Video of the clock.