Double-press: fall back to first channel with position enabled when primary precision == 0

[fgimenezm]

Double-press: fall back to first channel with position enabled when primary precision == 0

Summary

Make the double-press "send position" action use the same channel-selection logic used by automatic position broadcasts: prefer the node's current channel, but if that channel's position precision is 0, fall back to the lowest-indexed channel (0..7) with a non-zero position precision. If no channel exposes position, fall back to sending NodeInfo.

---

Problem / Background

• Automatic periodic position broadcasts already pick the first channel with a non-zero precision (PositionModule::sendOurPosition()).
• The double-press flow (MeshService::trySendPosition()) historically always sent on node->channel. If that channel's position_precision == 0 no position message is sent.
• This causes a surprising user experience: a double-press that intends to "send my location" doesn't send the position even though a secondary channel would have sent a usable location.

---

What changed

• File modified:
  • src/mesh/MeshService.cpp
• Function modified:
  • MeshService::trySendPosition(NodeNum dest, bool wantReplies)
• New behavior:
  1. If getPositionPrecisionForChannel(node->channel) != 0, send on node->channel (unchanged).
  2. Otherwise, scan channels 0..7 for the first channel with getPositionPrecisionForChannel(ch) != 0 and send the position on that channel.
  3. If no channel has position enabled, call nodeInfoModule->sendOurNodeInfo(dest, wantReplies, node->channel) (mirrors the prior fallback when no valid position exists).

This preserves dest and wantReplies semantics and matches automatic broadcast selection.

---

Rationale for NodeInfo fallback

• Falling back to NodeInfo when no channel allows position sharing produces a useful transmit and mirrors the existing "no position available" behavior.

---

Files changed

• src/mesh/MeshService.cpp (single, localized change in trySendPosition)

---

Test plan (manual)

1. Configure a device:
   • Primary channel (index 0): module_settings.position_precision = 0
   • Secondary (lowest-indexed, e.g. index 1): module_settings.position_precision > 0
2. Ensure the device has a valid position (GPS lock or fixed position).
3. Trigger double-press (tracker) or otherwise invoke INPUT_BROKER_SEND_PING.
4. Expect:
   • Log: Send position ping to 0x..., channel=<secondary index>
   • Sent packet contains POSITION payload with coordinates (not stripped).
5. Additional checks:
   • Primary with non-zero precision → double-press sends on primary.
   • No channel with position enabled → double-press sends NodeInfo.
6. Run CI/build to confirm no build regressions.

Suggested logs to verify:

• "Send position ping to 0x..., channel=X"
• Packet payload shows position fields (latitude_i / longitude_i present)

---

Risk / compatibility

• Risk: low. Change is small and localized to MeshService::trySendPosition.
• Compatibility: no API changes; only changes the chosen channel for manual position sends when the primary channel disables sharing.
• Recommendation: run CI across targets.

---

References

• Sharing Location on a Private Secondary Channel
  https://meshtastic.org/docs/configuration/tips/#sharing-location-on-a-private-secondary-channel

Summary by CodeRabbit

• Bug Fixes
  • Position updates now use an available channel when the node’s current channel doesn’t support position publishing.
  • If no channel supports position updates, the system sends a NodeInfo update instead.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Walkthrough

MeshService::trySendPosition now selects an enabled channel for position pings, searches channels 0–7 when needed, and falls back to sending NodeInfo on the original channel when position publishing is unavailable.

Changes

Position ping routing

Layer / File(s)	Summary
Channel selection and NodeInfo fallback src/mesh/MeshService.cpp	trySendPosition prefers the node’s current channel, searches for an enabled position channel, and sends NodeInfo with a false result when no channel supports position publishing.

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

Suggested reviewers: thebentern, ndoo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it omits the required attestation checklist and explicit testing status from the repository template.	Add the attestations section with completed testing status and note any untested devices or hardware so the PR matches the template.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title matches the main change: double-press position sending now falls back to another enabled channel when the primary cannot publish position.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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]

@fgimenezm, Welcome to Meshtastic!

Thanks for opening your first pull request. We really appreciate it.

We discuss work as a team in discord, please join us in the #firmware channel.
There's a big backlog of patches at the moment. If you have time,
please help us with some code review and testing of other PRs!

Welcome to the team 😄

[coderabbitai]

🧹 Nitpick comments (1)

src/mesh/MeshService.cpp (1)

311-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Include missing context in the fallback log.

Consider including wantReplies and channel in this log message to match the formatting of the analogous NodeInfo ping log on line 324. This improves observability when debugging fallback routing.

💡 Proposed fix

-                        LOG_INFO("No channel with position enabled; sending nodeinfo instead to 0x%08x", dest);
+                        LOG_INFO("No channel with position enabled; sending nodeinfo instead to 0x%08x, wantReplies=%d, channel=%d", dest, wantReplies, node->channel);

🤖 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/MeshService.cpp` at line 311, Update the fallback LOG_INFO call in
MeshService to include the wantReplies and channel values, matching the
formatting and context of the analogous NodeInfo ping log while retaining the
existing destination information.

🤖 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 `@src/mesh/MeshService.cpp`:
- Line 311: Update the fallback LOG_INFO call in MeshService to include the
wantReplies and channel values, matching the formatting and context of the
analogous NodeInfo ping log while retaining the existing destination
information.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bfb9017a-9f72-47c0-acc2-2c9dfd610bf7

📥 Commits

Reviewing files that changed from the base of the PR and between dd509a8 and b0e4063.

📒 Files selected for processing (1)

• src/mesh/MeshService.cpp

[caveman99]

Would it be too much to ask to test compile your code before you submit it?