perf(nodedb): bind encode-loop entries by const reference

[vidplace7]

What

cppcheck's iterateByValue check flagged five range-for loops in the NodeDatabase nanopb encode callbacks:

src/mesh/NodeDB.cpp:297,323,349,375   [low:performance] Range variable 'item' should be declared as const reference
src/mesh/NodeDBLegacyMigration.cpp:29 [low:performance] Range variable 'item' should be declared as const reference

All five do the same thing: copy an entire protobuf entry off the vector purely to hand its address to pb_encode_submessage(), which takes a const void *. Binding by const auto & removes the copy with no behavioral change.

Per encode pass this drops one copy per node of meshtastic_NodePositionEntry, NodeTelemetryEntry, NodeStatusEntry, NodeEnvironmentEntry and NodeInfoLite_Legacy. These callbacks run on every nodes.proto save.

Note for reviewers

The nodes_tag loop at src/mesh/NodeDB.cpp:265 is deliberately left by-value and is not part of this change. It is not a redundant copy — it mutates item.snr_q4 / item.snr into the on-disk quantized form before encoding, so it is a working copy. cppcheck correctly does not flag it; converting it would either fail to compile or corrupt the in-memory node list.

Three unrelated pre-existing cppcheck findings remain in NodeDB.cpp (functionStatic on getNodeId, constVariable at :3658, constVariablePointer at :3891). Left alone to keep this diff scoped to the iterateByValue set.

Testing

Native suites on macOS (native-macos), both green:

• test_nodedb_v25_roundtrip — 8 tests, 0 failures — exercises the four NodeDB.cpp callbacks
• test_nodedb_legacy_migration — 9 tests, 0 failures — exercises the NodeDBLegacyMigration.cpp callback

cppcheck 2.20.0 re-run on both files: no iterateByValue findings remain. (Invoked directly — a local pio check silently downgrades to cppcheck 2.11 and is not a valid reproduction of CI.)

trunk pre-commit hook clean on both files.

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

No on-device testing was performed — verification was native test suites plus static analysis only. The change is a compile-time binding change in serialization paths shared by all targets, so it is not variant-specific, but a sanity check that a real device still saves and reloads its NodeDB across a reboot would be welcome.

Summary by CodeRabbit

• Performance
  • Reduced unnecessary data copying during node database encoding.
  • Preserved existing encoding behavior and error handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 19b87285-dfad-40a4-a409-b6d8db2c8b5f

📥 Commits

Reviewing files that changed from the base of the PR and between f143c45 and 5bf8b34.

📒 Files selected for processing (1)

• src/mesh/NodeDB.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The protobuf encoding loops in current and legacy node database code now iterate entries by const reference. Encoding behavior, error handling, and control flow remain unchanged.

Changes

Encoding iteration updates

Layer / File(s)	Summary
Use const references in encoders src/mesh/NodeDB.cpp, src/mesh/NodeDBLegacyMigration.cpp	Current position, telemetry, status, environment, and legacy node encoding loops avoid copying each entry before encoding.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 5bf8b

This change removes unnecessary copies while protobuf-encoding node database entries without altering the encoding flow. No current merge-blocking risk remains.

Suggested reviewers: caveman99

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	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 identifies the performance change: binding NodeDB encode-loop entries by const reference instead of copying them.
Description check	✅ Passed	The description is complete and relevant. It explains the issue, scope, intentional exception, testing results, static analysis results, and the lack of device testing.
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.

• Fix all pre-merge checks with AI

✨ 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 cppcheck-nodedb

---

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.}

[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.