ieee802154: add helpers for RSSI and dBm conversion #15629

jia200x · 2020-12-14T12:47:11Z

Contribution description

As IEEE 802.15.4 2020 defines, the RSSI is an unsigned octet that maps to real RF power values. For RIOT we have been using RSSI as dBm but they don't represent the same thing.

Some stacks and netdev require RSSI in dBm instead of the definition of the standard, so this PR adds some helpers to translate both. It also uses RSSI in dBm for netdev_ieee802154_submac in order to be compliant with netdev.

Testing procedure

cd tests/unittests
make tests-ieee802154 test

Issues/PRs references

This could be helpful for #15616

benpicco · 2020-12-14T13:18:13Z

I was going to say: this should also fix the wrong rssi output in ping, but instead I get

2020-12-14 14:13:04,605 #  ping ff02::1
2020-12-14 14:13:04,618 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=0 ttl=64 rssi=40 dBm time=5.120 ms
2020-12-14 14:13:04,626 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=0 ttl=64 rssi=-52 dBm time=12.813 ms (DUP!)
2020-12-14 14:13:04,634 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=0 ttl=64 rssi=12 dBm time=20.938 ms (DUP!)
2020-12-14 14:13:05,618 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=1 ttl=64 rssi=40 dBm time=5.119 ms
2020-12-14 14:13:05,626 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=1 ttl=64 rssi=-68 dBm time=12.831 ms (DUP!)
2020-12-14 14:13:05,634 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=1 ttl=64 rssi=8 dBm time=20.959 ms (DUP!)
2020-12-14 14:13:06,617 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=2 ttl=64 rssi=40 dBm time=4.772 ms
2020-12-14 14:13:06,618 # 
2020-12-14 14:13:06,621 # --- ff02::1 PING statistics ---
2020-12-14 14:13:06,627 # 3 packets transmitted, 3 packets received, 4 duplicates, 0% packet loss
2020-12-14 14:13:06,631 # round-trip min/avg/max = 4.772/11.793/20.959 ms

with nrf802154.

With netdev_ieee802154_legacy I'm getting

2020-12-14 14:16:25,602 #  ping ff02::1
2020-12-14 14:16:25,613 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=0 ttl=64 rssi=-34 dBm time=4.030 ms
2020-12-14 14:16:25,622 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=0 ttl=64 rssi=-34 dBm time=11.811 ms (DUP!)
2020-12-14 14:16:25,630 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=0 ttl=64 rssi=-34 dBm time=20.281 ms (DUP!)
2020-12-14 14:16:25,639 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=0 ttl=64 rssi=-35 dBm time=28.752 ms (DUP!)
2020-12-14 14:16:25,647 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=0 ttl=64 rssi=-43 dBm time=37.220 ms (DUP!)
2020-12-14 14:16:25,656 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=0 ttl=64 rssi=-43 dBm time=45.693 ms (DUP!)
2020-12-14 14:16:25,664 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=0 ttl=64 rssi=-73 dBm time=54.173 ms (DUP!)
2020-12-14 14:16:25,672 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=0 ttl=64 rssi=-73 dBm time=62.304 ms (DUP!)
2020-12-14 14:16:25,680 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=0 ttl=64 rssi=-73 dBm time=70.431 ms (DUP!)
2020-12-14 14:16:25,688 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=0 ttl=64 rssi=-73 dBm time=78.561 ms (DUP!)
2020-12-14 14:16:26,614 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=1 ttl=64 rssi=-34 dBm time=4.030 ms
2020-12-14 14:16:26,622 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=1 ttl=64 rssi=-34 dBm time=11.807 ms (DUP!)
2020-12-14 14:16:26,630 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=1 ttl=64 rssi=-43 dBm time=20.277 ms (DUP!)
2020-12-14 14:16:26,639 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=1 ttl=64 rssi=-34 dBm time=28.920 ms (DUP!)
2020-12-14 14:16:26,647 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=1 ttl=64 rssi=-34 dBm time=37.391 ms (DUP!)
2020-12-14 14:16:26,656 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=1 ttl=64 rssi=-43 dBm time=45.867 ms (DUP!)
2020-12-14 14:16:26,664 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=1 ttl=64 rssi=-71 dBm time=54.340 ms (DUP!)
2020-12-14 14:16:26,672 # 12 bytes from fe80::ac8d:fee1:6041:91f1%6: icmp_seq=1 ttl=64 rssi=-44 dBm time=62.472 ms (DUP!)
2020-12-14 14:16:26,680 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=1 ttl=64 rssi=-70 dBm time=70.952 ms (DUP!)
2020-12-14 14:16:26,689 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=1 ttl=64 rssi=-71 dBm time=79.086 ms (DUP!)
2020-12-14 14:16:26,697 # 12 bytes from fe80::fec2:3d00:0:bb1%6: icmp_seq=1 ttl=64 rssi=-71 dBm time=87.216 ms (DUP!)
2020-12-14 14:16:27,613 # 12 bytes from fe80::e4ea:aff8:af5d:eae4%6: icmp_seq=2 ttl=64 rssi=-34 dBm time=4.030 ms
2020-12-14 14:16:27,614 # 
2020-12-14 14:16:27,616 # --- ff02::1 PING statistics ---
2020-12-14 14:16:27,623 # 3 packets transmitted, 3 packets received, 19 duplicates, 0% packet loss
2020-12-14 14:16:27,627 # round-trip min/avg/max = 4.030/41.802/87.216 ms

