Inputs are confusing :p

hi again,
I’ve been working on a binary clock and with a little help from people on here i was able to fix and improve my code for it! The final part for the little clock is adding time setting buttons; i have added 3 buttons for this - 2 toggles and 1 PTM. Now… I have set up 3 functions for this and they all link together so, logically i can’t see the problem and ‘grammatically’ the IDE cant find anything? I have used the seconds blipper as an indicator to show when it actually starts running the settings functions but it isnt working at all, so I’m thinking there is a problem with my initial switch readings and using pull ups? :confused: I’m getting there and its frustrating i need to ask for help haha but its all the fun of learning isn’t it! i was so close… darn! In other news through, it eventually got the clock itself working alright through :smiley: whoop! Thanks!

//Potential issue with counting solved (hours - by removing the loop to reduce the amount of hour runs done - so far so good)
//Version with time setting buttons ([toggle switch]SW1, [Toggle Switch]SW2, [PTM Switch]SW3)
  byte minOne = 4;
  byte minTwo = 5;
  byte minFour = 6;
  byte minEight = 7;
  byte minSixteen = 8;
  byte minThirtyTwo = 9;

  byte hourOne = 0;
  byte hourTwo = 1;
  byte hourFour = 2;
  byte hourEight = 3;

  byte AMLED = 10;
  byte PMLED = 11;
  byte sec = 12;
  //counters 
  byte secCount = 0;
  byte minCount = 0;
  byte hourCount = 0;
  byte AM = 1;
  byte PM = 0;

  byte SW1 = 14;
  byte SW2 = 15;
  byte SW3 = 18;

//C cant return 2 varibales from a function, so i cant return 2 from the settings function (hours AND minutes -_-)
  byte privMin = 0;
  byte privHour = 0;

void setup() {
  pinMode(minOne, OUTPUT);
  pinMode(minTwo, OUTPUT);
  pinMode(minFour, OUTPUT);
  pinMode(minEight, OUTPUT);
  pinMode(minSixteen, OUTPUT);
  pinMode(minThirtyTwo, OUTPUT);

  pinMode(hourOne, OUTPUT);
  pinMode(hourTwo, OUTPUT);
  pinMode(hourFour, OUTPUT);
  pinMode(hourEight, OUTPUT);

  pinMode(AMLED, OUTPUT);
  pinMode(PMLED, OUTPUT);
  pinMode(sec, OUTPUT);
//inputs being defined here
  pinMode(SW1, INPUT);
  pinMode(SW2, INPUT);
  pinMode(SW3, INPUT);
  //using pull up resisotrs -> had to invert it to mean when the switch is pressed, it becomes low so its not bouncing or doing what it wants
  digitalWrite(SW1, HIGH);
  digitalWrite(SW2, HIGH);
  digitalWrite(SW3, HIGH);
  
}

void loop() {
   // int tempHour;           //may be a cause of error if it doesnt work. temphour is to reduce the amount of times that the hour loop is done; only if the hourCount changes
    byte set = digitalRead(SW1);
    if (set == LOW)
    {
      settings(minCount, hourCount);
    }
    minCount = privMin;
    hourCount = privHour;
    minutes(minCount);
    hours(hourCount); 
    if (AM == 1)
    {
      digitalWrite(AMLED, HIGH);
      digitalWrite(PMLED, LOW);
    }
    if (PM == 1)
    {
      digitalWrite(PMLED, HIGH);
      digitalWrite(AMLED, LOW);
    }
    delay(800);
    digitalWrite(sec, HIGH);
    delay(199); //allowing 1ms for the rest of the loop to run
    digitalWrite(sec, LOW);
    secCount = secCount + 1;
    if (secCount == 60)
    {
      secCount = 0;
      minCount = minCount + 1;                      //concern expressed about the nested ifs. will this work? do they need to be after one another?
      if (minCount == 60)
      {
        minCount = 0;
        hourCount = hourCount + 1;
        if (hourCount == 12)
        {
          hourCount = 0;
          if (AM == 1)
          {
            AM = 0;
            PM = 1;
//            digitalWrite(AMLED, LOW);
//            digitalWrite(PMLED, HIGH);
          }
          else
          {
            AM = 1;
            PM = 0;
//            digitalWrite(PMLED, LOW);
//            digitalWrite(AMLED, HIGH);
          }
        }
        
      }
      
    
  }

}

