logging: audit log strings for terseness, reclaiming ~6.8 KB of string data

[jp-bennett]

Audits all 3,161 LOG_* format strings in src/ and rewrites the verbose ones to terser equivalents, reducing total log-string data from 121,628 to 114,834 bytes — 6,794 bytes saved (5.6%), with no strings dropped. A typical firmware image reclaims roughly 3–5 KB, since the biggest trims are in code nearly every target compiles (NodeDB, Router, AdminModule, MeshService, GPS, PhoneAPI, sensors).

What was changed, in passes:

• Mechanical pass (all of src/): strip trailing ./!/... and literal \n (the logger appends a newline itself), Error:/Warning: prefixes (the level tag already conveys it), and redundant "successfully".
• Judgment pass (top ~40 byte-heaviest files): rewrite wordy prose — "Failed to initialize X" → "X init failed", "Attempting to send" → "Send", dropped articles and filler, house abbreviations (config, init, msg, temp, BT).
• Global pass: "Unable to / Could not / Cannot" → "Can't".
• clang-format rewrap of lines whose length changed, using the pinned style (.trunk/configs/.clang-format, clang-format 16).

Deliberately preserved: every printf format specifier and argument list byte-for-byte (verified by an automated pre/post specifier-sequence check on every call), greppable tokens (chip/sensor/function/state names, key=value field names), the 0x%08x node/packet-ID convention, strings the external hardware-test harness greps for, the boot-logo banner's trailing newline and bare "." progress ticks, and identical duplicate literals (so linker string dedup still applies). Non-log strings (UI text, NMEA/UBX, MQTT topics, CSV headers) are untouched.

🤝 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)

Tested via the native (portduino) target: full compile is green, and the native test suite is GREEN — 44/44 suites, 352/352 test cases, all CLEAN (./bin/run-tests.sh). An automated check verified the format-specifier sequence of every LOG_* call is unchanged. No device-specific behavior is affected beyond log text; community testing on real hardware is welcome but the risk surface is log prose only.

Summary by CodeRabbit

• Style

  • Standardized diagnostic messages across power, GPS, displays, networking, telemetry, storage, Bluetooth, and platform components.
  • Improved clarity and consistency for errors, warnings, status updates, ACK/NACK responses, and wake events.
  • Removed unnecessary punctuation, prefixes, and embedded newline characters from messages.

• Bug Fixes

  • None. Runtime behavior, control flow, data handling, and public interfaces remain unchanged.

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fbb41c0a-da83-48a3-8009-6bbda6b8119f

📥 Commits

Reviewing files that changed from the base of the PR and between 05642ba and cf97e71.

📒 Files selected for processing (11)

• src/gps/RTC.cpp
• src/mesh/NextHopRouter.cpp
• src/mesh/NodeDB.cpp
• src/mesh/Router.cpp
• src/modules/AdminModule.cpp
• src/modules/CannedMessageModule.cpp
• src/modules/Telemetry/HostMetrics.cpp
• src/modules/Telemetry/Sensor/SCD30Sensor.cpp
• src/modules/Telemetry/Sensor/SCD4XSensor.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• src/platform/extra_variants/t_deck_pro/variant.cpp

🚧 Files skipped from review as they are similar to previous changes (11)

• src/platform/extra_variants/t_deck_pro/variant.cpp
• src/modules/Telemetry/HostMetrics.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• src/modules/Telemetry/Sensor/SCD30Sensor.cpp
• src/modules/CannedMessageModule.cpp
• src/mesh/Router.cpp
• src/mesh/NextHopRouter.cpp
• src/modules/AdminModule.cpp
• src/gps/RTC.cpp
• src/mesh/NodeDB.cpp
• src/modules/Telemetry/Sensor/SCD4XSensor.cpp

---

📝 Walkthrough

Walkthrough

This PR standardizes diagnostic log wording and punctuation across firmware subsystems. It updates messages in power, GPS, graphics, mesh, telemetry, platform, security, networking, and sleep code. Runtime behavior and public interfaces remain unchanged.

Changes

Diagnostic log cleanup

Layer / File(s)	Summary
Message wording normalization src/**/*.cpp, src/**/*.h	Diagnostic messages are shortened, reworded, and standardized. Trailing punctuation, redundant prefixes, embedded newlines, and inconsistent terms are removed across affected components.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the logging-string audit and its primary outcome: shorter log strings that reclaim approximately 6.8 KB.
Description check	✅ Passed	The description explains the scope, methods, preserved behavior, validation results, and hardware-testing status, and it follows the repository template sufficiently.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/meshtastic-logging-terse-2wgqmh

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 17

🧹 Nitpick comments (1)

src/main.cpp (1)