might be a driver issue though

jia200x · 2020-12-14T15:12:10Z

hmmm weird, I will check what's going on

jia200x · 2020-12-15T11:10:00Z

well... the original nrf802154 driver (nrf802154.c) has a different implementation of RSSI that's not compliant with the datasheet:

            /* Calculate RSSI by subtracting the offset from the datasheet.
             * Intentionally using a different calculation than the one from
             * figure 122 of the v1.1 product specification. This appears to
             * match real world performance better */
            radio_info->rssi = (int16_t)hwlqi + ED_RSSIOFFS;

The datasheet says:

PRF[dBm] = ED_RSSIOFFS + ED_RSSISCALE x VALHARDWARE

I really think we should use the one from the datasheet

fjmolinas · 2021-01-13T07:51:49Z

well... the original nrf802154 driver (nrf802154.c) has a different implementation of RSSI that's not compliant with the datasheet:

            /* Calculate RSSI by subtracting the offset from the datasheet.
             * Intentionally using a different calculation than the one from
             * figure 122 of the v1.1 product specification. This appears to
             * match real world performance better */
            radio_info->rssi = (int16_t)hwlqi + ED_RSSIOFFS;

The datasheet says:

PRF[dBm] = ED_RSSIOFFS + ED_RSSISCALE x VALHARDWARE

I really think we should use the one from the datasheet

But isn't it weird that in @benpicco test results we get rssi's of 40dbm and then -52dbm, that seems weird.

fjmolinas · 2021-01-13T07:53:17Z

well... the original nrf802154 driver (nrf802154.c) has a different implementation of RSSI that's not compliant with the datasheet:
            /* Calculate RSSI by subtracting the offset from the datasheet.
             * Intentionally using a different calculation than the one from
             * figure 122 of the v1.1 product specification. This appears to
             * match real world performance better */
            radio_info->rssi = (int16_t)hwlqi + ED_RSSIOFFS;
The datasheet says:
PRF[dBm] = ED_RSSIOFFS + ED_RSSISCALE x VALHARDWARE
I really think we should use the one from the datasheet
But isn't it weird that in @benpicco test results we get rssi's of 40dbm and then -52dbm, that seems weird.

Actually doesn't it also need the corrections in #15616?

fjmolinas · 2021-01-13T08:43:48Z

