Logging with colors

[IshDeshpa]

Added log.h library from Operating Systems class and made sure it works on picocom and PuTTY.

[gemini-code-assist]

Summary of Changes

Hello @IshDeshpa, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates a new colored logging library, log.h, designed to improve debugging visibility by categorizing log messages with different colors and severity levels. The changes include updating the build configuration, documenting the new utility, and providing a dedicated test application to validate its functionality on serial terminals like picocom and PuTTY.

Highlights

• Colored Logging Library: Introduced a new log.h header file that provides a colored logging utility with various severity levels (FATAL, ERROR, WARN, INFO, DEBUG, TRACE) to enhance debugging output.
• Makefile Update: Modified the Makefile to include a new CFLAG (-ffile-prefix-map) and reformatted the CFLAGS definition for improved readability.
• Documentation Added: Updated docs/FlashAndTheBug.md with a new section introducing the logging library and crediting its author.
• Logging Test Application: Added test/tests/log_test.c, a new test application that demonstrates the functionality of the colored logging library by printing messages at all defined levels through UART.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a new logging library with color support, which is a great addition for debugging. The implementation is clean, but there is a critical thread-safety issue in log.h due to the use of shared global buffers without synchronization. In a multithreaded environment like FreeRTOS, this will lead to race conditions and corrupted log output. I've provided a comment detailing this issue.

Additionally, I've made a few suggestions to improve code robustness and clarity in the new logging header and to correct a misleading comment in the test file. The changes to the Makefile and documentation look good.

[gemini-code-assist]

The global static buffers log_buf and log_before_buf are not thread-safe. This project uses FreeRTOS, and if two tasks call the log() macro concurrently, they will write to these buffers at the same time, leading to a race condition and corrupted log output.

To ensure thread safety, all operations within the log macro that use these shared buffers should be atomic. This can be achieved by protecting the critical section with a mutex. You'll likely need to add an initialization function for the logger to create the mutex, which should be called once at startup before any logging occurs.

[gemini-code-assist]

This function returns NULL if an unknown LoggingLevel is passed. This NULL is then passed to snprintf with a %s format specifier, which results in undefined behavior and could crash the program.

To make the logger more robust, consider adding a default case to handle unknown levels.

static inline const char *
log_level_name(LoggingLevel level)
{
    switch (level) {
    case L_TRACE: return "TRACE";
    case L_DEBUG: return "DEBUG";
    case L_INFO:  return "INFO";
    case L_WARN:  return "WARN";
    case L_ERROR: return "ERROR";
    case L_FATAL: return "FATAL";
    case L_NONE:  return "NONE";
    default:      return "?????";
    }
}

[gemini-code-assist]

Similar to log_level_name, this function returns NULL for an unknown LoggingLevel, which can lead to undefined behavior. Adding a default case will make the function more robust. It should probably return the 'reset color' code to avoid breaking terminal output.

static inline const char *
log_level_color(LoggingLevel level)
{
    switch (level) {
    case L_TRACE: return "\x1b[00;34m";
    case L_DEBUG: return "\x1b[00;35m";
    case L_INFO:  return "\x1b[00;36m";
    case L_WARN:  return "\x1b[00;33m";
    case L_ERROR: return "\x1b[00;31m";
    case L_FATAL: return "\x1b[37;41m";
    case L_NONE:  return "\x1b[0m";
    default:      return "\x1b[0m";
    }
}

[gemini-code-assist]

The casts (char *)&log_before_buf and (char *)&log_buf are unnecessary and potentially confusing. Since log_before_buf and log_buf are arrays of char, they will decay to char* when passed to printf. You can use them directly.

    printf("%-" AS_STRING(LOG_MAX_BEFORE_SIZE) "s | %s%s\n\r",
           log_before_buf, log_buf, log_level_color(L_NONE));

[gemini-code-assist]

The cast (char *)&log_buf is unnecessary. Since log_buf is an array of char, it will decay to a char* when passed to snprintf. You can use log_buf directly.

            snprintf(log_buf, LOG_MAX_LOG_SIZE, fmt);

[gemini-code-assist]

The comments describing the logging levels are incorrect and incomplete. They are missing L_INFO and have the wrong values for L_DEBUG and L_TRACE. This can be misleading for anyone reading the test.

The correct values from log.h are:

• L_INFO: 4
• L_DEBUG: 5
• L_TRACE: 6

// Logging levels:
// L_FATAL: 1
// L_ERROR: 2
// L_WARN: 3
// L_INFO: 4
// L_DEBUG: 5
// L_TRACE: 6