1370-1373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep changed rationale comments within the two-line limit.

Both comments preserve useful security rationale, but both exceed the repository comment-length rule. Compress each block without removing the non-obvious reason.

• src/main.cpp#L1370-L1373: reduce the four-line lockdown revert explanation to at most two lines.
• src/modules/AdminModule.cpp#L1180-L1182: reduce the three-line admin-key rotation explanation to at most two lines.

As per coding guidelines, code comments should normally be one or two lines and should not use explanatory blocks.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main.cpp` around lines 1370 - 1373, Compress the changed rationale
comment in src/main.cpp lines 1370-1373 to no more than two lines while
retaining that the failed revert leaves the DEK file present, keeping the device
in lockdown and allowing retry. Also compress the changed admin-key rotation
comment in src/modules/AdminModule.cpp lines 1180-1182 to no more than two lines
while preserving its non-obvious security rationale; no code changes are needed.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/gps/RTC.cpp`:
- Line 141: Update the four RTC warning calls around the relevant probe and
write paths to retain a model-specific prefix identifying the RTC family, while
shortening the remaining “RTC not found” text. Preserve each warning’s existing
context and formatting arguments, including the address where currently
provided.

In `@src/main.cpp`:
- Line 457: Update the startup banner LOG_INFO call to remove the leading
embedded newline characters, while preserving the banner text and relying on
LOG_INFO to add the record newline.

In `@src/mesh/http/WebServer.cpp`:
- Line 167: Remove the LOG_DEBUG("") call from the certificate-generation flow,
while preserving the surrounding yield() and watchdog handling.

In `@src/mesh/NextHopRouter.cpp`:
- Line 70: Use canonical 32-bit node ID formatting in both affected logs: update
the NextHopRouter log at src/mesh/NextHopRouter.cpp lines 70-70 to format p->to
as 0x%08x, and update the NodeDB log at src/mesh/NodeDB.cpp lines 4334-4334 to
format both oldNodeNum and newNodeNum as 0x%08x, preserving leading zeroes.
- Line 506: Update the retransmission log in the relevant NextHopRouter flow to
remove the trailing colon and following space from the LOG_DEBUG message,
leaving the delay value as the complete log line.

In `@src/mesh/Router.cpp`:
- Line 339: Update the LOG_ERROR message in the packet-receipt handling to use
clear invalid-destination wording, such as identifying the value as to=0 or
destination 0; leave the surrounding behavior unchanged.

In `@src/mesh/wifi/WiFiAPClient.cpp`:
- Line 158: Update the LOG_ERROR call in the mDNS setup failure path to use the
message "mDNS setup failed" instead of embedding the redundant error-level word.

In `@src/modules/AdminModule.cpp`:
- Line 209: Update the LOG_INFO call in the admin validation path to format the
32-bit node ID mp.from using the canonical 0x%08x format, preserving the
existing message text and behavior.

In `@src/modules/CannedMessageModule.cpp`:
- Line 1114: Update the LOG_DEBUG call in the router-like role branch to format
dest as a zero-padded 32-bit hexadecimal value with a 0x prefix, using 0x%08x
instead of %x.

In `@src/modules/StoreForwardModule.cpp`:
- Line 633: Remove the empty LOG_INFO("") call in the StoreForwardModule
diagnostic flow, leaving the surrounding PSRAM diagnostic logging unchanged.

In `@src/modules/Telemetry/Sensor/SCD30Sensor.cpp`:
- Line 227: Update the error log in getASC() for the
scd30.getAutoCalibrationStatus(...) call to describe the failed read operation,
using wording such as “Can't get ASC status” or “Can't read ASC state” instead
of “Can't send command”.
- Line 105: Remove the embedded “Error:” text from the LOG_ERROR messages in
SCD30Sensor.cpp at lines 105, 126, 271, 291, 310, 328, and 345, preserving their
existing failure details. In HostMetrics.cpp line 54, replace the
error-level-prefixed message with a failure description such as “Can't decode
HostMetrics module”.

In `@src/modules/Telemetry/Sensor/SCD4XSensor.cpp`:
- Line 511: Update the failure logs in getAmbientPressure() and
setAmbientPressure() to say “ambient pressure” instead of “altitude”, while
preserving the existing error code output.
- Line 47: Update the LOG_ERROR messages in the SCD4XSensor error paths,
including the powerUp() call and the other referenced locations, to remove
redundant “Error” or “Error: %u” wording. Use “Can’t …” or “… failed” phrasing,
and format numeric error codes as “rc=%u”.

In `@src/platform/esp32/MeshtasticOTA.cpp`:
- Line 112: Update the warning message in the OTA partition switching code to
spell “partition” correctly and use a single separator before the reason value,
preserving the existing result argument and LOG_WARN call.

