Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for known 0 hopsAway #1295

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

jamesarich
Copy link
Contributor

This change updates the database schema to allow storing null values for the 'hopsAway' field in the 'nodes' table.

This allows for better representation of unknown hop counts instead of defaulting to 0.
The UI is also updated to display signal information and node details accordingly when 'hopsAway' is null and zero.
image

This change updates the database schema to allow storing null values for the 'hopsAway' field in the 'nodes' table.

This allows for better representation of unknown hop counts instead of defaulting to 0.
The UI is also updated to display signal information and node details accordingly when 'hopsAway' is null.
Copy link

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

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

I'm assuming the DB stuff is right since I'm not really up on that part. I think there's an update needed in MeshService that I've outlined, plus making sure the wantconfig NodeInfo get initialized properly, and the display needed is slightly different than this

app/src/main/java/com/geeksville/mesh/ui/SignalInfo.kt Outdated Show resolved Hide resolved
This commit addresses two issues:

- **Signal Info Display**: The signal info display is enhanced to show
 channel information, RSSI, SNR, and hops away in a clearer and more organized way using separators. It also handles cases where hopsAway is null or 0.
- **Node Hop Calculation**: This now explicitly checks if hopsAway is present in the proto message before using it.

These changes improve the usability and accuracy of signal information for users.
Copy link

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

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

Looks good as far as what I know at least!

This replaces previous omitted logic in signalInfo to only display snr and rssi when the signal is poor.

- RSSI and SNR are
 now only displayed for local nodes (hopsAway = 0) when the signal quality is poor (SNR < 100 and RSSI < 0).
- This prevents unnecessary clutter in the UI for nodes with good signal strength or those that are further away.
- Revert whitespace changes
- Simplify `hopsAway` assignment using `if` expression.
- Revert whitespace changes
Changes the constants used for RSSI and SNR to `MAX_VALID_RSSI` and `MAX
_VALID_SNR` to better reflect their purpose. This makes it clearer that these values represent the maximum valid values for these metrics.
Copy link
Member

@andrekir andrekir left a comment

Choose a reason for hiding this comment

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

also please undo the protobuf update, then should be ready to merge

app/src/main/java/com/geeksville/mesh/NodeInfo.kt Outdated Show resolved Hide resolved
Since NodeInfo.hopsAway is nonNullable, set it to zero when converting from NodeEntity.
Updates the code to handle the new default value and display "unknown" when the value is -1.

Reverts database changes.
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.

3 participants