phoneapi: send status message config - #11129
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPhoneAPI now includes the StatusMessage module configuration in ChangesStatusMessage config streaming
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Test as want_config test
participant PhoneAPI as PhoneAPI::getFromRadio
participant FromRadio
Test->>PhoneAPI: send want_config request
PhoneAPI->>FromRadio: emit ModuleConfig_statusmessage
FromRadio-->>Test: decode node_status
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
…clude-statusmessage-config
00a4de9 to
91915a0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/test_stream_api/test_main.cpp (1)
494-494: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider explicitly null-terminating the string buffer.
While
"Ready"easily fits andmoduleConfigstarts zero-initialized,strncpystops padding at the specified limit and leaves the byte at[sizeof(...) - 1]completely untouched. Explicitly setting the final byte to\0is a good defensive practice to prevent non-null-terminated string bugs if this test code is ever modified to use longer strings or uninitialized buffers. This also aligns with the upstream idiom used inStatusMessageModule.cpp.♻️ Proposed refactor
- strncpy(moduleConfig.statusmessage.node_status, "Ready", sizeof(moduleConfig.statusmessage.node_status) - 1); + strncpy(moduleConfig.statusmessage.node_status, "Ready", sizeof(moduleConfig.statusmessage.node_status) - 1); + moduleConfig.statusmessage.node_status[sizeof(moduleConfig.statusmessage.node_status) - 1] = '\0';🤖 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 `@test/test_stream_api/test_main.cpp` at line 494, Explicitly set the final byte of moduleConfig.statusmessage.node_status to '\0' immediately after the strncpy call, using the buffer’s last index to guarantee null termination even if the source string or initialization changes.
🤖 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.
Nitpick comments:
In `@test/test_stream_api/test_main.cpp`:
- Line 494: Explicitly set the final byte of
moduleConfig.statusmessage.node_status to '\0' immediately after the strncpy
call, using the buffer’s last index to guarantee null termination even if the
source string or initialization changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 15be2815-21d1-4ab1-af9e-9658dec55d1e
📒 Files selected for processing (2)
src/mesh/PhoneAPI.cpptest/test_stream_api/test_main.cpp
⚡ Try this PR in the Web FlasherWarning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (30)
Build artifacts expire on 2026-08-20. Updated for |
Summary
Why
PhoneAPI iterates the StatusMessage tag but previously treated it as unhandled, so clients did not receive the local node status on their initial config download.
Fixes #11127
Validation
Summary by CodeRabbit