void minutes(int count)
{
  digitalWrite(minOne, LOW);
  digitalWrite(minTwo, LOW);
  digitalWrite(minFour, LOW);
  digitalWrite(minEight, LOW);
  digitalWrite(minSixteen, LOW);
  digitalWrite(minThirtyTwo, LOW);
  
  if (count >= 32)
  {
    count = count - 32;
    digitalWrite(minThirtyTwo, HIGH);
  }
  if (count >= 16)
  {
    count = count - 16;
    digitalWrite(minSixteen, HIGH);
  }
  if (count >= 8)
  {
    count = count - 8;
    digitalWrite(minEight, HIGH);
  }
  if (count >= 4)
  {
    count = count - 4;
    digitalWrite(minFour, HIGH);
  }
  if (count >= 2)
  {
    count = count - 2;
    digitalWrite(minTwo, HIGH);
  }
  if (count == 1)
  {
    count = count - 1;
    digitalWrite(minOne, HIGH);
  }
  if (count != 0)
  {
    //error - outputs 63
    digitalWrite(minOne, HIGH);
    digitalWrite(minTwo, HIGH);
    digitalWrite(minFour, HIGH);
    digitalWrite(minEight, HIGH);
    digitalWrite(minSixteen, HIGH);
    digitalWrite(minThirtyTwo, HIGH);
  } 
  return;
}

void hours(int count)
{
  digitalWrite(hourOne, LOW);
  digitalWrite(hourTwo, LOW);
  digitalWrite(hourFour, LOW);
  digitalWrite(hourEight, LOW);
  
  if (count >= 8)
  {
    count = count - 8;
    digitalWrite(hourEight, HIGH);
  }
  if (count >= 4)
  {
    count = count - 4;
    digitalWrite(hourFour, HIGH);
  }
  if (count >= 2)
  {
    count = count - 2;
    digitalWrite(hourTwo, HIGH);
  }
  if (count == 1)
  {
    count = count - 1;
    digitalWrite(hourOne, HIGH);
  }
  if (count != 0)
  {
    //error - outputs 15
    digitalWrite(hourOne, HIGH);
    digitalWrite(hourTwo, HIGH);
    digitalWrite(hourFour, HIGH);
    digitalWrite(hourEight, HIGH);
  }
  return;
}

void settings(byte mins, byte hours)
{
  byte switchOne = digitalRead(SW1);
  digitalWrite(sec, LOW); //does this so indicate its in settings mode
  if (switchOne == HIGH)
  {
    privMin = mins;
    privHour = hours;
    digitalWrite(sec, HIGH);
    //return(mins, hours); //doesnt work like this - C doesnt do this!
    // got it so that it uses 2 global variables... not great but a work around.
  }
  byte switchTwo = digitalRead(SW2);
  if (switchTwo == LOW)
  {
    mins = setMin(mins);
  }
  else if (switchTwo == HIGH)
  {
    hours = setHour(hours);
  }
}


byte setMin(byte mins)
{
  byte switchOne = digitalRead(SW1);
  byte switchTwo = digitalRead(SW2);
  if (switchOne == HIGH || switchTwo == HIGH)  // || is or
  {
    return(mins);
  }

  byte switchThree = digitalRead(SW3);
  if (switchThree = LOW)
  {
    mins = mins + 1;
    delay(500);
  }
}

byte setHour(byte hours)
{
  byte switchOne = digitalRead(SW1);
  byte switchTwo = digitalRead(SW2);
  if (switchOne == HIGH || switchTwo == LOW)
  {
    return(hours);
  }

  byte switchThree = digitalRead(SW3);
  if (switchThree = LOW)
  {
    hours = hours + 1;
    delay(500);
  }
}

infact, the new code doesn't work as a clock anymore!! :( This was the working one (but without settings)

  byte minOne = 4;
  byte minTwo = 5;
  byte minFour = 6;
  byte minEight = 7;
  byte minSixteen = 8;
  byte minThirtyTwo = 9;

  byte hourOne = 0;
  byte hourTwo = 1;
  byte hourFour = 2;
  byte hourEight = 3;

  byte AMLED = 10;
  byte PMLED = 11;
  byte sec = 12;
  //counters 
  byte secCount = 0;
  byte minCount = 0;
  byte hourCount = 0;
  byte AM = 1;
  byte PM = 0;

void setup() {
  pinMode(minOne, OUTPUT);
  pinMode(minTwo, OUTPUT);
  pinMode(minFour, OUTPUT);
  pinMode(minEight, OUTPUT);
  pinMode(minSixteen, OUTPUT);
  pinMode(minThirtyTwo, OUTPUT);

  pinMode(hourOne, OUTPUT);
  pinMode(hourTwo, OUTPUT);
  pinMode(hourFour, OUTPUT);
  pinMode(hourEight, OUTPUT);

  pinMode(AMLED, OUTPUT);
  pinMode(PMLED, OUTPUT);
  pinMode(sec, OUTPUT);
  
}

