Coding

I am having issues with my code. It is a Binary clock which displays Minutes, Hrs and if it is AM or PM. My current issue is that when it starts all the green LEDs are meant to be out for the first minute but the first LED turns on and indicates that the first minute has already finished.

// pins 3 - 13 are the regular digital pwm pins.
int ledPinsMin = { 3, 4, 5, 6, 7, 8};
int ledPinsHr = { 9, 10, 11, 12,};
int ledPinsPM = {13};

// set Start time here
int countM = 0; // Minutes
int countH = 0; // Hours
int countP = 0; // Day time

byte countMin;
byte countHr;
byte countPM;
#define nBitsMin sizeof(ledPinsMin)/sizeof(ledPinsMin[0])
#define nBitsHr sizeof(ledPinsHr)/sizeof(ledPinsHr[0])
#define nBitsPM sizeof(ledPinsPM)/sizeof(ledPinsPM[0])

void setup(void)
{
int i;
for (i = 0; i < nBitsMin; i++); {
pinMode(ledPinsMin*, OUTPUT);*

  • }*
  • for (i = 0; i < nBitsHr; i++) {*
    _ pinMode(ledPinsHr*, OUTPUT);_
    _
    }_
    _
    for (i = 0; i < nBitsPM; i++) {_
    _ pinMode(ledPinsPM, OUTPUT);
    }
    }
    void loop(void)
    {
    countM = (countM + 1);
    if (countM > 59)
    {
    countM = 0;
    countH = (countH + 1);
    if (countH > 12)
    {
    countH = 0;
    countP = (countP + 1);
    if (countP > 11)
    {
    countP = 0;
    countH = 0;
    countM = 0;
    }
    }
    }
    dispBinaryMin(countM);
    dispBinaryHr(countH);
    dispBinaryPM(countP);
    delay(60000); // Clock running speed.
    }
    void dispBinaryMin(byte nMin)
    {
    for (byte i = 0; i < nBitsMin; i++) {
    digitalWrite(ledPinsMin, nMin & 1);
    nMin /= 2;
    }
    }
    void dispBinaryHr(byte nHr)
    {
    for (byte i = 0; i < nBitsHr; i++) {
    digitalWrite(ledPinsHr, nHr & 1);
    nHr /= 2;
    }
    }
    void dispBinaryPM(byte nPM)
    {
    for (byte i = 0; i < nBitsPM; i++) {
    digitalWrite(ledPinsPM, nPM & 1);
    nPM /= 2;
    }
    }*_

Please Read this before posting a programming question and follow the instructions on posting code to get rid of teh italics in your post and make the code easier to copy for examination

This function

void dispBinaryMin(byte nMin)
{
  for (byte i = 0; i < nBitsMin; i++) {
    digitalWrite(ledPinsMin, nMin & 1);
    nMin /= 2;
  }
}

always writes to the memory address of ledPinsMin. You should be specifying the pin number which is contained withint ledPinsMin*.*
Same for your other functions.

When you create countM, you set it to zero. In the loop() you start by incrementing countM before waiting a minute.

The delay(60000); can be moved to the top of the loop().
Or the countM can be incremented at the bottom of the loop(), after the delay.

To increment a value, it is possible to use ‘++’

// countM = (countM + 1);   // increment countM
countM++;  // increment countM

Is the ‘countP’ either 0 or 1 ?
Then the ‘11’ should be a ‘1’ ?

// if (countP > 11) // bug
if (countP > 1)  // am or pm

When using a value of an array, use an index.

pinMode(ledPinsMin[i], OUTPUT);
digitalWrite(ledPinsMin[i], nMin & 1);

The digitalWrite() is expecting a HIGH or LOW. The “nMin & 1” is either 0 or 1, that is not the same. I prefer to use only HIGH or LOW.

The variables ‘countMin’, ‘countHr’, ‘CountPM’ are not used. That is confusing.

The delay(60000) is not be accurate.
When using millis(), it will be accurate. Then it will be just as accurate as the 16MHz resonator or crystal.
The official Arduino Uno with 16MHz has a resonator, which is not accurate.
Some clone Uno’s have a crystal, that is accurate.
The Arduino Leonardo has a crystal, that is also accurate.

You can still use “Modify” to change your post and put the sketch between code tags. The code tags is the ‘< / >’ button in the upper-left corner above the text field.

blh64:
This function

void dispBinaryMin(byte nMin)

{
 for (byte i = 0; i < nBitsMin; i++) {
   digitalWrite(ledPinsMin, nMin & 1);
   nMin /= 2;
 }
}



always writes to the memory address of ledPinsMin. You should be specifying the pin number which is contained withint ledPinsMin*.*
*Same for your other functions.*
*[/quote]*
*See reply #1*
*Using ­[­i] as an array index strikes again when posting not using code tags*

countH goes from 0 to 12, and gets reset to 0 again once it reaches 13. That’s 13 values, so you have 26 hours in a day. If you are doing a 12 hour clock with am/pm, it needs to go from 0 to 11, and then reset back to zero at 12. When you display hour “0”, that zero it needs to be translated into a value of twelve.

Also I find it easier to keep track of time in total minutes or seconds. It saves all the incrementing, rollover, wrapping logic everywhere. Then divide down to get am/pm and hours for display. That only needs to be in one place, so it makes life easier.

It also means doing things like alarms is dead easy, as you can just write...

if (current_time == alarm_time) {

Instead of having to do multiple checks...

if (ampm == alarm_ampm && hrs == alarm_hrs && mins == alarm_mins && secs == alarm_secs) {

Your use of delay is going to result in a very inaccurate clock because you aren’t taking into account execution time of the rest of the code. You should check out how to use millis() function to get correct delays.