logging: audit log strings for terseness, reclaiming ~6.8 KB of string data - #11374
logging: audit log strings for terseness, reclaiming ~6.8 KB of string data#11374jp-bennett wants to merge 7 commits into
Conversation
…ly' from log strings The logger already appends a newline and prints the level tag, so trailing '.', '!', '...', literal \n, and 'Error:'/'Warning:' prefixes inside format strings are wasted flash bytes. Same for 'successfully' (the affirmative form already implies it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
… system code Rewrite wordy log messages to terser equivalents - drop filler words (articles, 'attempting', 'due to', 'please'), use 'Can't X'/'X failed' phrasing, and abbreviate where the codebase already does (config, init, msg, BT). Format specifiers and argument lists are unchanged; distinctive greppable tokens are preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
Same terseness pass: drop filler, 'Can't X'/'X failed' phrasing, common abbreviations (temp, msg). Specifiers, arguments, and sensor-name prefixes unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
Same terseness pass over NodeDB, Router, MeshService, PhoneAPI, RadioInterface, NextHopRouter, and PacketHistory: 'X failed'/'Can't X' phrasing, imperative verbs, dropped filler. Specifiers and arguments unchanged; duplicate literals kept identical to preserve linker string dedup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis 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. ChangesDiagnostic log cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
src/main.cpp (1)
1370-1373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep 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
📒 Files selected for processing (87)
src/AmbientLightingThread.hsrc/Power.cppsrc/PowerFSM.cppsrc/SafeFile.cppsrc/gps/GPS.cppsrc/gps/RTC.cppsrc/graphics/Screen.cppsrc/graphics/TFTDisplay.cppsrc/graphics/draw/MenuHandler.cppsrc/graphics/eink/Drivers/EInk.cppsrc/graphics/niche/Drivers/EInk/EInk.cppsrc/graphics/niche/Utils/FlashData.hsrc/graphics/tftSetup.cppsrc/input/ButtonThread.cppsrc/main.cppsrc/mesh/CryptoEngine.cppsrc/mesh/IndicatorSerial.cppsrc/mesh/LR11x0Interface.cppsrc/mesh/LR20x0Interface.cppsrc/mesh/MemoryPool.hsrc/mesh/MeshModule.cppsrc/mesh/MeshService.cppsrc/mesh/NextHopRouter.cppsrc/mesh/NodeDB.cppsrc/mesh/PacketHistory.cppsrc/mesh/PhoneAPI.cppsrc/mesh/ProtobufModule.hsrc/mesh/RF95Interface.cppsrc/mesh/RadioInterface.cppsrc/mesh/RadioLibInterface.cppsrc/mesh/Router.cppsrc/mesh/SX126xInterface.cppsrc/mesh/WarmNodeStore.cppsrc/mesh/api/PacketAPI.cppsrc/mesh/eth/ethCert.cppsrc/mesh/eth/ethOTA.cppsrc/mesh/http/WebServer.cppsrc/mesh/raspihttp/PiWebServer.cppsrc/mesh/wifi/WiFiAPClient.cppsrc/modules/AdminModule.cppsrc/modules/CannedMessageModule.cppsrc/modules/ExternalNotificationModule.cppsrc/modules/HopScalingModule.cppsrc/modules/KeyVerificationModule.cppsrc/modules/MeshBeaconModule.cppsrc/modules/NeighborInfoModule.cppsrc/modules/NodeInfoModule.cppsrc/modules/PositionModule.cppsrc/modules/PowerStressModule.cppsrc/modules/RangeTestModule.cppsrc/modules/RemoteHardwareModule.cppsrc/modules/StoreForwardModule.cppsrc/modules/Telemetry/AirQualityTelemetry.cppsrc/modules/Telemetry/DeviceTelemetry.cppsrc/modules/Telemetry/EnvironmentTelemetry.cppsrc/modules/Telemetry/HealthTelemetry.cppsrc/modules/Telemetry/HostMetrics.cppsrc/modules/Telemetry/PowerTelemetry.cppsrc/modules/Telemetry/Sensor/BME680Sensor.cppsrc/modules/Telemetry/Sensor/DS248XSensor.cppsrc/modules/Telemetry/Sensor/NAU7802Sensor.cppsrc/modules/Telemetry/Sensor/RCWL9620Sensor.cppsrc/modules/Telemetry/Sensor/SCD30Sensor.cppsrc/modules/Telemetry/Sensor/SCD4XSensor.cppsrc/modules/Telemetry/Sensor/SEN5XSensor.cppsrc/modules/TraceRouteModule.cppsrc/modules/TrafficManagementModule.cppsrc/modules/esp32/PaxcounterModule.cppsrc/mqtt/MQTT.cppsrc/nimble/NimbleBluetooth.cppsrc/platform/esp32/ESP32CryptoEngine.cppsrc/platform/esp32/MeshtasticOTA.cppsrc/platform/esp32/main-esp32.cppsrc/platform/extra_variants/t5s3_epaper/variant.cppsrc/platform/extra_variants/t_deck_pro/variant.cppsrc/platform/nrf52/NRF52Bluetooth.cppsrc/platform/nrf52/main-nrf52.cppsrc/platform/nrf54l15/InternalFileSystem.cppsrc/platform/nrf54l15/NRF54L15Bluetooth.cppsrc/platform/portduino/GpsdSerial.cppsrc/platform/portduino/SimRadio.cppsrc/platform/portduino/USBHal.hsrc/platform/rp2xx0/main-rp2xx0.cppsrc/platform/stm32wl/main-stm32wl.cppsrc/security/EncryptedStorage.cppsrc/serialization/MeshPacketSerializer.cppsrc/sleep.cpp
| return RTCSetResultSuccess; | ||
| } else { | ||
| LOG_WARN("RTC not found (found address 0x%02X)", rtc_found.address); | ||
| LOG_WARN("RTC not found (addr 0x%02X)", rtc_found.address); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Retain the RTC model in missing-device warnings.
The four edited warnings now use the same RTC not found literal. The surrounding code handles different RTC families, but the new messages no longer identify which probe or write path failed. Keep a model-specific prefix while shortening the remaining text.
Also applies to: 187-187, 336-336, 361-361
🤖 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/gps/RTC.cpp` at 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.
| serialSinceMsec = millis(); | ||
|
|
||
| LOG_INFO("\n\n//\\ E S H T /\\ S T / C\n"); | ||
| LOG_INFO("\n\n//\\ E S H T /\\ S T / C"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the embedded newlines from the startup banner.
LOG_INFO already adds the record newline. The \n\n sequence still emits extra blank lines and conflicts with the cleanup objective.
Proposed fix
- LOG_INFO("\n\n//\\ E S H T /\\ S T / C");
+ LOG_INFO("//\\ E S H T /\\ S T / C");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_INFO("\n\n//\\ E S H T /\\ S T / C"); | |
| LOG_INFO("//\\ E S H T /\\ S T / C"); |
🤖 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` at 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.
| if ((millis() / 500) % 2) { | ||
| if (runLoop) { | ||
| LOG_DEBUG("."); | ||
| LOG_DEBUG(""); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Remove the empty debug record.
LOG_DEBUG("") still emits a blank debug line during certificate generation. It adds log I/O without diagnostic content. Remove the call and keep yield() and watchdog handling.
Proposed fix
- LOG_DEBUG("");
yield();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_DEBUG(""); |
🤖 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/mesh/http/WebServer.cpp` at line 167, Remove the LOG_DEBUG("") call from
the certificate-generation flow, while preserving the surrounding yield() and
watchdog handling.
|
|
||
| p->next_hop = getNextHop(p->to, p->relay_node).value_or(NO_NEXT_HOP_PREFERENCE); // set the next hop | ||
| LOG_DEBUG("Setting next hop for packet with dest %x to %x", p->to, p->next_hop); | ||
| LOG_DEBUG("Set next hop for dest %x to %x", p->to, p->next_hop); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use canonical formatting for all changed 32-bit node IDs.
The affected logs use inconsistent formats. Use 0x%08x at both sites to preserve leading zeroes and match firmware diagnostics.
src/mesh/NextHopRouter.cpp#L70-L70: formatp->toas0x%08x.src/mesh/NodeDB.cpp#L4334-L4334: formatoldNodeNumandnewNodeNumas0x%08x.
As per coding guidelines, format 32-bit node and packet IDs as 0x%08x.
📍 Affects 2 files
src/mesh/NextHopRouter.cpp#L70-L70(this comment)src/mesh/NodeDB.cpp#L4334-L4334
🤖 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/mesh/NextHopRouter.cpp` at 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.
Source: Coding guidelines
| auto d = iface->getRetransmissionMsec(pending->packet); | ||
| pending->nextTxMsec = millis() + d; | ||
| LOG_DEBUG("Setting next retransmission in %u msecs: ", d); | ||
| LOG_DEBUG("Next retransmission in %u msecs: ", d); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the trailing colon from the retransmission log.
LOG_DEBUG terminates the line, and printPacket runs separately. The colon and trailing space do not join the packet text.
Proposed fix
- LOG_DEBUG("Next retransmission in %u msecs: ", d);
+ LOG_DEBUG("Next retransmission in %u msecs", d);This follows the PR objective to remove redundant trailing punctuation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_DEBUG("Next retransmission in %u msecs: ", d); | |
| LOG_DEBUG("Next retransmission in %u msecs", d); |
🤖 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/mesh/NextHopRouter.cpp` at 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.
| LOG_INFO("%s: Found SCD41", sensorName); | ||
| if (!powerUp()) { | ||
| LOG_ERROR("%s: Error trying to execute powerUp()", sensorName); | ||
| LOG_ERROR("%s: Error executing powerUp()", sensorName); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove redundant error-level text from LOG_ERROR messages.
These changed strings still include Error or Error: %u, although LOG_ERROR already supplies the severity. Use Can't ... or ... failed, and keep numeric codes as rc=%u.
Proposed wording pattern
- LOG_ERROR("%s: Error executing powerUp()", sensorName);
+ LOG_ERROR("%s: powerUp() failed", sensorName);
- LOG_ERROR("%s: Can't get temp offset. Error: %u", sensorName, error);
+ LOG_ERROR("%s: Can't get temp offset, rc=%u", sensorName, error);Also applies to: 188-188, 427-427, 440-440, 450-450, 456-456, 487-487, 540-540, 587-587, 618-618, 660-660, 693-693
🤖 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/modules/Telemetry/Sensor/SCD4XSensor.cpp` at 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”.
|
|
||
| if (error != SCD4X_NO_ERROR) { | ||
| LOG_ERROR("%s: Unable to get altitude. Error code: %u", sensorName, error); | ||
| LOG_ERROR("%s: Can't get altitude. Error: %u", sensorName, error); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use ambient-pressure wording in these failure logs.
Line 511 and Line 580 are in getAmbientPressure() and setAmbientPressure(), but both messages say “altitude”. The messages identify the wrong operation. Use “ambient pressure” and retain the error code.
Proposed log fix
- LOG_ERROR("%s: Can't get altitude. Error: %u", sensorName, error);
+ LOG_ERROR("%s: Can't get ambient pressure, rc=%u", sensorName, error);
- LOG_ERROR("%s: Can't set altitude. Error: %u", sensorName, error);
+ LOG_ERROR("%s: Can't set ambient pressure, rc=%u", sensorName, error);Also applies to: 580-580
🤖 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/modules/Telemetry/Sensor/SCD4XSensor.cpp` at line 511, Update the failure
logs in getAmbientPressure() and setAmbientPressure() to say “ambient pressure”
instead of “altitude”, while preserving the existing error code output.
| // Partition and app checks should now be done in the AdminModule before this is called | ||
| if (result != ESP_OK) { | ||
| LOG_WARN("Unable to switch to OTA partiton. (Reason %d)", result); | ||
| LOG_WARN("Can't switch to OTA partiton. (Reason %d)", result); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the OTA partition log typo.
The message says partiton and uses redundant punctuation. Use OTA partition and one reason separator.
Proposed wording fix
- LOG_WARN("Can't switch to OTA partiton. (Reason %d)", result);
+ LOG_WARN("Can't switch to OTA partition (reason %d)", result);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_WARN("Can't switch to OTA partiton. (Reason %d)", result); | |
| LOG_WARN("Can't switch to OTA partition (reason %d)", result); |
🤖 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/platform/esp32/MeshtasticOTA.cpp` at 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.
| break; | ||
| } else { | ||
| LOG_DEBUG("CST3530 not response ~!"); | ||
| LOG_DEBUG("CST3530 not response ~"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the final diagnostic text.
Line [122] logs CST3530 not response ~. The ~ is stray, and not response is ungrammatical. Use CST3530 no response.
Proposed fix
- LOG_DEBUG("CST3530 not response ~");
+ LOG_DEBUG("CST3530 no response");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_DEBUG("CST3530 not response ~"); | |
| LOG_DEBUG("CST3530 no response"); |
🤖 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/platform/extra_variants/t_deck_pro/variant.cpp` at line 122, Update the
diagnostic text in the CST3530 response handling to exactly “CST3530 no
response”, removing the stray tilde and correcting the grammar.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
jp-bennett
changed the title
The terseness pass over-trimmed: the Meshtastic ASCII boot logo kept its blank line via a trailing \n, and three bare "." progress ticks were reduced to empty strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
2 participants
Audits all 3,161
LOG_*format strings insrc/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:
src/): strip trailing./!/...and literal\n(the logger appends a newline itself),Error:/Warning:prefixes (the level tag already conveys it), and redundant "successfully"..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%08xnode/packet-ID convention, strings the external hardware-test harness greps for, and identical duplicate literals (so linker string dedup still applies). Non-log strings (UI text, NMEA/UBX, MQTT topics, CSV headers) are untouched.🤝 Attestations
Tested via the native (portduino) target: full compile is green and the changes are string-literal-only. An automated check verified the format-specifier sequence of every
LOG_*call is unchanged. Native unit-test suite run is in progress; 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