This timer formatting code doesn't feel optimized to me. Am I missing something?

Working code at bottom:

I am outputting digital time to an oled display. It appears that the first way I tried to use less code with doesn’t give me the formatting I desire, I either have to write my min_str and sec_str to the screen separately, and the formatting isn’t great, and I get overlap issues…
OR
I use dtostrf twice on those strings, then sprintf twice on those strings to format my semicolon spacing, THEN use sprintf yet again to build the whole string so the semicolon isn’t spaced wrong.
I feel like I am missing a more efficient way of doing this…

First off, why wouldn’t something like this provide the expected result since I’ve already done two dtostrf’s prior to where it is in the code?

 sprintf(time_str, "%02d:%02d",m,s); 

When I do that, seconds just disappear and it doesn’t update… So then I tried…
Just printing min_str and sec_str like this:

sprintf(min_str, "%02d:",m);
  display.setTextColor(WHITE, BLACK);
  display.setTextSize(1);
  display.setCursor(0, 57);
   
  display.write(min_str);
  
  sprintf(sec_str, "%02d",s); 
  display.setCursor(17,57);

  display.write(sec_str);

at the first minute, the semicolon is spaced too far from the 1.

So then I tried the full block of code below using the 3 sprintfs, it is formatted correctly, but I feel like this is too many steps to get the desired result, which is something like 00:00, 01:34, etc…

using 96x64 color oled SSD1331

[code]

#include <Adafruit_SSD1331.h>
#include <SPI.h>
#include <Wire.h>
#include <Adafruit_GFX.h>

//********************NANO OLED DEFINITIONS for use when uploading NANO Code

#define SCL 13
#define SDA 11
#define CS 10
#define RST 9
#define DC 8

//Adafruit_SSD1331 display = Adafruit_SSD1331(CS, DC,SDA, SCL, RST); // for non -spi on worse boards

Adafruit_SSD1331 display = Adafruit_SSD1331(&SPI, CS, DC, RST);
//SPISettings settingsA(2000000, MSBFIRST, SPI_MODE1); // use lower MB values for worse boards to reduce impedance issues

// _________________________________________________________________Color definitions

#define BLACK 0x0000
#define BLUE 0x0331
#define RED 0xF820
#define GREEN 0x07E0
#define WHITE 0xFFFF
#define YELLOW 0xFFE0
#define BROWN 0x79E0

void setup() {

SPI.begin();
display.begin(); // initialize with the I2C addr 0x3D (for the 128x64)
display.fillScreen(BLACK);

}

void loop() {

elapsedTimer();
}

void elapsedTimer() {

unsigned long m,s;
unsigned long getTime = millis();

char time_str[7] = “”;
char min_str[3] = “”;
char sec_str[3] = “”;

m= int(getTime/60000);
getTime=getTime%60000;
s= int(getTime/1000);

dtostrf(s, 2, 0, sec_str);
dtostrf(m, 2, 0, min_str);

sprintf(min_str, “%02d”,m);
sprintf(sec_str, “%02d”,s);

// sprintf(time_str, “%02d:%02d”,m,s); // this didn’t work as expected

sprintf(time_str, “%02s:%02s”,min_str,sec_str);

display.setTextColor(WHITE, BLACK);
display.setTextSize(1);
display.setCursor(0,25);
display.write(time_str);

}[/code]

dtostrf is for formating double (floats), which the sprintf() “%f” option doesn’t support on arduino.

there’s no need to use dtostrf() because the values you want to display are ints.

shouldn’t seconds be (getTIme % 60000) / 1000 ?

%d is for signed integers; your variables m and s are unsigned long and you have to use %lu.

You have to be very careful as min_str and sec_str can only hold two digits; %02d or %02lu does not mean that the result is truncated to two digits. If millis() reaches 0xFFFFFFFF, m will be 6046 which will not fit in a 2+1 character array.

That makes sense now. Thanks!

If I replace that in the code the device locks up at 32 seconds…

Optimized as follows: Didn’t realize I was trying things that wouldn’t work because of limited functionality.

void elapsedTimer() {

int m,s;
unsigned long getTime = millis();

char time_str[7] = "";   
 
  m= getTime/60000;
  getTime=getTime%60000;
  s= getTime/1000;
  
  sprintf(time_str, "%02i:%02i",m,s);  

  display.setTextColor(WHITE, BLACK);
  display.setTextSize(1);
  display.setCursor(0,57);
  display.write(time_str);
}

This timer will never need to run more than 15 min.
My next task will be to try to start this timer on a variable state change, and have it count from that point up, not from the time the sketch started. That should be easy enough I hope…

i’d like to see that code

That’s because you can’t pass ‘unsigned long’ values and tell sprintf() to treat them as ‘int’. I think you need:

sprintf(time_str, “%02lu:%02lu”, m, s);

That was before I changed my sprintf and got rid of the dtostrf. It works fine now. and the code is much shorter. Thanks.

your response suggests that what i recommended, not some other change you may have been making, will cause a lock-up.

if it doesn’t, i think you should delete your comment.

seems to work correct in the following code

void
loop (void)
{
    char          time_str [10];
    unsigned long getTime = millis();

    int m = getTime/60000;
    int s = (getTime % 60000)/1000;

    sprintf(time_str, "%02i:%02i",m,s);
    Serial.println (time_str);
}

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.