Skip to content

Conversation

@BlinkDynamo
Copy link

@BlinkDynamo BlinkDynamo commented Nov 15, 2025

Fixes #7609

Changed syslog messages from always using NILVALUE for the TIMESTAMP field. Now uses RFC5424 UTC timestamp format when a valid time is available from RTC. Falls back to NILVALUE if no valid time is available.

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

* Changed syslog messages from always using NILVALUE for the TIMESTAMP field.

* Now uses RFC5424 UTC timestamp format when a valid time is available from RTC.

* Falls back to NILVALUE if no valid time is available.
@CLAassistant
Copy link

CLAassistant commented Nov 15, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@BlinkDynamo BlinkDynamo changed the title Populate syslog timestamp field when time is available (#7609) Populate syslog timestamp field when time is available Nov 15, 2025
@fifieldt fifieldt added the enhancement New feature or request label Nov 16, 2025
@fifieldt fifieldt requested a review from jp-bennett November 16, 2025 00:24
@thebentern thebentern requested a review from Copilot November 17, 2025 01:14
Copilot finished reviewing on behalf of thebentern November 17, 2025 01:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances syslog messages by populating the timestamp field with RFC5424 UTC timestamps when valid time is available from the RTC, instead of always using NILVALUE ("-").

Key changes:

  • Added RTC time retrieval to check if valid time is available
  • Implemented RFC5424-compliant UTC timestamp formatting (YYYY-MM-DDTHH:MM:SSZ)
  • Falls back to NILVALUE when no valid time exists

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +188 to +194
char timestamp_buffer[21];
snprintf(timestamp_buffer, sizeof(timestamp_buffer), "%04d-%02d-%02dT%02d:%02d:%02dZ",
time_info->tm_year + 1900, time_info->tm_mon + 1, time_info->tm_mday,
time_info->tm_hour, time_info->tm_min, time_info->tm_sec);
this->_client->print(timestamp_buffer);
} else {
this->_client->print(SYSLOG_NILVALUE);
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gmtime() function can return NULL if the time value is invalid. This could lead to a null pointer dereference when accessing time_info->tm_year. Add a null check before using the returned pointer.

struct tm *time_info = gmtime(&timestamp);
if (time_info != NULL) {
    char timestamp_buffer[21];
    snprintf(timestamp_buffer, sizeof(timestamp_buffer), "%04d-%02d-%02dT%02d:%02d:%02dZ",
             time_info->tm_year + 1900, time_info->tm_mon + 1, time_info->tm_mday,
             time_info->tm_hour, time_info->tm_min, time_info->tm_sec);
    this->_client->print(timestamp_buffer);
} else {
    this->_client->print(SYSLOG_NILVALUE);
}
Suggested change
char timestamp_buffer[21];
snprintf(timestamp_buffer, sizeof(timestamp_buffer), "%04d-%02d-%02dT%02d:%02d:%02dZ",
time_info->tm_year + 1900, time_info->tm_mon + 1, time_info->tm_mday,
time_info->tm_hour, time_info->tm_min, time_info->tm_sec);
this->_client->print(timestamp_buffer);
} else {
this->_client->print(SYSLOG_NILVALUE);
if (time_info != NULL) {
char timestamp_buffer[21];
snprintf(timestamp_buffer, sizeof(timestamp_buffer), "%04d-%02d-%02dT%02d:%02d:%02dZ",
time_info->tm_year + 1900, time_info->tm_mon + 1, time_info->tm_mday,
time_info->tm_hour, time_info->tm_min, time_info->tm_sec);
this->_client->print(timestamp_buffer);
} else {
this->_client->print(SYSLOG_NILVALUE);
}

Copilot uses AI. Check for mistakes.
this->_client->print(F(">1 "));

// Format RFC5424 timestamp if valid time is available.
uint32_t timestamp_sec = getValidTime(RTCQualityDevice, false);
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RTCQualityDevice enum value should be qualified with the RTCQuality:: namespace. Based on other usages in the codebase (e.g., in RedirectablePrint.cpp, NMEAWPL.cpp), this should be RTCQuality::RTCQualityDevice.

uint32_t timestamp_sec = getValidTime(RTCQuality::RTCQualityDevice, false);
Suggested change
uint32_t timestamp_sec = getValidTime(RTCQualityDevice, false);
uint32_t timestamp_sec = getValidTime(RTCQuality::RTCQualityDevice, false);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Set syslog timestamp field when possible

4 participants