Skip to content

Conversation

@1nv
Copy link
Contributor

@1nv 1nv commented Dec 10, 2025

Description

This change adds metrics to the reaction dialog, next to the emoji which a user reacted with. The metrics include 'Hops Away' for messages that went though hops or 'SNR/RSSI' for messages received directly – this is the same behavior you see with normal messages.

Below is the reaction dialog before:
meshtastic_screenshot_before
and after:
meshtastic_screenshot

Motivation

Our mesh network uses bots for testing connectivity. When a user sends a ping message to a specific channel, bots automatically reply with a message that includes 'Hops Away' or 'SNR/RSSI' values, so that the user could see which end-points received their message along with the aforementioned metrics on the way to those end-points. They can also see metrics on the way back in the message history.
To reduce clutter, I made a bot that sends a reaction with hop count (e.g. 1️⃣) or signal quality (e.g. 🟨 for fair quality). This way the user can still see the metrics towards my end-point, but no additional messages visible in channel are generated. However, they cannot see metrics on the way back, although they are technically available.
This change adds those missing metrics, at a cost of storing those within the database.
This can also be useful for networks without bots, as it can provide additional insights and the behavior is consistent with normal messages.

Additional Notes

I'm new to Kotlin, so I might have done something wrong. I have run Spotless and Detekt to check for errors.

…is useful for testing connectivity when users or bots react to a user's message.
@CLAassistant
Copy link

CLAassistant commented Dec 10, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@1nv
Copy link
Contributor Author

1nv commented Dec 15, 2025

@jamesarich Hi, do I need to take further steps to mark this PR for review?

@jamesarich
Copy link
Collaborator

@jamesarich Hi, do I need to take further steps to mark this PR for review?

Just patience. We're volunteers and busy 😅

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds SNR/RSSI and Hops Away metrics to the reaction dialog, providing users with signal quality information similar to what's shown for regular messages. The implementation includes database schema changes to store these metrics, UI updates to display them, and service logic refactoring to extract common hop calculation logic.

Key Changes:

  • Database schema upgraded to v24 with new columns (snr, rssi, hopsAway) added to the reactions table with appropriate default values
  • New getHopsAwayForPacket() helper function extracts duplicate hop calculation logic
  • Reaction dialog UI updated to display metrics conditionally based on signal data availability

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
core/database/schemas/org.meshtastic.core.database.MeshtasticDatabase/24.json New schema file defining reactions table with added metric columns (snr, rssi, hopsAway)
core/database/src/main/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt Increments database version to 24 and adds AutoMigration entry
core/database/src/main/kotlin/org/meshtastic/core/database/entity/Packet.kt Extends Reaction and ReactionEntity data classes with metric fields, updates mapping function
app/src/main/java/com/geeksville/mesh/service/MeshService.kt Adds getHopsAwayForPacket() helper, refactors duplicate logic, populates metrics when storing reactions
feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/component/Reaction.kt Updates ReactionDialog UI to display SNR/RSSI for direct messages or hops for relayed messages, adds preview data

Comment on lines 160 to 170
if (reaction.snr != 0.0f) {
if (reaction.hopsAway == 0) {
Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
Snr(reaction.snr)
Rssi(reaction.rssi)
}
} else {
Text(
text = stringResource(Res.string.hops_away_template, reaction.hopsAway),
style = MaterialTheme.typography.labelSmall,
)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

When hopsAway is -1 (indicating invalid hop data per the logic in getHopsAwayForPacket), this will display "Hops Away: -1" to the user, which could be confusing. Consider checking if hopsAway is greater than 0 before showing hops, otherwise fall back to showing SNR/RSSI when snr is not 0. This would better match the user's expectation that -1 represents missing or invalid data rather than a meaningful hop count.

Suggested change
if (reaction.snr != 0.0f) {
if (reaction.hopsAway == 0) {
Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
Snr(reaction.snr)
Rssi(reaction.rssi)
}
} else {
Text(
text = stringResource(Res.string.hops_away_template, reaction.hopsAway),
style = MaterialTheme.typography.labelSmall,
)
if (reaction.hopsAway > 0) {
Text(
text = stringResource(Res.string.hops_away_template, reaction.hopsAway),
style = MaterialTheme.typography.labelSmall,
)
} else if (reaction.snr != 0.0f) {
Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) {
Snr(reaction.snr)
Rssi(reaction.rssi)

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'Hops Away: -1' is consistent with normal message behavior.

Copy link
Collaborator

@jamesarich jamesarich left a comment

Choose a reason for hiding this comment

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

image

Adding the field to the end of the row detracts from the primary usecase of this view - which is to view who sent the reaction. There is also more potential of overflow when screen sizes are small and fontsizes are large.

As this signal/hops information isn't necessary for the basic user and common usecases it needs to be very secondary and unobtrusive. Let's make each reaction list item two rows, moving the signal/hops info to the 2nd row - similar to how it is presented at the bottom of the message.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.56%. Comparing base (8d858de) to head (ef68ada).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...in/java/com/geeksville/mesh/service/MeshService.kt 0.00% 9 Missing ⚠️
...meshtastic/feature/messaging/component/Reaction.kt 0.00% 7 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #3964      +/-   ##
========================================
- Coverage   0.56%   0.56%   -0.01%     
========================================
  Files        402     402              
  Lines      23884   23893       +9     
  Branches    3061    3061              
========================================
  Hits         136     136              
- Misses     23727   23736       +9     
  Partials      21      21              

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

@1nv
Copy link
Contributor Author

1nv commented Dec 16, 2025

@jamesarich I have updated the code for two-row layout, and added time string as well. I also slightly reduced spacing between entries, so that more could be seen on the same screen.
meshtastic_screenshot_new

@1nv 1nv requested a review from jamesarich December 16, 2025 14:00
Copy link
Collaborator

@jamesarich jamesarich left a comment

Choose a reason for hiding this comment

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

This seems a bit less obtrusive, and nice add of timestamp.

@jamesarich jamesarich enabled auto-merge December 16, 2025 14:38
@1nv 1nv changed the title feat: add SNR/RSSI or Hops Away metrics to the reaction dialog feat: add SNR/RSSI/Hops Away metrics and timestamp to the reaction dialog Dec 16, 2025
@jamesarich jamesarich disabled auto-merge December 16, 2025 14:40
@jamesarich jamesarich added enhancement New feature or request ui Something to do with the UI of the Android app labels Dec 16, 2025
@jamesarich jamesarich added this pull request to the merge queue Dec 16, 2025
Merged via the queue into meshtastic:main with commit 24f40b2 Dec 16, 2025
6 checks passed
@jamesarich
Copy link
Collaborator

@1nv just wanted to say thanks again for the contribution 🤝.
If you're interested in contributing more regularly, feel free to reach out on discord - I'm @olm3c. We can get you better looped in to conversations and processes there ➰ .

@jamesarich jamesarich added this to the 2.7.10 milestone Dec 16, 2025
@1nv
Copy link
Contributor Author

1nv commented Dec 16, 2025

@jamesarich Thank you for quick review and constructive feedback!

@1nv
Copy link
Contributor Author

1nv commented Dec 17, 2025

Small follow-up fix:
#4024

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

Labels

enhancement New feature or request ui Something to do with the UI of the Android app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants