feat: Add central logging pragma by jpirnay · Pull Request #843 · crosspoint-reader/crosspoint-reader

jpirnay · 2026-02-11T20:52:07Z

Summary

Definition and use of a central LOG function, that can later be extended or completely be removed (for public use where debugging information may not be required) to save flash by suppressing the -DENABLE_SERIAL_LOG like in the slim branch
What changes are included?

Additional Context

By using the central logger the usual:

#include <HardwareSerial.h>
...
  Serial.printf("[%lu] [WCS] Obfuscating/deobfuscating %zu bytes\n", millis(), data.size());

would then become

#include <Logging.h>
...
  LOG_DBG("WCS", "Obfuscating/deobfuscating %zu bytes", data.size());

You do have LOG_DBG for debug messages, LOG_ERR for error messages and LOG_INF for informational messages. Depending on the verbosity level defined (see below) soe of these message types will be suppressed/not-compiled.

The normal compilation (default) will create a firmware.elf file of 42.194.356 bytes, the same code via slim will create 42.024.048 bytes - 170.308 bytes less
Firmware.bin : 6.469.984 bytes for default, 6.418.672 bytes for slim - 51.312 bytes less

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? NO

Uri-Tauber · 2026-02-11T22:28:13Z

Very Nice!

lib/Logging/Logging.h

jpirnay · 2026-02-12T09:17:08Z

I extended / renamed the logging macros to
- LOG_ERR to be used for error messages
- LOG_INF to be used for informational messages
- LOG_DBG to be used for debug messages
Added a LOG_LEVEL define that controls log verbosity levels:
- 0: ERR only
- 1: ERR + INF
- 2: ERR + INF + DBG
  If LOG_LEVEL is not defined, it defaults to 0
The logging macros are now conditionally defined based on both ENABLE_SERIAL_LOG (to enable/disable logging entirely) and LOG_LEVEL (to control verbosity)

I did my best to assign LOG_ERR where I found them appropriate, but I am sure I missed quite a lot

jpirnay · 2026-02-12T09:34:19Z

Profile	Verbosity	.elf size	Delta	.bin size	Delta
slim	suppressed	42.024.044	-	6.418.672	-
gh_release	err	42.048.784	+24.740	6.432.384	+13.712
gh_release_rc	err +inf	42.058.268	+34.224	6.432.736	+14.064
default	err +inf +dbg	42.198.452	+174.408	6.473.376	+54.704

ngxson · 2026-02-12T12:13:55Z

platformio.ini

 [env:gh_release]
 extends = base
 build_flags =
  ${base.build_flags}
  -DCROSSPOINT_VERSION=\"${crosspoint.version}\"
+  -DENABLE_SERIAL_LOG
+  -DLOG_LEVEL=0 ; Set log level to error for release builds  

 [env:gh_release_rc]
 extends = base
 build_flags =
  ${base.build_flags}
  -DCROSSPOINT_VERSION=\"${crosspoint.version}-rc+${sysenv.CROSSPOINT_RC_HASH}\"
+  -DENABLE_SERIAL_LOG
+  -DLOG_LEVEL=1 ; Set log level to info for release candidate builds  


In general, I think both RC and release should have the same level=info, WDYT @daveallie ?

ngxson · 2026-02-12T12:16:41Z

Asking for extra review from @coderabbitai since the change is large, just have a quick look to see if any log statement are mislabeled (for example, LOG_INF --> LOG_ERR and vice versa)

coderabbitai · 2026-02-12T12:18:08Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@ngxson I'll review the log statements in this PR to check for any mislabeled severity levels.

🏁 Script executed:

#!/bin/bash
# Get the PR diff to review all log statements
gh pr diff 843 > /tmp/pr_diff.txt

# Extract all LOG_ERR, LOG_INF, and LOG_DBG statements with context
echo "=== LOG_ERR statements ==="
grep -n "LOG_ERR" /tmp/pr_diff.txt | head -100

echo -e "\n=== LOG_INF statements ==="
grep -n "LOG_INF" /tmp/pr_diff.txt | head -100

echo -e "\n=== LOG_DBG statements ==="
grep -n "LOG_DBG" /tmp/pr_diff.txt | head -100

Length of output: 14592

@ngxson I've reviewed the log statements in this PR. The LOG_ERR and LOG_INF statements look correctly labeled. However, I found several LOG_DBG statements that should likely be LOG_ERR instead, as they represent actual error conditions rather than debug information.

Issues Found

Many LOG_DBG statements with "Could not", "Failed to", or "Cannot" messages appear to be error conditions that would prevent the application from functioning correctly. Here are the main categories:

1. File I/O and Parsing Errors (should be LOG_ERR)

LOG_DBG("EBP", "Could not find or size META-INF/container.xml");
LOG_DBG("EBP", "Could not read META-INF/container.xml");
LOG_DBG("EBP", "Could not find valid rootfile in container.xml");
LOG_DBG("EBP", "Could not find content.opf in zip");
LOG_DBG("EBP", "Could not get size of content.opf");
LOG_DBG("EBP", "Could not read content.opf");
LOG_DBG("EBP", "Could not parse content.opf");
LOG_DBG("EBP", "Could not parse content.opf from cached bookMetadata for CSS files");

2. Memory Allocation and Setup Failures (should be LOG_ERR)

LOG_DBG("EBP", "Could not setup content.opf parser");
LOG_DBG("EBP", "Could not setup toc ncx parser");
LOG_DBG("EBP", "Could not allocate memory for toc ncx parser");
LOG_DBG("EBP", "Could not setup toc nav parser");
LOG_DBG("EBP", "Could not allocate memory for toc nav parser");

3. Cache Writing Failures (should be LOG_ERR)

LOG_DBG("EBP", "Could not begin writing cache");
LOG_DBG("EBP", "Could not begin writing content.opf pass");
LOG_DBG("EBP", "Could not end writing content.opf pass");
LOG_DBG("EBP", "Could not begin writing toc pass");
LOG_DBG("EBP", "Could not end writing toc pass");
LOG_DBG("EBP", "Could not end writing cache");
LOG_DBG("EBP", "Could not update mappings and sizes");

4. Data Processing Failures (should be LOG_ERR)

LOG_DBG("EBP", "Could not process all toc ncx data");
LOG_DBG("EBP", "Could not process all toc nav data");
LOG_DBG("EBP", "Could not create temp CSS file");
LOG_DBG("EBP", "Could not read CSS file: %s");
LOG_DBG("EBP", "Could not open temp CSS file for reading");

5. Runtime State Errors (should be LOG_ERR)

LOG_DBG("EBP", "Cannot generate cover BMP, cache not loaded");
LOG_DBG("EBP", "Cannot generate thumb BMP, cache not loaded");
LOG_DBG("EBP", "Failed to read item, empty href");
LOG_DBG("EBP", "Failed to read item %s");
LOG_DBG("EBP", "getSpineItem called but cache not loaded");
LOG_DBG("EBP", "getSpineItem index:%d is out of range");
LOG_DBG("EBP", "getTocItem called but cache not loaded");
LOG_DBG("EBP", "getTocItem index:%d is out of range");
LOG_DBG("EBP", "Section not found for TOC index %d");
LOG_DBG("EBP", "Section not found for text reference");

6. Serious Warnings (should be LOG_ERR)

LOG_DBG("EBP", "Warning: Could not parse any TOC format");
LOG_DBG("BMC", "Warning: Could not get size for spine item: %s");

Legitimate LOG_DBG Usage

These are correctly labeled as debug:

Timing information: "OPF pass completed in %lu ms"
State transitions: "Entering write mode", "Cache not found, building spine/TOC cache"
Success messages: "Successfully parsed content.opf", "Loaded ePub: %s"
Expected/optional conditions: "No ncx file specified", "Cache does not exist, no action needed"
Deliberately ignored errors: "Could not cleanup tmp files - ignoring"