void loop() {
   // int tempHour;           //may be a cause of error if it doesnt work. temphour is to reduce the amount of times that the hour loop is done; only if the hourCount changes 
    minutes(minCount);
    hours(hourCount); 
    if (AM == 1)
    {
      digitalWrite(AMLED, HIGH);
      digitalWrite(PMLED, LOW);
    }
    if (PM == 1)
    {
      digitalWrite(PMLED, HIGH);
      digitalWrite(AMLED, LOW);
    }
    delay(800);
    digitalWrite(sec, HIGH);
    delay(199); //allowing 1ms for the rest of the loop to run
    digitalWrite(sec, LOW);
    secCount = secCount + 1;
    if (secCount == 60)
    {
      secCount = 0;
      minCount = minCount + 1;                      //concern expressed about the nested ifs. will this work? do they need to be after one another?
      if (minCount == 60)
      {
        minCount = 0;
        hourCount = hourCount + 1;
        if (hourCount == 12)
        {
          hourCount = 0;
          if (AM == 1)
          {
            AM = 0;
            PM = 1;
//            digitalWrite(AMLED, LOW);
//            digitalWrite(PMLED, HIGH);
          }
          else
          {
            AM = 1;
            PM = 0;
//            digitalWrite(PMLED, LOW);
//            digitalWrite(AMLED, HIGH);
          }
        }
        
      }
      
    
  }

}

void minutes(int count)
{
  digitalWrite(minOne, LOW);
  digitalWrite(minTwo, LOW);
  digitalWrite(minFour, LOW);
  digitalWrite(minEight, LOW);
  digitalWrite(minSixteen, LOW);
  digitalWrite(minThirtyTwo, LOW);
  
  if (count >= 32)
  {
    count = count - 32;
    digitalWrite(minThirtyTwo, HIGH);
  }
  if (count >= 16)
  {
    count = count - 16;
    digitalWrite(minSixteen, HIGH);
  }
  if (count >= 8)
  {
    count = count - 8;
    digitalWrite(minEight, HIGH);
  }
  if (count >= 4)
  {
    count = count - 4;
    digitalWrite(minFour, HIGH);
  }
  if (count >= 2)
  {
    count = count - 2;
    digitalWrite(minTwo, HIGH);
  }
  if (count == 1)
  {
    count = count - 1;
    digitalWrite(minOne, HIGH);
  }
  if (count != 0)
  {
    //error - outputs 63
    digitalWrite(minOne, HIGH);
    digitalWrite(minTwo, HIGH);
    digitalWrite(minFour, HIGH);
    digitalWrite(minEight, HIGH);
    digitalWrite(minSixteen, HIGH);
    digitalWrite(minThirtyTwo, HIGH);
  } 
  return;
}

void hours(int count)
{
  digitalWrite(hourOne, LOW);
  digitalWrite(hourTwo, LOW);
  digitalWrite(hourFour, LOW);
  digitalWrite(hourEight, LOW);
  
  if (count >= 8)
  {
    count = count - 8;
    digitalWrite(hourEight, HIGH);
  }
  if (count >= 4)
  {
    count = count - 4;
    digitalWrite(hourFour, HIGH);
  }
  if (count >= 2)
  {
    count = count - 2;
    digitalWrite(hourTwo, HIGH);
  }
  if (count == 1)
  {
    count = count - 1;
    digitalWrite(hourOne, HIGH);
  }
  if (count != 0)
  {
    //error - outputs 15
    digitalWrite(hourOne, HIGH);
    digitalWrite(hourTwo, HIGH);
    digitalWrite(hourFour, HIGH);
    digitalWrite(hourEight, HIGH);
  }
  return;
}

“infact, the new code doesn’t work as a clock anymore!! :(”
You do have older versions?

yeah, the code in the reply is the last working version (should’ve said)… but in the code with settings, it doesnt?

thanks

If you want to learn coding, I suggest: - how to debounce switches (Debounce, StateChangeDetection) - blink without delay - how to access bits (bitRead() or &)

Then you can rewrite your code with a fraction of the current size, what makes it easier to debug.

okay cheer mate, i'll have to do that :confused:

i found a little niggle in the new one that stopped it counting up and am now working on setting the time using a different method. I did as suggested and looked up how to use inputs and turns out I had it a bit wrong :confused: but now i have it so it clears the timer and lights up one LED to say that its in the mode.
the tricky but next is going to be the actual time setting!

Thanks for you suggestion :slight_smile:

James