sys/inet/ieee802154/radio: fix wrong rssi conversions treewide #15616

fjmolinas · 2020-12-11T13:05:31Z

Contribution description

I'm under the impression that the rssi conversion is wrongly done in many places involved with the radio_hal. The api says:

    /**
     * @brief RSSI of the received frame.
     *
     * The RSSI is a measure of the RF power in dBm for the received frame.
     * The minimum and maximum values are 0 (-174 dBm) and 254 (80 dBm).
     */
    uint8_t rssi;

But in practice what was being done for cc2538 was a wrong compensation (with some weird double negations) which led to a -28db penalty in the reported value.

A similar case happend for nrf52840.

Then all users of this reported value where using the value as dB and not adjusting by the offset.

I came across this issue when comparing Vanilla OpenWSN and RIOT OpenWSN side by side and realizing there was an unexplained penalty in the reported RSSI.

I went back in history to look at when these changes where made specially for cc2538, I honestly got quite confused with the whole discussion so maybe I'm wrong about the values?

Testing procedure

Look at the code
tests/ieee802154_hal should print correct rssi.

Related Issues

Related to #14672
Related to #14791
Related to #14791

fjmolinas · 2020-12-11T13:06:03Z

Added the RFC tag because I'm a bit confused about how the RSSI reporting is being done TBH.

cpu/cc2538/radio/cc2538_rf_radio_ops.c

fjmolinas · 2020-12-11T13:23:53Z

Maybe if @benpicco has the reference to #14371 (comment) this could be clarified? Seems that comment motivated the change.

tests/ieee802154_hal/main.c

cpu/cc2538/radio/cc2538_rf_radio_ops.c

fjmolinas · 2020-12-11T15:21:46Z

cpu/nrf52/radio/nrf802154/nrf802154_radio.c

-            radio_info->rssi = _hwval_to_ieee802154_dbm(hwlqi) + IEEE802154_RADIO_RSSI_OFFSET;
+            radio_info->rssi = _hwval_to_ieee802154_dbm(hwlqi) - IEEE802154_RADIO_RSSI_OFFSET;


Same here, can it overflow?

if it comes from the hardware, I would prefer to have it clamped. As described above, we never know what might happen in the radio

drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c

cpu/nrf52/radio/nrf802154/nrf802154_radio.c

cpu/cc2538/radio/cc2538_rf_radio_ops.c

pkg/openwsn/contrib/radio_hal.c

fjmolinas · 2021-01-13T08:30:16Z

Rebased to master

fjmolinas · 2021-01-13T08:30:27Z

@jia200x rebase on top of #15629

maribu

One comment regarding the nRF802154 radio. But likely I'm just missing context to understand this.

maribu · 2021-01-13T13:11:39Z

cpu/nrf52/radio/nrf802154/nrf802154_radio.c

+            /* We calculate RSSI from LQI, since it's already 8-bit
+               saturated */


Maybe I'm missing context, but this comment confuses me more than it helps. I also don't quite understand how RSSI can be calculated from LQI. There doesn't seem to be a consistent definition (at least when also considering non-802.15.4 devices) for the LQI. But it seems to be mostly based on bit error rates, on "how easy a received signal can be demodulated" (citing the CC1101 datasheet), or the signal to noise ratio.

So as far as I understood, the LQI can be quite good for a weak signal (if both receiver and transmitter perform optimal and no interference, noise, scattering, reflection or other effects impacting the signal quality are present). Similar, a high RSSI doesn't necessarily mean a high LQI: E.g. when the receiver is right next to the sender, but also right next to an interfering device (e.g. an 802.11 device) the link quality can be poor.

So to my best knowledge, RSSI and LQI are orthogonal to each other and one cannot compute one from the other.

@jia200x mind suggesting a better comment since I took this from your suggestion :)

this comes from the datasheet:

When a packet is received a link quality indicator (LQI) is also generated and appended immediately after the last received octet. When using IEEE 802.15.4 compliant frame this will be just after the MSDU since the FCS is not reported. In the case of a non-complient frame it will be appended after the full frame. The LQI reported by hardware must be converted to IEEE 802.15.4 range by an 8-bit saturating multiplication by 4, as shown in the code example for ED sampling. The LQI is only valid for frames equal to or longer than three octets. When receiving a frame the RSSI (reported as negative dB) will be measured at three points during the reception. These three values will be sorted and the middle one selected (median 3) for then to be remapped within the LQI range. The following figure illustrates the LQI measurement and how the data is arranged in the DataRAM:

Maybe we can add a reference to the datasheet for this?

So to my best knowledge, RSSI and LQI are orthogonal to each other and one cannot compute one from the other.

This was my understanding too. I agree it looks weird. That's what I interpret from the datasheet at least.

alternatively, we can introduce a CAP. That would help to optimize the binary later

From the standard I'm not convinced the value is wrong ieee802154-2020 p.457:

10.2.7 Link quality indicator (LQI) The LQI measurement is a characterization of the strength and/or quality of a received packet. The measurement may be implemented using receiver ED, a signal-to-noise ratio estimation, or a combination of these methods. The use of the LQI result by the network or application layers is not specified in this standard. The LQI measurement shall be performed for each received packet. The minimum and maximum LQI values (0x00 and 0xff) should be associated with the lowest and highest quality compliant signals detectable by the receiver, and LQI values in between should be uniformly distributed between these two limits. At least eight unique values of LQI shall be used.

Ok, then let's leave it as it is now. I had the same impression as @maribu , but I htink the standard is the reference here.
I know that comment about LQI and RSSI was written somewhere (at86rf2xx?), but I think it will be better to follow the standard here

@maribu are you ok with that? :)

I added the RM reference to the comment.

fjmolinas · 2021-01-13T16:48:28Z

Rebased.

fjmolinas · 2021-01-13T17:38:57Z

@maribu @jia200x let me know if its Ok to squash

maribu · 2021-01-13T18:41:26Z

I'm fine with squashing :-)

fjmolinas · 2021-01-13T18:45:40Z

squashed

jia200x

ACK. We should have this one before the Hard Feature freeze and this is not considered I high impact feature. Let's get it there.

jia200x · 2021-01-13T19:38:33Z

thanks for the fix!!

fjmolinas · 2021-01-13T22:43:43Z

Thanks for the thorough review @jia200x!

fjmolinas added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking labels Dec 11, 2020

fjmolinas requested review from jia200x and benpicco December 11, 2020 13:05

fjmolinas requested review from aabadie, bergzand, leandrolanzieri, MichelRottleuthner, SemjonWilke and smlng as code owners December 11, 2020 13:05

fjmolinas added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Dec 11, 2020

fjmolinas commented Dec 11, 2020

View reviewed changes

cpu/cc2538/radio/cc2538_rf_radio_ops.c Show resolved Hide resolved

miri64 requested a review from roberthartung December 11, 2020 14:18

jia200x reviewed Dec 11, 2020

View reviewed changes

tests/ieee802154_hal/main.c Outdated Show resolved Hide resolved

fjmolinas commented Dec 11, 2020

View reviewed changes

cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated Show resolved Hide resolved

fjmolinas commented Dec 11, 2020

View reviewed changes

drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c Outdated Show resolved Hide resolved

jia200x mentioned this pull request Dec 14, 2020

ieee802154: add helpers for RSSI and dBm conversion #15629

Merged

jia200x reviewed Dec 15, 2020

View reviewed changes

cpu/nrf52/radio/nrf802154/nrf802154_radio.c Outdated Show resolved Hide resolved

jia200x reviewed Dec 15, 2020

View reviewed changes

cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated Show resolved Hide resolved

jia200x reviewed Dec 15, 2020

View reviewed changes

pkg/openwsn/contrib/radio_hal.c Outdated Show resolved Hide resolved

fjmolinas force-pushed the pr_ieee802154_rssi_conversion branch from 63b25e3 to e49420c Compare January 13, 2021 08:29

fjmolinas requested a review from miri64 as a code owner January 13, 2021 08:29

fjmolinas added the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 13, 2021

benpicco requested a review from maribu January 13, 2021 09:49

maribu reviewed Jan 13, 2021

View reviewed changes

fjmolinas force-pushed the pr_ieee802154_rssi_conversion branch from e49420c to 621a5b1 Compare January 13, 2021 16:48

fjmolinas added 3 commits January 13, 2021 19:45

cpu/cc2538/radio/cc2538_rf_radio_ops: fix rssi reporting

f8bc934

cpu/nrf52/nrf802154_radio: fix rssi calculation

6d4d00e

pkg/openwsn/contrib/radio_hal: fix rssi conversion

49c90cd

fjmolinas removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 13, 2021

fjmolinas force-pushed the pr_ieee802154_rssi_conversion branch from c484985 to 49c90cd Compare January 13, 2021 18:45

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 13, 2021

jia200x approved these changes Jan 13, 2021

View reviewed changes

jia200x merged commit db44967 into RIOT-OS:master Jan 13, 2021

fjmolinas deleted the pr_ieee802154_rssi_conversion branch January 13, 2021 22:43

N11cc00 mentioned this pull request Jul 31, 2024

Wrong RSSI calculation for the nrf802154 #20803

Closed

		radio_info->rssi = _hwval_to_ieee802154_dbm(hwlqi) + IEEE802154_RADIO_RSSI_OFFSET;
		radio_info->rssi = _hwval_to_ieee802154_dbm(hwlqi) - IEEE802154_RADIO_RSSI_OFFSET;

		/* We calculate RSSI from LQI, since it's already 8-bit
		saturated */

sys/inet/ieee802154/radio: fix wrong rssi conversions treewide #15616

sys/inet/ieee802154/radio: fix wrong rssi conversions treewide #15616

Uh oh!

Conversation

fjmolinas commented Dec 11, 2020

Contribution description

Testing procedure

Related Issues

Uh oh!

fjmolinas commented Dec 11, 2020

Uh oh!

Uh oh!

fjmolinas commented Dec 11, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjmolinas Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

maribu commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

jia200x left a comment

Choose a reason for hiding this comment

Uh oh!

jia200x commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

Uh oh!

fjmolinas Jan 13, 2021 •

edited

Loading