In `@src/platform/extra_variants/t_deck_pro/variant.cpp`:
- Line 122: Update the diagnostic text in the CST3530 response handling to
exactly “CST3530 no response”, removing the stray tilde and correcting the
grammar.

In `@src/platform/nrf52/main-nrf52.cpp`:
- Line 537: Remove the empty LOG_DEBUG("") call in the system-off failure
reporting path of main-nrf52.cpp; do not emit a blank log record, unless
replacing it with a meaningful diagnostic for the fallback state is required.

---

Nitpick comments:
In `@src/main.cpp`:
- Around line 1370-1373: Compress the changed rationale comment in src/main.cpp
lines 1370-1373 to no more than two lines while retaining that the failed revert
leaves the DEK file present, keeping the device in lockdown and allowing retry.
Also compress the changed admin-key rotation comment in
src/modules/AdminModule.cpp lines 1180-1182 to no more than two lines while
preserving its non-obvious security rationale; no code changes are needed.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9748fe84-c5ae-4bc8-a2e3-736bba0002b0

📥 Commits

Reviewing files that changed from the base of the PR and between 5f198c4 and f8c2841.

📒 Files selected for processing (87)

• src/AmbientLightingThread.h
• src/Power.cpp
• src/PowerFSM.cpp
• src/SafeFile.cpp
• src/gps/GPS.cpp
• src/gps/RTC.cpp
• src/graphics/Screen.cpp
• src/graphics/TFTDisplay.cpp
• src/graphics/draw/MenuHandler.cpp
• src/graphics/eink/Drivers/EInk.cpp
• src/graphics/niche/Drivers/EInk/EInk.cpp
• src/graphics/niche/Utils/FlashData.h
• src/graphics/tftSetup.cpp
• src/input/ButtonThread.cpp
• src/main.cpp
• src/mesh/CryptoEngine.cpp
• src/mesh/IndicatorSerial.cpp
• src/mesh/LR11x0Interface.cpp
• src/mesh/LR20x0Interface.cpp
• src/mesh/MemoryPool.h
• src/mesh/MeshModule.cpp
• src/mesh/MeshService.cpp
• src/mesh/NextHopRouter.cpp
• src/mesh/NodeDB.cpp
• src/mesh/PacketHistory.cpp
• src/mesh/PhoneAPI.cpp
• src/mesh/ProtobufModule.h
• src/mesh/RF95Interface.cpp
• src/mesh/RadioInterface.cpp
• src/mesh/RadioLibInterface.cpp
• src/mesh/Router.cpp
• src/mesh/SX126xInterface.cpp
• src/mesh/WarmNodeStore.cpp
• src/mesh/api/PacketAPI.cpp
• src/mesh/eth/ethCert.cpp
• src/mesh/eth/ethOTA.cpp
• src/mesh/http/WebServer.cpp
• src/mesh/raspihttp/PiWebServer.cpp
• src/mesh/wifi/WiFiAPClient.cpp
• src/modules/AdminModule.cpp
• src/modules/CannedMessageModule.cpp
• src/modules/ExternalNotificationModule.cpp
• src/modules/HopScalingModule.cpp
• src/modules/KeyVerificationModule.cpp
• src/modules/MeshBeaconModule.cpp
• src/modules/NeighborInfoModule.cpp
• src/modules/NodeInfoModule.cpp
• src/modules/PositionModule.cpp
• src/modules/PowerStressModule.cpp
• src/modules/RangeTestModule.cpp
• src/modules/RemoteHardwareModule.cpp
• src/modules/StoreForwardModule.cpp
• src/modules/Telemetry/AirQualityTelemetry.cpp
• src/modules/Telemetry/DeviceTelemetry.cpp
• src/modules/Telemetry/EnvironmentTelemetry.cpp
• src/modules/Telemetry/HealthTelemetry.cpp
• src/modules/Telemetry/HostMetrics.cpp
• src/modules/Telemetry/PowerTelemetry.cpp
• src/modules/Telemetry/Sensor/BME680Sensor.cpp
• src/modules/Telemetry/Sensor/DS248XSensor.cpp
• src/modules/Telemetry/Sensor/NAU7802Sensor.cpp
• src/modules/Telemetry/Sensor/RCWL9620Sensor.cpp
• src/modules/Telemetry/Sensor/SCD30Sensor.cpp
• src/modules/Telemetry/Sensor/SCD4XSensor.cpp
• src/modules/Telemetry/Sensor/SEN5XSensor.cpp
• src/modules/TraceRouteModule.cpp
• src/modules/TrafficManagementModule.cpp
• src/modules/esp32/PaxcounterModule.cpp
• src/mqtt/MQTT.cpp
• src/nimble/NimbleBluetooth.cpp
• src/platform/esp32/ESP32CryptoEngine.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• src/platform/esp32/main-esp32.cpp
• src/platform/extra_variants/t5s3_epaper/variant.cpp
• src/platform/extra_variants/t_deck_pro/variant.cpp
• src/platform/nrf52/NRF52Bluetooth.cpp
• src/platform/nrf52/main-nrf52.cpp
• src/platform/nrf54l15/InternalFileSystem.cpp
• src/platform/nrf54l15/NRF54L15Bluetooth.cpp
• src/platform/portduino/GpsdSerial.cpp
• src/platform/portduino/SimRadio.cpp
• src/platform/portduino/USBHal.h
• src/platform/rp2xx0/main-rp2xx0.cpp
• src/platform/stm32wl/main-stm32wl.cpp
• src/security/EncryptedStorage.cpp
• src/serialization/MeshPacketSerializer.cpp
• src/sleep.cpp

[thebentern]

Went through all 17 inline findings. 12 were still live against the current branch, 4 were already resolved by the two follow-up commits, and 1 I've left alone deliberately.

Fixed

Node/packet ID convention (copilot-instructions.md: "Format node IDs and packet IDs as 0x%08x")

• NextHopRouter.cpp:70 — p->to → 0x%08x. p->next_hop stays 0x%x: it's the last-byte relay hint, not a NodeNum, which matches lines 269/284/287 in the same file.
• NodeDB.cpp:4333 — oldNodeNum/newNodeNum %u → 0x%08x.
• AdminModule.cpp:209 and CannedMessageModule.cpp:1114.
• I also converted the sibling in each if/else pair (AdminModule.cpp:212, CannedMessageModule.cpp:1111). Those were pre-existing %x and untouched by this PR, but leaving one branch on %x and the other on 0x%08x three lines apart would read as an oversight.

Message names the wrong operation — these were real bugs, good catches:

• SCD4XSensor.cpp:511 and :580 are getAmbientPressure() / setAmbientPressure() but logged "altitude". Now "ambient pressure". (Lines 487/540 genuinely are the altitude accessors and keep their wording.)
• SCD30Sensor.cpp:227 is getASC() but logged "Can't send command" → "Can't get ASC status". Line 207 in setASC() really is a write, so it keeps the original.

Redundant level text in LOG_ERROR

• . Error: %u → , rc=%u at 15 sites, matching the existing rc=%d house style. All of them are confined to the two Sensirion files.
• Error executing X() → X() failed; Error performing FRC → FRC failed.
• HostMetrics.cpp:54 — "Error decoding" → "Can't decode".

Typos and wording

• MeshtasticOTA.cpp:112 — "OTA partiton.  (Reason %d)" → "OTA partition (reason %d)".
• t_deck_pro/variant.cpp:122 — "CST3530 not response ~" → "CST3530 no response".
• Router.cpp:339 — "Packet received with to: of 0" → "with to=0".
• NextHopRouter.cpp:506 — dropped the dangling ": ".

RTC — worth a note, because the stated rationale doesn't hold but the underlying problem does. The four sites sit in mutually-exclusive #ifdef branches, so no build ever compiles two RTC families and the family can't actually be ambiguous. What is ambiguous is read vs. set: readFromRTC() and perhapsSetRTC() each emit a byte-identical literal, which the linker then dedups into one string. Split into "RTC read: not found" / "RTC set: not found".

Not changed

• main.cpp:457 (banner \n\n) — deliberate. The leading blank lines separate the boot logo from preceding output, and 33de80d restored them after the terseness pass over-trimmed them. The PR description lists the banner as intentionally preserved.
• WebServer.cpp:167, StoreForwardModule.cpp:633, main-nrf52.cpp:537 (LOG_DEBUG("")) — already fixed; 33de80d restored all three to ".". These findings were raised against the intermediate state.
• Nitpick on comment length at main.cpp:1370-1373 / AdminModule.cpp:1180-1182 — those comments are pre-existing and untouched by this PR; they show up only as diff context. Compressing security rationale isn't something I want to fold into a log-string audit.

Verification

trunk fmt clean. An automated check over all 37 changed lines confirms each printf specifier sequence is byte-identical except for the 6 intentional %x/%u → %08x node-ID widenings, all on uint32_t arguments.

Built t-deck-pro (esp32-s3) locally, which compiles all 11 touched files — including the four the native target can't reach (both Sensirion sensors, the ESP32 OTA path, and the T-Deck Pro variant). SUCCESS, no new warnings. The native target can't be built on macOS (portduino needs glibc's argp.h), so I used the cross-build instead of run-tests.sh.