Skip to content

fix: skip nodeinfo upsert when we recieve defaults from the node#3796

Merged
jamesarich merged 1 commit into
meshtastic:mainfrom
mdecourcy:fix/skip-nodeinfo-upsert-when-defaults
Nov 24, 2025
Merged

fix: skip nodeinfo upsert when we recieve defaults from the node#3796
jamesarich merged 1 commit into
meshtastic:mainfrom
mdecourcy:fix/skip-nodeinfo-upsert-when-defaults

Conversation

@mdecourcy

@mdecourcy mdecourcy commented Nov 24, 2025

Copy link
Copy Markdown
Contributor

Root Cause

The firmware has a limited node database size that varies by hardware:

  • nRF52: 80 nodes
  • ESP32 (basic): 100 nodes
  • ESP32-S3 (16MB flash): 250 nodes
  • STM32WL: 10 nodes

When the firmware's nodeDB fills up and needs to add a new node, it evicts the oldest non-favorite node. When the evicted node is later re-encountered, the firmware creates a fresh entry with default placeholder values (memset to zero except node number):

  • longName: "Meshtastic XXXX" (where XXXX = last 4 hex digits of node ID)
  • shortName: "XXXX"
  • hwModel: UNSET
  • All other fields zeroed

The bug was in MeshService.kt:installNodeInfo() - it unconditionally overwrote the app's stored node information whenever the firmware sent NodeInfo with user data, even when that data was just firmware's default placeholder. This caused users to see nodes lose their real names and revert to generic "Meshtastic XXXX" names after the firmware's nodeDB cycled through enough nodes.

Fix

Modified installNodeInfo() to detect when firmware sends placeholder data by checking both:

  1. Name matches pattern ^Meshtastic [0-9a-fA-F]{4}$
  2. Hardware model is UNSET

When both conditions are true AND the app has existing valid user data, we preserve all existing user information instead of overwriting it with the placeholder.

This ensures:

  • Real user names/data are preserved even when firmware evicts and re-creates nodes
  • Legitimate updates (actual name changes, hwModel updates) are still applied
  • All other node fields (position, lastHeard, metrics, etc.) continue to update normally
  • Message routing works correctly (uses node numbers, not user data)

Draft because it'll take a bit for my nodedb to repopulate to confirm, but after reading firmwawre/app I'm 99.9% sure this will fix

@github-actions github-actions Bot added the bugfix PR tag label Nov 24, 2025

@DaneEvans DaneEvans left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks sensible.

@mdecourcy mdecourcy marked this pull request as ready for review November 24, 2025 03:36
@jamesarich jamesarich added this pull request to the merge queue Nov 24, 2025
Merged via the queue into meshtastic:main with commit 5520978 Nov 24, 2025
6 checks passed
@codecov

codecov Bot commented Nov 24, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.56%. Comparing base (5d61e78) to head (33ac1d8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...in/java/com/geeksville/mesh/service/MeshService.kt 0.00% 15 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #3796      +/-   ##
========================================
- Coverage   0.56%   0.56%   -0.01%     
========================================
  Files        381     381              
  Lines      21738   21747       +9     
  Branches    2683    2687       +4     
========================================
  Hits         122     122              
- Misses     21596   21605       +9     
  Partials      20      20              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamesarich jamesarich added this to the 2.7.8 milestone Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants