Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option to use datetime stamp during logging. (IDFGH-3855) #5761

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+74 −6
Open

Added option to use datetime stamp during logging. (IDFGH-3855) #5761

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
187db04
Added option to use datetime stamp during logging.
Molorius Aug 20, 2020
14304ee
Further separated timestamp and datetimestamp.
Molorius Sep 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions components/log/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,12 @@ menu "Log output"
bool "System Time"
endchoice

config LOG_TIMESTAMP_SOURCE_SYSTEM_DATETIME
Copy link
Contributor

@projectgus projectgus Aug 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Relatively minor, but for ease of use suggest adding this as a "choice" so the choices become "Milliseconds Since Boot", "System Time", "System Date and Time"

(Means some of the macro guards in esp_log.h will need to be updated as well.)

Copy link
Author

Choose a reason for hiding this comment

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

It is now another choice in the kconfig.

bool "Log Date and Time"
default "n"
depends on LOG_TIMESTAMP_SOURCE_SYSTEM
help
Use full date and time in logs, rather than just time.
e.g. (1990-01-31 00:01:30.000)

endmenu
16 changes: 14 additions & 2 deletions components/log/log_freertos.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ void esp_log_impl_unlock(void)

char *esp_log_system_timestamp(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

The description of this function in esp_log.h needs to be updated to match

Copy link
Author

Choose a reason for hiding this comment

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

Would you prefer changing the name of the function or splitting it into esp_log_system_timestamp and esp_log_system_datetimestamp? The xTaskGetSchedulerState portion would be put into a static function for use between both.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Molorius Good point! That is probably a cleaner solution, thank you for pointing it out.

However if it becomes less clean to implement in some way, I'm OK with the current implementation - it just needs to be noted in the header that it might return a date+time depending on config.

Copy link
Author

Choose a reason for hiding this comment

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

I separated the datetime elements into a separate function and updated esp_log.h to match.

{
static char buffer[18] = {0};
#if CONFIG_LOG_TIMESTAMP_SOURCE_SYSTEM_DATETIME
static char buffer[24] = {0};
#else
static char buffer[13] = {0};
#endif
static _lock_t bufferLock = 0;

if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED) {
Expand All @@ -79,7 +83,15 @@ char *esp_log_system_timestamp(void)

_lock_acquire(&bufferLock);
snprintf(buffer, sizeof(buffer),
"%02d:%02d:%02d.%03ld",
#if CONFIG_LOG_TIMESTAMP_SOURCE_SYSTEM_DATETIME
"%04u-%02u-%02u "
#endif
"%02u:%02u:%02u.%03lu",
#if CONFIG_LOG_TIMESTAMP_SOURCE_SYSTEM_DATETIME
timeinfo.tm_year+1900,
timeinfo.tm_mon+1,
timeinfo.tm_mday,
#endif
timeinfo.tm_hour,
timeinfo.tm_min,
timeinfo.tm_sec,
Expand Down