Pinging between two nrf52840-mdk looks fine on my side at least rebased on #156616

2021-01-13 09:38:12,419 #  ping6 fe80::6cd6:5bb6:4b52:cd2f -c 20
2021-01-13 09:38:12,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=0 ttl=64 rssi=-56 dBm time=8.153 ms
2021-01-13 09:38:13,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=1 ttl=64 rssi=-56 dBm time=7.513 ms
2021-01-13 09:38:14,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=2 ttl=64 rssi=-56 dBm time=7.834 ms
2021-01-13 09:38:15,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=3 ttl=64 rssi=-56 dBm time=6.873 ms
2021-01-13 09:38:16,436 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=4 ttl=64 rssi=-56 dBm time=9.113 ms
2021-01-13 09:38:17,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=5 ttl=64 rssi=-56 dBm time=5.887 ms
2021-01-13 09:38:18,439 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=6 ttl=64 rssi=-56 dBm time=13.084 ms
2021-01-13 09:38:19,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=7 ttl=64 rssi=-56 dBm time=5.567 ms
2021-01-13 09:38:20,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=8 ttl=64 rssi=-72 dBm time=7.513 ms
2021-01-13 09:38:21,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=9 ttl=64 rssi=-56 dBm time=6.846 ms
2021-01-13 09:38:22,436 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=10 ttl=64 rssi=-56 dBm time=9.753 ms
2021-01-13 09:38:23,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=11 ttl=64 rssi=-56 dBm time=7.833 ms
2021-01-13 09:38:24,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=12 ttl=64 rssi=-56 dBm time=5.567 ms
2021-01-13 09:38:25,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=13 ttl=64 rssi=-56 dBm time=5.913 ms
2021-01-13 09:38:26,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=14 ttl=64 rssi=-56 dBm time=7.513 ms
2021-01-13 09:38:27,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=15 ttl=64 rssi=-56 dBm time=8.154 ms
2021-01-13 09:38:28,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=16 ttl=64 rssi=-72 dBm time=7.833 ms
2021-01-13 09:38:29,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=17 ttl=64 rssi=-72 dBm time=7.513 ms
2021-01-13 09:38:30,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=18 ttl=64 rssi=-56 dBm time=6.874 ms
2021-01-13 09:38:31,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=19 ttl=64 rssi=-56 dBm time=7.166 ms
2021-01-13 09:38:31,434 #

fjmolinas · 2021-01-13T08:55:08Z

Pinging between two nrf52840-mdk looks fine on my side at least rebased on #156616

2021-01-13 09:38:12,419 #  ping6 fe80::6cd6:5bb6:4b52:cd2f -c 20
2021-01-13 09:38:12,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=0 ttl=64 rssi=-56 dBm time=8.153 ms
2021-01-13 09:38:13,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=1 ttl=64 rssi=-56 dBm time=7.513 ms
2021-01-13 09:38:14,435 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=2 ttl=64 rssi=-56 dBm time=7.834 ms
2021-01-13 09:38:15,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=3 ttl=64 rssi=-56 dBm time=6.873 ms
2021-01-13 09:38:16,436 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=4 ttl=64 rssi=-56 dBm time=9.113 ms
2021-01-13 09:38:17,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=5 ttl=64 rssi=-56 dBm time=5.887 ms
2021-01-13 09:38:18,439 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=6 ttl=64 rssi=-56 dBm time=13.084 ms
2021-01-13 09:38:19,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=7 ttl=64 rssi=-56 dBm time=5.567 ms
2021-01-13 09:38:20,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=8 ttl=64 rssi=-72 dBm time=7.513 ms
2021-01-13 09:38:21,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=9 ttl=64 rssi=-56 dBm time=6.846 ms
2021-01-13 09:38:22,436 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=10 ttl=64 rssi=-56 dBm time=9.753 ms
2021-01-13 09:38:23,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=11 ttl=64 rssi=-56 dBm time=7.833 ms
2021-01-13 09:38:24,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=12 ttl=64 rssi=-56 dBm time=5.567 ms
2021-01-13 09:38:25,432 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=13 ttl=64 rssi=-56 dBm time=5.913 ms
2021-01-13 09:38:26,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=14 ttl=64 rssi=-56 dBm time=7.513 ms
2021-01-13 09:38:27,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=15 ttl=64 rssi=-56 dBm time=8.154 ms
2021-01-13 09:38:28,434 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=16 ttl=64 rssi=-72 dBm time=7.833 ms
2021-01-13 09:38:29,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=17 ttl=64 rssi=-72 dBm time=7.513 ms
2021-01-13 09:38:30,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=18 ttl=64 rssi=-56 dBm time=6.874 ms
2021-01-13 09:38:31,433 # 12 bytes from fe80::6cd6:5bb6:4b52:cd2f%6: icmp_seq=19 ttl=64 rssi=-56 dBm time=7.166 ms
2021-01-13 09:38:31,434 #

