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

Added link stats packet #33

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

Conversation

David-OConnor
Copy link

@David-OConnor David-OConnor commented Jun 9, 2023

Follow-up to RC packet chat. Currently based on ELRS. Open to changes to make it more general A/R.

@David-OConnor
Copy link
Author

I'm going to use this packet format unless the community wants something different.

@hamishwillee
Copy link

hamishwillee commented Jun 21, 2023

@David-OConnor Thanks for linking me in for co-ordination with MAVLink mavlink/mavlink#1920.

I hope to discuss this in the MAVLink call tonight (I personally don't know much about radio links). I've reproduced the two proposed messages below.

I think they are relatively similar except that the MAVLink version seems to report RSSI/SNR from both ends of the transmission. It is not clear to me if the radio can get the information from the other end of the communication. If it can, then having that information would be useful presumably.

From an alignment perspective it would be good to have a similar naming convention and ranges, so we can easily map. After the call I will hopefully be able to communicate in a useful way on this.

#
# This definition describes metadata and statistics for a RF control link.
# It provides information regarding a link's signal quality, as well as transmitter
# power, and antenna information. It accomodates single-radio, and dual-radio
# link architectures. It provides information for both uplink (control unit to airborne
# receiver), and downlink (airborne receiver to control unit) data.
#

uint8 STATUS_RSSI_VALID = 1 # RSSI field is valid

# Uplink - received signal strength, in decibel-milliwatts (RSSI dBm) for antennas 1 and 2 respectively. 
# Ranges from # -255 to 0. (An integer value of 100 means RSSI is -100dBm)
uint8 uplink_rssi_1
uint8 uplink_rssi_2

# Link quality, on a scale from 1 to 255. Is a proxy for packet loss. 0 means data is unavailable. 1 and 255 are mapped to 0% and 100% of packets received.
uint8 uplink_link_quality

# Signal-to-noise ratio.
int8 uplink_snr

# This is an implementation-specific enum. May represent the OTA protocol used,
# and transmission rate.
uint8 rf_mode

# This is an implementation-specific enum describing the current transmitter nominal
# power level.
uint8 uplink_tx_power

# Statistics for downlink data, ie telemetry. See their respective `uplink` fields for
# descriptions.
uint8 downlink_rssi
uint8 downlink_link_quality
int8 downlink_snr

# Active antenna on transmitter and receiver. Example: 1 for antenna 1, and 2 for antenna 2. 
# Not applicable for single-antenna links.
uint8 tx_active_antenna
uint8 rx_active_antenna

Mavlink "Proposal": RADIO_LINK_STATS

  • rssi values are in MAVLink units: 0 represents weakest signal, 254 represents maximum signal by default.
  • Can be changed to dBm with a flag RADIO_LINK_STATS_FLAGS_RSSI_DBM.
- uint8_t flags (including RADIO_LINK_STATS_FLAGS_RSSI_DBM)
- uint8_t rx_LQ : (%) Link quality
- uint8_t rx_rssi1 : Rssi of antenna1
- int8_t rx_snr1 :  Noise on antenna1. Radio dependent. 
- uint8_t  rx_rssi2 : Rssi of antenna1 
- int8_t rx_snr2 :  Noise on antenna2. Radio dependent. 
- uint8_t rx_receive_antenna :  0: antenna1, 1: antenna2
- uint8_t rx_transmit_antenna :  0: antenna1, 1: antenna2
- uint8_t tx_LQ : (%)
- uint8_t tx_rssi1 : Rssi of antenna1. 
- int8_t tx_snr1 : Noise on antenna1. Radio dependent.
- uint8_t tx_rssi2: Rssi of antenna2
- int8_t tx_snr2 : Noise on antenna2. Radio dependent.
- tx_receive_antenna 0: antenna1, 1: antenna2
- uint8_t tx_transmit_antenna:  0: antenna1, 1: antenna2

@David-OConnor
Copy link
Author

David-OConnor commented Jun 21, 2023

Added downlink LQ. Concur on all points. I prefer the uplink and downlink names since they're more explicit for things like SNR and LQ. Explicitly labeled RSSI as RSSI dBm. Using the names tx and rx for the active antenna fields.

# Active antenna on transmitter and receiver. Example: 1 for antenna 1, and 2 for antenna 2.
# Not applicable for single-antenna links.
uint8 tx_active_antenna
uint8 Rx_active_antenna

Choose a reason for hiding this comment

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

Consistency

Suggested change
uint8 Rx_active_antenna
uint8 rx_active_antenna

Comment on lines +16 to +17
# Link quality, on a scale from 1 to 255. Is a proxy for packet loss. 0 means data is unavailable. 1 and 255 are mapped to 0% and 100% of packets received.
uint8 uplink_link_quality
Copy link

@hamishwillee hamishwillee Jun 22, 2023

Choose a reason for hiding this comment

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

This version would align with MAVLink better.

  • MAVLink uses 0 as a proxy for invalid by preference, and max field values if "0" is meaningful number for the field., In this case 0 is valid, so suggest we swap around.
  • Usually it is easier for users parsing the data to supply percentage data as a 0..100 range. The only reason to use the full range is if you need finer resolution, which I do not believe is the case here.
  • Have no discussed the words with OllieW yet.
Suggested change
# Link quality, on a scale from 1 to 255. Is a proxy for packet loss. 0 means data is unavailable. 1 and 255 are mapped to 0% and 100% of packets received.
uint8 uplink_link_quality
# Link quality, as a percentage of valid packets received over some time period [0..100]. 256 means data is unavailable.
uint8 uplink_link_quality

Choose a reason for hiding this comment

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

@David-OConnor Can we swap to this pattern?

Comment on lines +36 to +39
# Active antenna on transmitter and receiver. Example: 1 for antenna 1, and 2 for antenna 2.
# Not applicable for single-antenna links.
uint8 tx_active_antenna
uint8 rx_active_antenna

Choose a reason for hiding this comment

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

Is this the uplink or the downlink? Or is this a typo - ie. do you mean uplink_tx_active_antenna and uplink_rx_active_antenna or something else?

From discussion on MAVLink issue:

a unit which has two antenna may or may not split them into one is for Tx and the other one for Rx.
Usually, at least in nearly all cases I'm aware of, they both would be used to e.g. receive, which is when called receive diversity. Note that many bidirectional units are half-duplex, i.e. work as transmit-receive-transmit-receive-.... so that both antenna can be used for receiving one frame at the same time.
I guess the system you have in mind is a system there two over-the-air channels/frequencies are used in parallel, in a full-duplex sense, there one channel/frequency is for transmission in one direction and the other for transmission in the other direction. As said, essentially all systems I'm aware of are of the half-duplex nature.
Anyway, also the full-duplex case can be represented by the suggested fields.

My assumption is therefore that you need to be able to get info for antenna usage on both uplink and downlink.
(Note, Ollie uses Rx for air unit, and Tx for ground unit).

I'm still confused by the intent here - i.e. if you have bidirectional data, and the antenna both receive and both send in a cycle how would you represent it. I mean you'd have to show "no rx antenna and both 1 and 2 on the tx, then visa versa).
Also if it is switching, how useful is that?

@olliw42
Copy link
Contributor

olliw42 commented Jun 22, 2023

on antenna, pl note that each unit does both receive and transmit and may handle antenna for both differently. Therefore you CANNOT just use one field like "active_antenna" but must use "active_antenna_used_for_receive" and "active_antenna_used_for_transmit". Since both sides of the link are represented in the message, there thus must be 2x2 = 4 active_antenna fields...

@hamishwillee
Copy link

I think we're now aligned in the sense that the data can be mapped between systems. We could be a bit closer if you chose to do this #33 (review) - but that would be up to you.

I'm not sure if we need more than this at this point or not.

@David-OConnor
Copy link
Author

Given the DroneCAN RC channels packet was merged as 0-255, I'd like to keep that.

Thoughts on merging this? Thank you!

@hamishwillee
Copy link

Thoughts on merging this?

Thanks @David-OConnor .

From a MAVLink perspective "right now" this is fine - there isn't a 1:1 mapping but your current data could be mapped to our subset of the data by a flight stack. If we wanted to take your superset of fields we could add those too. I'm not planning on pushing the MAVLink side of this until someone wants to start using it, since I'm not going to implement it.

My concern here is that the number of people who have looked at this is quite small. Ollie, me, you. I worry that perhaps we just don't have a broad enough worldview. Worth spending a little more time to get more input?

I'll see if I can get more eyes on the MAVLink side of things. Are there more people you can involve in the DroneCan community?

@tridge
Copy link
Member

tridge commented Aug 31, 2023

@peterbarker can you have a look at this?

Copy link
Member

@tridge tridge left a comment

Choose a reason for hiding this comment

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

how is this message linked to a receiver from the dronecan.sensors.rc.RCInput message?

@peterbarker
Copy link
Contributor

On the valid-data question, the rc-input packet uses a flags field to indicate which fields are present. Is there some reason not to use that pattern in here too, negating the question about "good invalid values"?

Should this be a per-antenna message, rather than having fields which are redundant in the non-diverse case? So include an antenna number rather than multiple (e.g.) RSSI fields.

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.

5 participants