Recommendation

Consider changing all LOG_DBG messages that indicate failure conditions (starting with "Could not", "Failed to", "Cannot", "Warning: Could not") to LOG_ERR, as these represent actual errors that users would want to see even at the lowest verbosity level (LOG_LEVEL=0).

ngxson · 2026-02-12T14:35:01Z

@jpirnay also as a safe-guard, I think we can define Serial to something else in one of the headers, so if any on-going PR try to log to Serial, an error will be triggered at compile time

#ifdef Serial // in case Serial is defined in Arduino header
#undef Serial
#endif
#define Serial static_assert(false && "use LOG_* macro instead");

jpirnay · 2026-02-12T15:27:56Z

@jpirnay also as a safe-guard, I think we can define Serial to something else in one of the headers, so if any on-going PR try to log to Serial, an error will be triggered at compile time
#ifdef Serial // in case Serial is defined in Arduino header
#undef Serial
#endif
#define Serial static_assert(false && "use LOG_* macro instead");

My naive attempt to do that in Logging.h like this:

...
HardwareSerial& logSerial = Serial;
....

#ifdef Serial  // in case Serial is defined in Arduino header
#undef Serial
#endif
#define Serial static_assert(false && "use LOG_* macro instead");

will cause framework libraries to fail compiling

ngxson · 2026-02-12T16:23:38Z

Something like this will work:

static HWCDC& logSerial = Serial;

...

class MySerialImpl : public Print {
 public:
  void begin(unsigned long baud) {
    logSerial.begin(baud);
  }
  __attribute__((deprecated("Use LOG_* macro instead")))
  size_t printf(const char * format, ...);
  size_t write(uint8_t b) override;
  size_t write(const uint8_t* buffer, size_t size) override;
  void flush() override;
  static MySerialImpl instance;
};

#define Serial MySerialImpl::instance

The version above is quite draft, please adapt it properly. Not sure if there are any __attribute__ that throw an error on compile, instead of a warning.

jpirnay · 2026-02-12T16:45:55Z

That works now as intended - thank you for your guidance

Profile	Verbosity	firmware.elf	Delta	firmware.bin	Delta
slim	suppressed	42.031.068	-	6.417.008	-
gh_release	err	42.057.876	26.808	6.435.824	18.816
gh_release_rc	err +inf	42.063.268	32.200	6.436.192	19.184
default	err +inf +dbg	42.201.592	170.524	6.473.536	56.528

ngxson · 2026-02-12T22:04:27Z

I managed to save ~10KB flash space via the last commit, please give it a try

osteotek · 2026-02-12T23:29:47Z

Having slim build is nice, but I think the default build with all debugging messages enabled will always be the common denominator

osteotek · 2026-02-12T23:38:43Z

Build from current master branch gives me firmware.bin size 6302416 bytes, default build from this PR's branch - 6294336 bytes. About 8kb size reduction, nice

…into feat-log

