Skip to content

Fixes a crash when you favorite a node#11146

Open
Xaositek wants to merge 1 commit into
developfrom
fix_crash_on_favorite
Open

Fixes a crash when you favorite a node#11146
Xaositek wants to merge 1 commit into
developfrom
fix_crash_on_favorite

Conversation

@Xaositek

@Xaositek Xaositek commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
  • Identified this issue when you favorited a node on the iOS app
  • Fixes said crash, verified with several nodes on my own mesh

Summary by CodeRabbit

  • Bug Fixes
    • Prevented unnecessary LoRa radio reconfiguration when changes only affect module, device, or node-database settings.
    • Preserved saving of updated configuration data while avoiding unrelated radio reset notifications.

@Xaositek Xaositek self-assigned this Jul 22, 2026
@Xaositek Xaositek added bugfix Pull request that fixes bugs 2.8 labels Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

MeshService::reloadConfig now resets radio configuration and notifies observers only for LoRa-impacting configuration or channel changes, while continuing to persist the selected configuration segments.

Changes

Mesh configuration reload

Layer / File(s) Summary
Gate radio reconfiguration by changed segments
src/mesh/MeshService.cpp
Radio reset and hardware-change notification now require SEGMENT_CONFIG or SEGMENT_CHANNELS; selected segments are still saved afterward.

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

Suggested reviewers: thebentern, caveman99, harukitoreda

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is too brief and omits the required attestations/testing details from the template. Replace the placeholder text with a fuller PR description and include the test attestations, affected devices, and fixes #bugnum if applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing a node-favoriting crash.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix_crash_on_favorite

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/mesh/MeshService.cpp (1)

140-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Condense the explanatory comment.

This new comment spans three lines and includes details that can be expressed more concisely. As per coding guidelines, comments must be minimal—one or two lines maximum—and explain only non-obvious reasons.

🤖 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` around lines 140 - 142, Condense the explanatory
comment above the LoRa reinitialization logic in MeshService to one or two
lines, retaining only the non-obvious reason that radio-affecting LoRa
configuration or channel changes require reinitialization while unrelated saves
do not.

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.

Nitpick comments:
In `@src/mesh/MeshService.cpp`:
- Around line 140-142: Condense the explanatory comment above the LoRa
reinitialization logic in MeshService to one or two lines, retaining only the
non-obvious reason that radio-affecting LoRa configuration or channel changes
require reinitialization while unrelated saves do not.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1669d420-4618-4002-875f-d9f98198da24

📥 Commits

Reviewing files that changed from the base of the PR and between f90c134 and 9dfc329.

📒 Files selected for processing (1)
  • src/mesh/MeshService.cpp

@github-actions

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.8 bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants