Skip to content

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Jan 6, 2026

Summary

Add a new setting to control how hop counts are calculated for nodes. This provides flexibility in how users want to visualize node proximity.

Three calculation modes:

  • NodeInfo packet (default): Uses hopsAway from NodeInfo packets (current behavior)
  • Traceroute length: Uses the shorter of route/routeBack from traceroutes
  • All messages: Uses hops from most recently received packet (hopStart - hopLimit)

Affects:

  • Node list hop indicators
  • Map marker coloring
  • Messages tab node list
  • NodeDetailsBlock display
  • Node sorting by hops

Changes:

  • Add lastMessageHops column to database for tracking packet hops
  • Track hopStart - hopLimit on incoming packets in meshtasticManager
  • Add nodeHopsCalculation setting to SettingsContext
  • Create getEffectiveHops utility function in src/utils/nodeHops.ts
  • Update NodesTab, MessagesTab, and NodeDetailsBlock to use effective hops
  • Add setting dropdown in Node Display section of Settings

Test plan

  • Verify setting appears in Settings → Node Display section
  • Verify changing setting enables Save button
  • Verify all three calculation modes work correctly
  • Verify map markers update color based on selected mode
  • Verify node lists show correct hop counts
  • Run system tests

System Test Results

Configuration Import:     ✓ PASSED
Quick Start Test:         ✓ PASSED
Security Test:            ✓ PASSED
Reverse Proxy Test:       ✓ PASSED
Reverse Proxy + OIDC:     ✓ PASSED
Virtual Node CLI Test:    ✓ PASSED
Backup & Restore Test:    ✓ PASSED

✓ ALL SYSTEM TESTS PASSED

🤖 Generated with Claude Code

Add a new setting to control how hop counts are calculated for nodes.
Three options are available:

- NodeInfo packet (default): Uses hopsAway from NodeInfo packets
- Traceroute length: Uses the shorter of route/routeBack from traceroutes
- All messages: Uses hops from most recently received packet (hopStart - hopLimit)

This setting affects:
- Node list hop indicators
- Map marker coloring
- Messages tab node details
- NodeDetailsBlock display
- Node sorting by hops

Changes:
- Add lastMessageHops column to database for tracking packet hops
- Track hopStart - hopLimit on incoming packets in meshtasticManager
- Add nodeHopsCalculation setting to SettingsContext
- Create getEffectiveHops utility function in src/utils/nodeHops.ts
- Update NodesTab, MessagesTab, and NodeDetailsBlock to use effective hops
- Add setting dropdown in Node Display section of Settings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Jan 6, 2026

Claude finished @Yeraze's task —— View job


Code Review Complete

I've thoroughly reviewed the Node Hops Calculation setting implementation. This is a well-architected feature that provides flexibility in how hop counts are calculated. Here's my comprehensive feedback:

Todo List:

  • Read and analyze all changed files
  • Review database schema changes
  • Check UI component integration
  • Analyze utility functions and type safety
  • Evaluate performance implications
  • Check for security concerns
  • Assess test coverage
  • Provide comprehensive feedback

🟢 Strengths

Architecture & Design

  • Clean separation of concerns: getEffectiveHops() utility centralizes hop calculation logic
  • Backwards compatibility: Default 'nodeinfo' mode preserves existing behavior
  • Consistent integration: All UI components (NodesTab, MessagesTab, NodeDetailsBlock) use the same utility
  • Type safety: Proper TypeScript types throughout

Implementation Quality

  • Database schema: Clean addition of lastMessageHops column with proper null checking
  • Data tracking: meshtasticManager.ts correctly tracks hopStart - hopLimit from incoming packets
  • Settings integration: Seamless localStorage persistence with server sync
  • Error handling: Graceful fallbacks in traceroute JSON parsing and missing data scenarios

User Experience

  • Three clear calculation modes with intuitive names
  • Comprehensive coverage: Affects all relevant UI areas (map markers, node lists, details)
  • Real-time updates: Changes apply immediately across all components

🟡 Areas for Improvement

1. Performance Considerations (src/utils/nodeHops.ts:33-66)

// Current traceroute filtering happens on every call
const relevantTraceroutes = traceroutes.filter(tr => 
  (tr.fromNodeNum === currentNodeNum && tr.toNodeNum === node.nodeNum) ||
  (tr.fromNodeNum === node.nodeNum && tr.toNodeNum === currentNodeNum)
);

Recommendation: Consider memoizing traceroute lookups or creating an indexed map for better performance with large traceroute datasets.

2. Error Handling Enhancement (src/utils/nodeHops.ts:44-59)

try {
  if (tr.route) {
    const route = JSON.parse(tr.route);
    if (Array.isArray(route) && route.length < minHops) {
      minHops = route.length;
    }
  }
} catch {
  // Ignore parse errors - too silent
}

Recommendation: Add debug logging for JSON parse errors to aid troubleshooting:

} catch (error) {
  logger.debug(`Failed to parse traceroute route for ${tr.fromNodeNum}->${tr.toNodeNum}:`, error);
}

3. Data Validation (src/server/meshtasticManager.ts)

if (hopStart !== undefined && hopStart !== null &&
    hopLimit !== undefined && hopLimit !== null &&
    hopStart >= hopLimit) {

Recommendation: Add bounds checking for hop values to prevent unrealistic data:

if (hopStart !== undefined && hopStart !== null &&
    hopLimit !== undefined && hopLimit !== null &&
    hopStart >= hopLimit && hopStart <= 255 && hopLimit >= 0) {

4. Settings UI Enhancement (src/components/SettingsTab.tsx)

Recommendation: Add tooltips explaining what each calculation mode does:

  • "NodeInfo packet": Uses hopsAway from node info broadcasts
  • "Traceroute length": Uses shortest path from traceroute data
  • "All messages": Uses hops from most recently received packet

🟢 Security Assessment

  • No security concerns identified: All user input is validated
  • Proper type checking: No injection vectors in hop calculation logic
  • Safe JSON parsing: Errors are caught and handled gracefully

🟡 Test Coverage Recommendations

While system tests pass, consider adding unit tests for:

// Test cases for getEffectiveHops()
describe('getEffectiveHops', () => {
  it('should fallback to nodeinfo when traceroute fails to parse')
  it('should handle missing traceroute data gracefully')
  it('should prefer shorter of route/routeBack in traceroute mode')
  it('should return 999 for unknown hops')
})

🟢 Code Quality Score: 8.5/10

This implementation demonstrates solid engineering practices with proper TypeScript usage, clean architecture, and comprehensive integration. The minor improvements suggested would enhance robustness and observability without affecting core functionality.

Key Praise Points:

  • Excellent separation of concerns with utility function
  • Comprehensive UI integration
  • Backwards compatible implementation
  • Clean database schema evolution

@Yeraze Yeraze merged commit efb3b6d into main Jan 6, 2026
14 checks passed
@Yeraze Yeraze deleted the feature/node-hops-calculation-setting branch January 6, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants