Added Neighbors to the repeater hub and a screen to display the Neighbors #42
Merged
Conversation
zjs81
reviewed
Jan 17, 2026
zjs81
left a comment
Owner
There was a problem hiding this comment.
Code Review: PR #42 - Added Neighbors to the repeater hub
Files Changed:
lib/l10n/app_en.arb(+3 lines)lib/screens/neighbours_screen.dart(+381 lines, new file)lib/screens/repeater_hub_screen.dart(+22/-1 lines)lib/widgets/snr_indicator.dart(+65 lines, new file)
Overview
This PR adds a new "Neighbors" feature to the repeater hub, allowing users to view zero-hop neighbors of a repeater. The implementation includes:
- A new
NeighboursScreenfor displaying neighbor data - A reusable
SNRIconwidget for signal-to-noise ratio visualization - Integration into the repeater hub navigation
- English localization strings
Code Quality Issues
1. Inconsistent Indentation in lib/screens/neighbours_screen.dart:63-75
Current code:
_frameSubscription = connector.receivedFrames.listen((frame) {
if (frame.isEmpty) return;
if(frame[0] == respCodeSent){
_tagData = frame.sublist(2, 6);
_timeEstment = frame.buffer.asByteData().getUint32(6, Endian.little);
}
// Check if it's a binary response
if (frame[0] == pushCodeBinaryResponse && listEquals(frame.sublist(2, 6), _tagData)) {
_handleNeighboursResponse(context, connector, frame.sublist(6));
}
});
}Should be:
_frameSubscription = connector.receivedFrames.listen((frame) {
if (frame.isEmpty) return;
if (frame[0] == respCodeSent) {
_tagData = frame.sublist(2, 6);
_timeEstment = frame.buffer.asByteData().getUint32(6, Endian.little);
}
// Check if it's a binary response
if (frame[0] == pushCodeBinaryResponse && listEquals(frame.sublist(2, 6), _tagData)) {
_handleNeighboursResponse(context, connector, frame.sublist(6));
}
});
}Note: Also missing space after if keyword (if(frame[0] → if (frame[0]).
2. Missing Newline at End of Files
Both new files are missing a trailing newline:
lib/screens/neighbours_screen.dart- ends with}on line 381lib/widgets/snr_indicator.dart- ends with}on line 65
Fix: Add a blank line after the final closing brace in each file.
3. Unused Import in lib/screens/neighbours_screen.dart:4
Current code:
import 'package:meshcore_open/models/radio_settings.dart';This import is not used anywhere in the file and should be removed.
4. Unused Instance Variables in lib/screens/neighbours_screen.dart:36-37
_timeEstment (line 37):
- Typo in name: should be
_timeEstimate - Assigned on line 68 but never read anywhere
_neighbourCount (line 36):
- Assigned on line 131 but never read
Recommendation: Either remove these if unused, or add TODO comments if they're placeholders for future features (e.g., pagination).
Potential Bugs
1. Operator Precedence Issue in lib/screens/neighbours_screen.dart:307
Current code:
if (_isLoaded || _hasData && !(_parsedNeighbours == null || _parsedNeighbours!.isEmpty))Due to operator precedence (&& binds tighter than ||), this evaluates as:
if (_isLoaded || (_hasData && !(_parsedNeighbours == null || _parsedNeighbours!.isEmpty)))This means the card will show if _isLoaded is true, regardless of whether _parsedNeighbours has data.
Suggested fix:
if (_parsedNeighbours != null && _parsedNeighbours!.isNotEmpty)2. Protocol Payload Needs Documentation in lib/screens/neighbours_screen.dart:158
Current code:
final frame = buildSendBinaryReq(
repeater.publicKey,
payload: Uint8List.fromList([reqTypeGetNeighbours, 0x00, 0x0F, 0x00, 0x00, 0x00, _reqNeighboursKeyLen])
);The magic bytes are unexplained. Following the pattern in meshcore_protocol.dart where frame builders have comments explaining their format:
Suggestion - add a helper function in lib/connector/meshcore_protocol.dart:
// Build neighbours request payload
// Format: [req_type][offset_lo][offset_hi][count_lo][count_hi][reserved][key_len]
Uint8List buildGetNeighboursPayload({
int offset = 0,
int count = 15,
int keyLen = 4,
}) {
return Uint8List.fromList([
reqTypeGetNeighbours,
offset & 0xFF,
(offset >> 8) & 0xFF,
count & 0xFF,
(count >> 8) & 0xFF,
0x00, // reserved
keyLen,
]);
}Then use it in neighbours_screen.dart:
final frame = buildSendBinaryReq(
repeater.publicKey,
payload: buildGetNeighboursPayload(),
);3. Unused Function in lib/widgets/snr_indicator.dart:3-19
Current code:
List<double> getLoRaMinimumSNR(int spreadingFactor) {
switch (spreadingFactor) {
case 7:
return [4.0, -2.0, -4.0, -6.0];
// ...
}
}This function is defined but never called. The SNRIcon widget has a default snrLevels but doesn't use this function.
Recommendation: Either remove it until needed, or wire it up:
SNRIcon(
snr: snr,
snrLevels: getLoRaMinimumSNR(currentSpreadingFactor),
)Style Issues
1. Redundant Widget Nesting in lib/screens/neighbours_screen.dart:352-368
Current code:
Widget _buildInfoRow(String label, String value, double snr) {
return Padding(
padding: const EdgeInsets.symmetric(vertical: 3),
child: Row(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Expanded(
child: ListTile(
contentPadding: EdgeInsets.zero,
title: Text(label, style: const TextStyle(fontWeight: FontWeight.w500)),
subtitle: Text(value),
trailing: SNRIcon(snr: snr),
),
),
],
),
);
}The Row with a single Expanded child is unnecessary.
Should be:
Widget _buildInfoRow(String label, String value, double snr) {
return Padding(
padding: const EdgeInsets.symmetric(vertical: 3),
child: ListTile(
contentPadding: EdgeInsets.zero,
title: Text(label, style: const TextStyle(fontWeight: FontWeight.w500)),
subtitle: Text(value),
trailing: SNRIcon(snr: snr),
),
);
}2. Incorrect Comment in lib/screens/repeater_hub_screen.dart:148
Current code:
const SizedBox(height: 12),
// Settings button
_buildManagementCard(
context,
icon: Icons.group,
title: l10n.repeater_neighbours,Should be:
const SizedBox(height: 12),
// Neighbours button
_buildManagementCard(Summary
| Priority | Issue | File | Line |
|---|---|---|---|
| High | Indentation & spacing in listener callback | neighbours_screen.dart |
63-75 |
| High | Operator precedence may cause wrong display logic | neighbours_screen.dart |
307 |
| Medium | Unused import (radio_settings.dart) |
neighbours_screen.dart |
4 |
| Medium | Unused variables (_neighbourCount, _timeEstment) |
neighbours_screen.dart |
36-37 |
| Medium | Unused function getLoRaMinimumSNR() |
snr_indicator.dart |
3-19 |
| Medium | Magic bytes need documentation (consider helper in meshcore_protocol.dart) |
neighbours_screen.dart |
158 |
| Low | Missing trailing newlines | Both new files | EOF |
| Low | Redundant Row/Expanded wrapper | neighbours_screen.dart |
352-368 |
| Low | Wrong comment ("Settings" → "Neighbours") | repeater_hub_screen.dart |
148 |
Overall this adds useful functionality for viewing repeater neighbors! The main items to address are the indentation fix and the boolean logic clarification.
45ab966 to
04a713b
Compare
f4c4551 to
b41ccee
Compare
c0444f5 to
6c8a149
Compare
- Added `untranslated.json` to track untranslated messages. - Updated localization keys in various language files to use camelCase format for consistency. - Modified `neighbours_screen.dart` to reference updated localization keys.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added neighbors to the repeater hub and a screen to display the neighbors.
Few things that have lose end: