Added 12 Hour Format 🚀

[AntonioBerna]

Close #222 using the following code:

int NTPClient::getHours12() const {
  int hours24 = this->getHours();
  if (hours24 == 0) {
    return 12; // Midnight is 12 AM
  } else if (hours24 > 12) {
    return hours24 - 12; // PM hours
  } else {
    return hours24; // AM hours (1-12)
  }
}

bool NTPClient::isPM() const {
  return (this->getHours() >= 12);
}

String NTPClient::getFormattedTime12() const {
  int hours12 = this->getHours12();
  String hoursStr = hours12 < 10 ? "0" + String(hours12) : String(hours12);

  int minutes = this->getMinutes();
  String minuteStr = minutes < 10 ? "0" + String(minutes) : String(minutes);

  int seconds = this->getSeconds();
  String secondStr = seconds < 10 ? "0" + String(seconds) : String(seconds);

  String period = this->isPM() ? " PM" : " AM";

  return hoursStr + ":" + minuteStr + ":" + secondStr + period;
}

[github-actions]

Memory usage change @ 789bcf4

Board	flash	%	RAM for global variables	%
esp8266:esp8266:huzzah	N/A	N/A	N/A	N/A

Click for full report table

Board	examples/Advanced flash	%	examples/Advanced RAM for global variables	%	examples/Basic flash	%	examples/Basic RAM for global variables	%	examples/IsTimeSet flash	%	examples/IsTimeSet RAM for global variables	%	examples/TimeFormats flash	%	examples/TimeFormats RAM for global variables	%
esp8266:esp8266:huzzah	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A

Click for full report CSV

Board,examples/Advanced<br>flash,%,examples/Advanced<br>RAM for global variables,%,examples/Basic<br>flash,%,examples/Basic<br>RAM for global variables,%,examples/IsTimeSet<br>flash,%,examples/IsTimeSet<br>RAM for global variables,%,examples/TimeFormats<br>flash,%,examples/TimeFormats<br>RAM for global variables,%
esp8266:esp8266:huzzah,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A

[Randomblock1]

I would strongly recommend rewriting this. This is a much simpler and efficient way to do it:

int NTPClient::getHours12() const {
  int h = this->getHours() % 12;
  return (h == 0) ? 12 : h;
}

bool NTPClient::isPM() const {
  return this->getHours() >= 12;
}

String NTPClient::getFormattedTime12() const {
  char buffer[12]; // "HH:MM:SS AM\0"
  int h12 = getHours12();
  int m = getMinutes();
  int s = getSeconds();
  const char* period = isPM() ? "PM" : "AM";
  snprintf(buffer, sizeof(buffer), "%02d:%02d:%02d %s", h12, m, s, period);
  return String(buffer);
}

Modulo makes the hour calculation simpler, snprintf formats the string from inputs and is much more efficient than concatenating strings.

[AntonioBerna]

I would strongly recommend rewriting this. This is a much simpler and efficient way to do it:

int NTPClient::getHours12() const {
  int h = this->getHours() % 12;
  return (h == 0) ? 12 : h;
}

bool NTPClient::isPM() const {
  return this->getHours() >= 12;
}

String NTPClient::getFormattedTime12() const {
  char buffer[12]; // "HH:MM:SS AM\0"
  int h12 = getHours12();
  int m = getMinutes();
  int s = getSeconds();
  const char* period = isPM() ? "PM" : "AM";
  snprintf(buffer, sizeof(buffer), "%02d:%02d:%02d %s", h12, m, s, period);
  return String(buffer);
}

Modulo makes the hour calculation simpler, snprintf formats the string from inputs and is much more efficient than concatenating strings.

My implementation follows the structure of the 24-hour implementation:

String NTPClient::getFormattedTime() const {
  unsigned long rawTime = this->getEpochTime();
  unsigned long hours = (rawTime % 86400L) / 3600;
  String hoursStr = hours < 10 ? "0" + String(hours) : String(hours);

  unsigned long minutes = (rawTime % 3600) / 60;
  String minuteStr = minutes < 10 ? "0" + String(minutes) : String(minutes);

  unsigned long seconds = rawTime % 60;
  String secondStr = seconds < 10 ? "0" + String(seconds) : String(seconds);

  return hoursStr + ":" + minuteStr + ":" + secondStr;
}

Do you think a major refactoring is needed?

The options are:

1. Leave my implementation following the current standard
2. Change everything and create a new standard based on snprintf

[Randomblock1]

Sorry, I didn't notice that it was based off the existing code.
I like the idea of using snprintf. It's simpler to understand & format, and it's 1 stack allocation vs like 7 heap allocations.

However, I don't know how much more/less flash including that uses, and I also am not sure if the efficiency gains are going to be that noticeable. I mean it's only a few bytes of strings. Not sure if we should prioritize size or speed, but I think it's likely 1 extra KB is not worth a minor speed improvement.

If we really want fast & light, maybe we use something like this:

String NTPClient::getFormattedTime12() const {
  char buffer[12]; // "HH:MM:SS AM\0"
  
  int h12 = getHours12();
  int m = getMinutes();
  int s = getSeconds();

  buffer[0] = (h12 / 10) + '0';
  buffer[1] = (h12 % 10) + '0';
  buffer[2] = ':';
  buffer[3] = (m / 10) + '0';
  buffer[4] = (m % 10) + '0';
  buffer[5] = ':';
  buffer[6] = (s / 10) + '0';
  buffer[7] = (s % 10) + '0';
  buffer[8] = ' ';
  
  bool pm = isPM();
  buffer[9] = pm ? 'P' : 'A';
  buffer[10] = 'M';
  buffer[11] = '\0';

  return String(buffer);
}

Not particularly pretty, but it doesn't use Strings nor snprintf. For something like a static format template this is much smaller and faster.

[AntonioBerna]

@Randomblock1 I followed the existing pattern in getFormattedTime() to maintain consistency. Since these functions are called infrequently (typically once per second for display), the performance difference is negligible. If we want to optimize, we should optimize both functions together, but I don't think it's necessary for this use case. The String overhead is acceptable for occasional formatting calls.

[CLAassistant]

All committers have signed the CLA.

[AntonioBerna]

@per1234 news?