I do get the same wrong results if only testing this PR though. Maybe we should merge that one before.

maribu

ACK. If you agree with the style nit, squash in right away.

Also: As this is a dependency for a bug fix PR, I think this should go in despite soft feature freeze. (And this PR isn't scary either, especially due to proper unit testing.)

sys/include/net/ieee802154.h

jia200x · 2021-01-13T13:02:23Z

done! I will need a re-ACK I guess :)

fjmolinas · 2021-01-13T13:09:15Z

done! I will need a re-ACK I guess :)

No need, the change is only cosmetic.

fjmolinas · 2021-01-13T13:09:35Z

@jia200x you will need to rebase to master there is a missing file that is causing the fail in check-pr

jia200x · 2021-01-13T13:14:57Z

done!

maribu

re-ACK ;-) (But I agree with @fjmolinas, that no re-ACK was need ;-))

jia200x · 2021-01-13T13:53:42Z

I thought GH would block merging since the code changed in between

jia200x · 2021-01-13T13:53:51Z

thanks for the review!

jia200x requested a review from miri64 as a code owner December 14, 2020 12:47

jia200x requested review from benpicco and fjmolinas December 14, 2020 12:47

jia200x force-pushed the pr/ieee802154_rssi_dbm branch from 9115c73 to a081c6e Compare December 14, 2020 12:54

benpicco added Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Dec 14, 2020

jia200x mentioned this pull request Dec 15, 2020

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

Merged

maribu approved these changes Jan 13, 2021

View reviewed changes

sys/include/net/ieee802154.h Outdated Show resolved Hide resolved

jia200x force-pushed the pr/ieee802154_rssi_dbm branch from a081c6e to 51b6ce8 Compare January 13, 2021 13:02

jia200x added 3 commits January 13, 2021 14:14

ieee802154: add helpers for RSSI and dBm conversion

49f4ecc

tests/unittest/ieee802154: add tests for RSSI and dBm conversion

59d5a80

netdev_ieee802154_submac: use RSSI in dBm

728b84d

jia200x force-pushed the pr/ieee802154_rssi_dbm branch from 51b6ce8 to 728b84d Compare January 13, 2021 13:15

maribu approved these changes Jan 13, 2021

View reviewed changes

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

ieee802154: add helpers for RSSI and dBm conversion #15629

ieee802154: add helpers for RSSI and dBm conversion #15629

Uh oh!

Conversation

jia200x commented Dec 14, 2020

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

benpicco commented Dec 14, 2020

Uh oh!

jia200x commented Dec 14, 2020

Uh oh!

jia200x commented Dec 15, 2020

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

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!

Uh oh!

jia200x commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

fjmolinas commented Jan 13, 2021

Uh oh!

jia200x commented Jan 13, 2021

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

jia200x commented Jan 13, 2021

Uh oh!

jia200x commented Jan 13, 2021

Uh oh!

Uh oh!