Cutting down code

I'am building a clock with some HPDL 1414 displays and a DS3231 RTC. It works just fine, but I have some code that for sure can be optimised. Lets take seconds, each second have to be converted before sending the data, I'am using following code: (just part of code, I'am sure you'll get the idea). Any help would be great. Regards Rob

void seccc(){
  if (second == 10){
   sec2 = 49;
   sec = 48;
   sek2();
    sek1();
  }else{
     if (second == 11){
   sec2 = 49;
   sec = 49;
   sek2();
    sek1();
      }else{
      if (second == 12){
   sec2 = 49;
   sec = 50;
   sek2();
    sek1();
      }else{
      if (second == 13){
   sec2 = 49;
   sec = 51;
   sek2();
    sek1();
      }else{
      if (second == 14){
   sec2 = 49;
   sec = 52;
   sek2();
    sek1();
      }else{
      if (second == 15){
   sec2 = 49;
   sec = 53;
   sek2();
    sek1();
void seccc()
{
  if(second>=10 && second<=15)
  {
    sec2=49;
    sec=48+second-10;
    sek2();
    sek1();
  }
}

As your always set sec2 = 49 then that just moves out and goes to the top. You then always call sek1() and sek2() so these move to the bottom. We then and then we look at sec and se that you could calculate it so your code ends pu looking like this.

void seccc(){
      sec2 = 49;
     sec = calculation // YOU WORK IT OUT!!!!!!!
     sek1();
     sek2();
}

@OP you work out the calculation for sec. @every one else let the OP do it.

Mark

sec=48+second-10;

Wrong!!!

Mark

Rob, I think you have sent everyone up a dead end by not explaining what you really need and not posting your complete sketch, not even the complete function.

Sounds to me like you need to convert seconds (and presumably minutes and hours) into 2 codes for your display to show as 2 digits.

All you need to do is this:

To get the code for the tens of seconds, use "seconds / 10 + 48" To get the code for the units of seconds use "seconds % 10 + 48"

That's assuming you have the seconds in binary already, it looks from your sketch that you do. But the seconds, minutes and hours values from the ds3231 come as binary-coded-decimal. I assume you must have converted these to binary somewhere else in your code. If not, just replace "10" with "16" in my formulae above.

Paul

holmes4: Wrong!!!!!

Does that mean you don't know the answer or are trying to misguide the OP? (:

Cheers, Jarkko

Does that mean you don't know the answer or are trying to misguide the OP? (:

No it means theres an error it your code. Hint - 2 constants in the same expression.

Mark

Erm, that's not an error. You should look into how compiler optimizes constant expressions.

Thanks for the help. Paul was spot on. It really cut a LOT of crap away. The conversion from BCD to DEC could probably also be removed. Next is adding day of week txt, day og month and month txt. Here is the ugly code, it has some debug stuff in it too:

#include "Wire.h"
#define DS3231_I2C_ADDRESS 0x68  // This is the I2C address
#define I2C_WRITE Wire.send 
#define I2C_READ Wire.receive
byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
byte zero;
// Convert normal decimal numbers to binary coded decimal
byte decToBcd(byte val)
{
  return ( (val/10*16) + (val%10) );
}
// Convert binary coded decimal to normal decimal numbers
byte bcdToDec(byte val)
{
  return ( (val/16*10) + (val%16) );
}
   void set()
   {
   Wire.beginTransmission(DS3231_I2C_ADDRESS);
   I2C_WRITE(zero);
   I2C_WRITE(decToBcd(second) & 0x7f);    // 0 to bit 7 starts the clock
   I2C_WRITE(decToBcd(minute));
   I2C_WRITE(decToBcd(hour));                                  
   I2C_WRITE(decToBcd(dayOfWeek));
   I2C_WRITE(decToBcd(dayOfMonth));
   I2C_WRITE(decToBcd(month));
   I2C_WRITE(decToBcd(year));
   Wire.endTransmission();
}
// Gets the date and time from the ds1307 and prints result
void getDateDs3231()
{
  // Reset the register pointer
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  I2C_WRITE(zero);
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS, 7);

  // A few of these need masks because certain bits are control bits
  second     = bcdToDec(I2C_READ() & 0x7f);
  minute     = bcdToDec(I2C_READ());
  hour       = bcdToDec(I2C_READ() & 0x3f);  // Need to change this if 12 hour am/pm
  dayOfWeek  = bcdToDec(I2C_READ());
  dayOfMonth = bcdToDec(I2C_READ());
  month      = bcdToDec(I2C_READ());
  year       = bcdToDec(I2C_READ());
}
//----------------------------------------------
void sek1(){
   PORTA = second%10+48;
    delay(1);                // wait for a ms 
    digitalWrite(15, LOW); 
    digitalWrite(18, LOW);  
   delay(1);                // wait for a ms
    digitalWrite(23, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(23, HIGH);  // Write Pin High
    delay(1);    
 }
//------------------------------------------------------------------------------------------
void sek2(){
   PORTA = second/10+48;
    delay(1);                // wait for a ms    
    digitalWrite(15, HIGH);
    digitalWrite(18, LOW);  
   delay(1);                // wait for a ms
    digitalWrite(23, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(23, HIGH);  // Write Pin High
   delay(1);  
}
//---------------------------------------------------------------------------------------------
void minutt1(){
  PORTA = minute%10+48;
    delay(1);                // wait for a ms 
    digitalWrite(15, HIGH);  
    digitalWrite(18, HIGH);   
   delay(1);                // wait for a ms
    digitalWrite(23, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(23, HIGH);  // Write Pin High
   delay(1);    
 }
//------------------------------------------------------------------------------------------
void minutt2(){
  PORTA = minute/10+48;
    delay(1);                // wait for a ms    
    digitalWrite(15, LOW); 
    digitalWrite(18, LOW);  
   delay(1);                // wait for a ms
    digitalWrite(0, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(0, HIGH);  // Write Pin High
   delay(1);  
}
//---------------------------------------------------------------------------------------------
void time1(){
  PORTA = hour%10+48;
    delay(1);                // wait for a ms 
    digitalWrite(15, LOW); 
    digitalWrite(18, HIGH); 
   delay(1);                // wait for a ms
    digitalWrite(0, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(0, HIGH);  // Write Pin High
   delay(1);    
 }
//------------------------------------------------------------------------------------------
void time2(){
  PORTA = hour/10+48;
    delay(1);                // wait for a ms    
    digitalWrite(15, HIGH);  
    digitalWrite(18, HIGH);   
   delay(1);                // wait for a ms
    digitalWrite(0, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(0, HIGH);  // Write Pin High
   delay(1);  
}
//--------------------------------------------------------------------------------------------
void setup() {
  DDRA = DDRA | B11111111; // set port a to output
  pinMode(15, OUTPUT); //a0 digit select
  pinMode(18, OUTPUT); //a1 digit select
  pinMode(19, OUTPUT); //wr display 6
  pinMode(20, OUTPUT); //wr display 5
  pinMode(21, OUTPUT); //wr display 4
  pinMode(22, OUTPUT); //wr display 3
  pinMode(23, OUTPUT); //wr display 2
   pinMode(0, OUTPUT); //wr display 1 
      digitalWrite(22, HIGH);
       digitalWrite(21, HIGH);
        digitalWrite(20, HIGH);
        digitalWrite(19, HIGH);  
  Wire.begin();
Serial.begin(9600); // for debug brug!
  zero=0x00;
      hour = 23;
 minute = 59;
second = 55;
set();
//-------------------------------------------------------------------------------------------------------------------
}
void loop() {
   getDateDs3231(); // kører sub routine for at hente tid fra rtc, vender tilbage herfra og kører lcd routine 
  Serial.println(hour, DEC);
//--------------------skriver . i display --------------------------------------------------------------------------------------
    PORTA = 46;
    delay(1);                // wait for a ms  
    digitalWrite(15, LOW);  
    digitalWrite(18, HIGH);   
   delay(1);                // wait for a ms
    digitalWrite(23, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(23, HIGH);  // Write Pin High
delay(1);                // wait for a ms  
   digitalWrite(15, HIGH);  
    digitalWrite(18, LOW);   
 delay(1);                // wait for a ms
  digitalWrite(0, LOW);   // Write pin Low
  delay(1);                // wait for a ms
    digitalWrite(0, HIGH);  // Write Pin High
   delay (1);
////---------------------------------------------------------------------------------------------------------    

sek1();
sek2();
minutt1();
minutt2();
time1();
time2();
}

Hi Rob, another suggestion would be to replace your 8 functions to set each of your 8 digits:

void displayDigit(byte code, int digAdrs1, int digAdrs2, int chipSel){
   PORTA = code;
    delay(1);                // wait for a ms 
    digitalWrite(15, digAdrs1); 
    digitalWrite(18, digAdrs2);  
   delay(1);                // wait for a ms
    digitalWrite(chipSel, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(chipSel, HIGH);  // Write Pin High
    delay(1);    
 }

Then, change this:

sek1();
sek2();
minutt1();
minutt2();
time1();
time2();

To this:

displayDigit(second%10+48, LOW, LOW, 23)
displayDigit(second/10+48, HIGH, LOW, 23);
displayDigit(minute%10+48, HIGH, HIGH, 0);
displayDigit(minute/10+48, LOW, LOW, 0);
displayDigit(hour%10+48, LOW, HIGH, 0);
displayDigit(hour/10+48, HIGH, HIGH, 0);

(I think there is an error in the above. The units of minutes and tens of hours are being sent to the same digit?)

Also this:

    PORTA = 46;
    delay(1);                // wait for a ms  
    digitalWrite(15, LOW);  
    digitalWrite(18, HIGH);   
   delay(1);                // wait for a ms
    digitalWrite(23, LOW);   // Write pin Low
    delay(1);                // wait for a ms
    digitalWrite(23, HIGH);  // Write Pin High
delay(1);                // wait for a ms  
   digitalWrite(15, HIGH);  
    digitalWrite(18, LOW);   
 delay(1);                // wait for a ms
  digitalWrite(0, LOW);   // Write pin Low
  delay(1);                // wait for a ms
    digitalWrite(0, HIGH);  // Write Pin High
   delay (1);

becomes this:

displayDigit(46, LOW, HIGH, 23)
displayDigit(46, HIGH, LOW, 0)

(But I don't know what this is for. Should be in setup() not loop()?)

Paul