* master: feat: use pre-compressed HTML pages (crosspoint-reader#861) docs: Add requirement device be on when flashing (crosspoint-reader#877) fix: Account for `nbsp;` character as non-breaking space (crosspoint-reader#757) feat: Add central logging pragma (crosspoint-reader#843)

## Summary * Definition and use of a central LOG function, that can later be extended or completely be removed (for public use where debugging information may not be required) to save flash by suppressing the -DENABLE_SERIAL_LOG like in the slim branch * **What changes are included?** ## Additional Context * By using the central logger the usual: ``` #include <HardwareSerial.h> ... Serial.printf("[%lu] [WCS] Obfuscating/deobfuscating %zu bytes\n", millis(), data.size()); ``` would then become ``` #include <Logging.h> ... LOG_DBG("WCS", "Obfuscating/deobfuscating %zu bytes", data.size()); ``` You do have ``LOG_DBG`` for debug messages, ``LOG_ERR`` for error messages and ``LOG_INF`` for informational messages. Depending on the verbosity level defined (see below) soe of these message types will be suppressed/not-compiled. * The normal compilation (default) will create a firmware.elf file of 42.194.356 bytes, the same code via slim will create 42.024.048 bytes - 170.308 bytes less * Firmware.bin : 6.469.984 bytes for default, 6.418.672 bytes for slim - 51.312 bytes less ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _NO_ --------- Co-authored-by: Xuan Son Nguyen <[email protected]>

jpirnay added 2 commits February 11, 2026 21:27

Add logging pragma

af0403e

Add forgotten flag

4b9f0ea

ngxson reviewed Feb 11, 2026

View reviewed changes

lib/Logging/Logging.h Show resolved Hide resolved

ngxson reviewed Feb 11, 2026

View reviewed changes

lib/Logging/Logging.h Outdated Show resolved Hide resolved

Add additional logging levels

776cb4b

clang-format applied

8aefe9b

jpirnay requested a review from ngxson February 12, 2026 09:21

ngxson reviewed Feb 12, 2026

View reviewed changes

jpirnay added 2 commits February 12, 2026 16:14

Report critical failures with LOG_ERR

338e5f2

Change missed statements

7d3c093

Introduce deprecated warnings

f1638ee

jpirnay requested a review from ngxson February 12, 2026 16:46

save more flash space

caee5d5

ngxson added 3 commits February 12, 2026 23:05

clang format

a4e47ec

Merge branch 'master' into feat-log

1844d04

nits

a53f81e

ngxson previously approved these changes Feb 12, 2026

View reviewed changes

ngxson requested a review from a team February 12, 2026 22:13

jpirnay added 2 commits February 13, 2026 08:35

Merge branch 'master' of https://github.com/jpirnay/crosspoint-reader …

d4f1fa0

…into feat-log

Make branch compatible with master again

5e09074

jpirnay dismissed ngxson’s stale review via 5e09074 February 13, 2026 07:39

ngxson approved these changes Feb 13, 2026

View reviewed changes

osteotek approved these changes Feb 13, 2026

View reviewed changes

ngxson merged commit cb24947 into crosspoint-reader:master Feb 13, 2026
6 checks passed

jdk2pq mentioned this pull request Feb 15, 2026

feat: add bold toggle, portrait swap, and typographic enhancements #883

Closed

daveallie mentioned this pull request Feb 19, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

jpirnay deleted the feat-log branch March 1, 2026 09:41

Uh oh!

Conversation

jpirnay commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Additional Context

AI Usage

Uh oh!

Uri-Tauber commented Feb 11, 2026

Uh oh!

Uh oh!

Uh oh!

jpirnay commented Feb 12, 2026

Uh oh!

jpirnay commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngxson Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

ngxson commented Feb 12, 2026

Uh oh!

coderabbitai bot commented Feb 12, 2026

Issues Found

1. File I/O and Parsing Errors (should be LOG_ERR)

2. Memory Allocation and Setup Failures (should be LOG_ERR)

3. Cache Writing Failures (should be LOG_ERR)

4. Data Processing Failures (should be LOG_ERR)

5. Runtime State Errors (should be LOG_ERR)

6. Serious Warnings (should be LOG_ERR)

Legitimate LOG_DBG Usage

Recommendation

Uh oh!

ngxson commented Feb 12, 2026

Uh oh!

jpirnay commented Feb 12, 2026

Uh oh!

ngxson commented Feb 12, 2026

Uh oh!

jpirnay commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngxson commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osteotek commented Feb 12, 2026

Uh oh!

osteotek commented Feb 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jpirnay commented Feb 11, 2026 •

edited

Loading

jpirnay commented Feb 12, 2026 •

edited

Loading

jpirnay commented Feb 12, 2026 •

edited

Loading

ngxson commented Feb 12, 2026 •

edited

Loading