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.

Sign up for GitHub

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

Jump to bottom

cpu/nrf52: fix RSSI calculation in nrf802154_radio #20839

Merged
merged 1 commit into from
Sep 6, 2024
Merged

cpu/nrf52: fix RSSI calculation in nrf802154_radio #20839

merged 1 commit into from
Sep 6, 2024

Conversation

mguetschow
Copy link
Contributor

Contribution description

As detailed in #20803, The calculation of RSSI based on the hardware-provided LQI value on master is wrong. The documentation states that

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.

without further detailing the applied mapping.

Reusing the RSSI measurement functionality, I was able to find out the (reverse) mapping with the diff below.

Testing procedure

Apply the following diff enabling automatic RSSI measurement during packet reception:

diff --git a/cpu/nrf52/radio/nrf802154/nrf802154_radio.c b/cpu/nrf52/radio/nrf802154/nrf802154_radio.c
index 37fcba3fab..ccaf44d8a9 100644
--- a/cpu/nrf52/radio/nrf802154/nrf802154_radio.c
+++ b/cpu/nrf52/radio/nrf802154/nrf802154_radio.c
@@ -220,7 +220,7 @@ static int _request_op(ieee802154_dev_t *dev, ieee802154_hal_op_t op, void *ctx)
         _disable_blocking();
         state = STATE_RX;
         NRF_RADIO->PACKETPTR = (uint32_t) rxbuf;
-        NRF_RADIO->SHORTS = DEFAULT_SHORTS;
+        NRF_RADIO->SHORTS = DEFAULT_SHORTS | RADIO_SHORTS_ADDRESS_RSSISTART_Msk;
         NRF_RADIO->TASKS_RXEN = 1;
         break;
     case IEEE802154_HAL_OP_SET_IDLE: {
@@ -343,6 +343,7 @@ static int _read(ieee802154_dev_t *dev, void *buf, size_t max_size,
                through comparison with the RSSI value provided by NRF_RADIO->RSSISAMPLE
                after enabling the ADDRESS_RSSISTART short. */
             int8_t rssi_dbm = hwlqi + ED_RSSIOFFS - 1;
+            printf("real: %ld - calc: %d\n", -NRF_RADIO->RSSISAMPLE, rssi_dbm);
             radio_info->rssi = ieee802154_dbm_to_rssi(rssi_dbm);
         }
         memcpy(buf, &rxbuf[1], pktlen);
@@ -436,6 +437,7 @@ static void _timer_cb(void *arg, int chan)
 
         ack[3] = rxbuf[3];
 
+        NRF_RADIO->SHORTS = DEFAULT_SHORTS;
         NRF_RADIO->PACKETPTR = (uint32_t) &ack;
         NRF_RADIO->TASKS_TXEN = 1;
         dev->cb(dev, IEEE802154_RADIO_INDICATION_RX_DONE);

Flash and send some packets from a different node:

$ make -C examples/gnrc_networking BOARD=nrf52840dk flash term
make: Entering directory '/home/mikolai/TUD/Code/RIOT/examples/gnrc_networking'
Building application "gnrc_networking" for "nrf52840dk" with CPU "nrf52".

"make" -C /home/mikolai/TUD/Code/RIOT/pkg/cmsis/ 
"make" -C /home/mikolai/TUD/Code/RIOT/boards/common/init
"make" -C /home/mikolai/TUD/Code/RIOT/boards/nrf52840dk
"make" -C /home/mikolai/TUD/Code/RIOT/boards/common/nrf52xxxdk
"make" -C /home/mikolai/TUD/Code/RIOT/core
"make" -C /home/mikolai/TUD/Code/RIOT/core/lib
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf52
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/cortexm_common
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/cortexm_common/periph
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf52/periph
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf52/radio/nrf802154
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf52/vectors
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf5x_common
"make" -C /home/mikolai/TUD/Code/RIOT/cpu/nrf5x_common/periph
"make" -C /home/mikolai/TUD/Code/RIOT/drivers
"make" -C /home/mikolai/TUD/Code/RIOT/drivers/netdev
"make" -C /home/mikolai/TUD/Code/RIOT/drivers/netdev_ieee802154_submac
"make" -C /home/mikolai/TUD/Code/RIOT/drivers/periph_common
"make" -C /home/mikolai/TUD/Code/RIOT/sys
"make" -C /home/mikolai/TUD/Code/RIOT/sys/auto_init
"make" -C /home/mikolai/TUD/Code/RIOT/sys/div
"make" -C /home/mikolai/TUD/Code/RIOT/sys/event
"make" -C /home/mikolai/TUD/Code/RIOT/sys/evtimer
"make" -C /home/mikolai/TUD/Code/RIOT/sys/fmt
"make" -C /home/mikolai/TUD/Code/RIOT/sys/frac
"make" -C /home/mikolai/TUD/Code/RIOT/sys/isrpipe
"make" -C /home/mikolai/TUD/Code/RIOT/sys/libc
"make" -C /home/mikolai/TUD/Code/RIOT/sys/luid
"make" -C /home/mikolai/TUD/Code/RIOT/sys/malloc_thread_safe
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/crosslayer/inet_csum
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netapi
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netif
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netif/hdr
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netif/ieee802154
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netif/init_devs
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netif/pktq
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/netreg
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/icmpv6
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/icmpv6/echo
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/icmpv6/error
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/ipv6
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/ipv6/hdr
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/ipv6/nib
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/ndp
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/ctx
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/frag
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/frag/fb
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/frag/rb
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/iphc
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/network_layer/sixlowpan/nd
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/pkt
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/pktbuf
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/pktbuf_static
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/pktdump
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/routing/rpl
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/gnrc/transport_layer/udp
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/link_layer/eui_provider
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/link_layer/ieee802154
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/link_layer/l2util
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/netif
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/netutils
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/network_layer/icmpv6
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/network_layer/ipv6/addr
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/network_layer/ipv6/hdr
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/network_layer/sixlowpan
"make" -C /home/mikolai/TUD/Code/RIOT/sys/net/transport_layer/udp
"make" -C /home/mikolai/TUD/Code/RIOT/sys/newlib_syscalls_default
"make" -C /home/mikolai/TUD/Code/RIOT/sys/od
"make" -C /home/mikolai/TUD/Code/RIOT/sys/preprocessor
"make" -C /home/mikolai/TUD/Code/RIOT/sys/ps
"make" -C /home/mikolai/TUD/Code/RIOT/sys/random
"make" -C /home/mikolai/TUD/Code/RIOT/sys/shell
"make" -C /home/mikolai/TUD/Code/RIOT/sys/shell/cmds
"make" -C /home/mikolai/TUD/Code/RIOT/sys/stdio
"make" -C /home/mikolai/TUD/Code/RIOT/sys/stdio_uart
"make" -C /home/mikolai/TUD/Code/RIOT/sys/trickle
"make" -C /home/mikolai/TUD/Code/RIOT/sys/tsrb
"make" -C /home/mikolai/TUD/Code/RIOT/sys/ztimer
   text    data     bss     dec     hex filename
  95752     224   18768  114744   1c038 /home/mikolai/TUD/Code/RIOT/examples/gnrc_networking/bin/nrf52840dk/gnrc_networking.elf
/home/mikolai/TUD/Code/RIOT/dist/tools/jlink/jlink.sh flash /home/mikolai/TUD/Code/RIOT/examples/gnrc_networking/bin/nrf52840dk/gnrc_networking.elf
### Flashing Target ###
### Flashing elf file ###
SEGGER J-Link Commander V7.94e (Compiled Jan 15 2024 15:19:58)
DLL version V7.94e, compiled Jan 15 2024 15:19:33

J-Link Commander will now exit on Error

J-Link Command File read successfully.
Processing script file...
J-Link>loadfile /home/mikolai/TUD/Code/RIOT/examples/gnrc_networking/bin/nrf52840dk/gnrc_networking.elf
J-Link connection not established yet but required for command.
Connecting to J-Link via USB...O.K.
Firmware: J-Link OB-nRF5340-NordicSemi compiled Oct 30 2023 12:13:06
Hardware version: V1.00
J-Link uptime (since boot): 0d 00h 01m 22s
S/N: 1050216542
License(s): RDI, FlashBP, FlashDL, JFlash, GDB
USB speed mode: Full speed (12 MBit/s)
VTref=3.300V
Target connection not established yet but required for command.
Device "NRF52840_XXAA" selected.


Connecting to target via SWD
InitTarget() start
InitTarget() end - Took 3.08ms
Found SW-DP with ID 0x2BA01477
DPIDR: 0x2BA01477
CoreSight SoC-400 or earlier
Scanning AP map to find all available APs
AP[2]: Stopped AP scan as end of AP map has been reached
AP[0]: AHB-AP (IDR: 0x24770011)
AP[1]: JTAG-AP (IDR: 0x02880000)
Iterating through AP map to find AHB-AP to use
AP[0]: Core found
AP[0]: AHB-AP ROM base: 0xE00FF000
CPUID register: 0x410FC241. Implementer code: 0x41 (ARM)
Found Cortex-M4 r0p1, Little endian.
Cortex-M: The connected J-Link (S/N 1050216542) uses an old firmware module: V1 (current is 2)
FPUnit: 6 code (BP) slots and 2 literal slots
CoreSight components:
ROMTbl[0] @ E00FF000
[0][0]: E000E000 CID B105E00D PID 000BB00C SCS-M7
[0][1]: E0001000 CID B105E00D PID 003BB002 DWT
[0][2]: E0002000 CID B105E00D PID 002BB003 FPB
[0][3]: E0000000 CID B105E00D PID 003BB001 ITM
[0][4]: E0040000 CID B105900D PID 000BB9A1 TPIU
[0][5]: E0041000 CID B105900D PID 000BB925 ETM
Memory zones:
  Zone: "Default" Description: Default access mode
Cortex-M4 identified.
'loadfile': Performing implicit reset & halt of MCU.
Reset: Halt core after reset via DEMCR.VC_CORERESET.
Reset: Reset device via AIRCR.SYSRESETREQ.
Downloading file [/home/mikolai/TUD/Code/RIOT/examples/gnrc_networking/bin/nrf52840dk/gnrc_networking.elf]...
Comparing flash   [100%] Done.
Erasing flash     [100%] Done.
Programming flash [100%] Done.
J-Link: Flash download: Bank 0 @ 0x00000000: 1 range affected (98304 bytes)
J-Link: Flash download: Total: 3.180s (Prepare: 0.063s, Compare: 0.029s, Erase: 2.001s, Program & Verify: 1.043s, Restore: 0.042s)
J-Link: Flash download: Program & Verify speed: 92 KB/s
O.K.
J-Link>r
Reset delay: 0 ms
Reset type NORMAL: Resets core & peripherals via SYSRESETREQ & VECTRESET bit.
Reset: Halt core after reset via DEMCR.VC_CORERESET.
Reset: Reset device via AIRCR.SYSRESETREQ.
J-Link>g
Memory map 'after startup completion point' is active
J-Link>exit

Script processing completed.

/home/mikolai/TUD/Code/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-mikolai" -rn "2024-08-28_11.42.58-gnrc_networking-nrf52840dk"  
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2024-08-28 11:42:58,584 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2024-08-28 11:42:59,587 # NETOPT_TX_END_IRQ not implemented by driver
2024-08-28 11:42:59,588 # main(): This is RIOT! (Version: 2024.07-devel-543-g93395e-nrf802154-rssi)
2024-08-28 11:42:59,588 # RIOT network stack example application
2024-08-28 11:42:59,589 # All up, running the shell now
> 2024-08-28 11:42:59,589 # real: -45 - calc: -45
2024-08-28 11:43:00,198 # real: -45 - calc: -44
2024-08-28 11:43:01,198 # real: -45 - calc: -45
2024-08-28 11:43:01,345 # real: -45 - calc: -45
2024-08-28 11:43:02,198 # real: -45 - calc: -44
2024-08-28 11:43:03,197 # real: -45 - calc: -45
2024-08-28 11:43:04,198 # real: -45 - calc: -45
2024-08-28 11:43:05,198 # real: -45 - calc: -45
2024-08-28 11:43:06,197 # real: -45 - calc: -45
2024-08-28 11:43:07,197 # real: -46 - calc: -46
2024-08-28 11:43:08,197 # real: -45 - calc: -45
2024-08-28 11:43:09,199 # real: -45 - calc: -45
2024-08-28 11:43:10,199 # real: -45 - calc: -45
2024-08-28 11:43:11,197 # real: -45 - calc: -44
2024-08-28 11:43:12,197 # real: -45 - calc: -44
2024-08-28 11:43:13,199 # real: -45 - calc: -45
2024-08-28 11:43:14,198 # real: -45 - calc: -44
2024-08-28 11:43:15,198 # real: -45 - calc: -45
2024-08-28 11:43:16,198 # real: -45 - calc: -45
2024-08-28 11:43:17,198 # real: -56 - calc: -55
2024-08-28 11:43:18,198 # real: -31 - calc: -31
2024-08-28 11:43:19,198 # real: -32 - calc: -33
2024-08-28 11:43:20,199 # real: -35 - calc: -35
2024-08-28 11:43:21,198 # real: -34 - calc: -34
2024-08-28 11:43:22,197 # real: -46 - calc: -46
2024-08-28 11:43:23,199 # real: -52 - calc: -52
2024-08-28 11:43:24,198 # real: -46 - calc: -47
2024-08-28 11:43:25,197 # real: -46 - calc: -45
2024-08-28 11:43:26,198 # real: -52 - calc: -52
2024-08-28 11:43:27,199 # real: -75 - calc: -75
2024-08-28 11:43:28,198 # real: -81 - calc: -81
2024-08-28 11:43:29,199 # real: -77 - calc: -77
2024-08-28 11:43:30,198 # real: -78 - calc: -78
2024-08-28 11:43:31,197 # real: -84 - calc: -85
2024-08-28 11:43:32,197 # real: -81 - calc: -80
2024-08-28 11:43:33,198 # real: -79 - calc: -79
2024-08-28 11:43:34,197 # real: -44 - calc: -44
2024-08-28 11:43:35,199 # real: -42 - calc: -42
2024-08-28 11:43:36,198 # real: -45 - calc: -45
2024-08-28 11:43:37,199 # real: -44 - calc: -44
2024-08-28 11:43:38,199 # real: -64 - calc: -64
2024-08-28 11:43:39,199 # real: -73 - calc: -73
2024-08-28 11:43:40,198 # real: -82 - calc: -82
2024-08-28 11:43:41,198 # real: -66 - calc: -78
2024-08-28 11:43:42,207 # real: -80 - calc: -79
2024-08-28 11:43:43,197 # real: -62 - calc: -63
2024-08-28 11:43:43,350 # real: -62 - calc: -62
2024-08-28 11:43:44,199 # real: -56 - calc: -57
2024-08-28 11:43:45,198 # real: -46 - calc: -47
2024-08-28 11:43:46,199 # real: -57 - calc: -57
2024-08-28 11:43:47,199 # real: -52 - calc: -52
2024-08-28 11:43:48,198 # real: -53 - calc: -53
2024-08-28 11:43:49,199 # real: -61 - calc: -61
2024-08-28 11:43:50,199 # real: -56 - calc: -56
2024-08-28 11:43:51,199 # real: -60 - calc: -59
2024-08-28 11:43:52,199 # real: -60 - calc: -60
2024-08-28 11:43:53,198 # real: -62 - calc: -62
2024-08-28 11:43:54,202 # real: -64 - calc: -64
2024-08-28 11:43:55,199 # real: -61 - calc: -61
2024-08-28 11:43:56,200 # real: -60 - calc: -60
2024-08-28 11:43:57,198 # real: -45 - calc: -45
2024-08-28 11:43:58,199 # real: -51 - calc: -51

The RSSI value (given in dBm) calculated from the (hardware-provided) LQI value (calc) closely matches the reported RSSI value (real).

Alternatives considered

We could use the normal RSSI measurement approach as in the diff above to get the actual RSSI value. But since calculating RSSI back from LQI seems to work fine, I'd propose to skip that (potential) overhead.

Issues/PRs references

Fixes #20803

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Aug 28, 2024
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 28, 2024
@riot-ci
Copy link

riot-ci commented Aug 28, 2024 •
edited
Loading

Murdock results

✔️ PASSED

076dcef cpu/nrf52: fix RSSI calculation in nrf802154_radio

Success Failures Total Runtime
10193 0 10193 14m:46s

Artifacts

@benpicco
Copy link
Contributor

What's the drawback of always setting RADIO_SHORTS_ADDRESS_RSSISTART_Msk to measure the real RSSI value?

@mguetschow
Copy link
Contributor Author

What's the drawback of always setting RADIO_SHORTS_ADDRESS_RSSISTART_Msk to measure the real RSSI value?

I don't really see the necessity if we can deduce it from LQI anyways and I would expect it to be a minor overhead in hardware which we could avoid. But that feeling would certainly need some benchmarking to be backed up or falsified.

I'm not opposed to going that route instead.

@KSKNico
Copy link
Contributor

KSKNico commented Sep 6, 2024

I have tested the PR on two nrf802154 and can confirm that it works as intended.

@benpicco benpicco enabled auto-merge September 6, 2024 14:23
@benpicco benpicco added this pull request to the merge queue Sep 6, 2024
Merged via the queue into RIOT-OS:master with commit ed9faa9 Sep 6, 2024
27 checks passed
@mguetschow mguetschow deleted the nrf802154-rssi branch September 6, 2024 21:30
@mguetschow
Copy link
Contributor Author

Thanks for your support both of you!

@benpicco benpicco added this to the Release 2024.10 milestone Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@miri64 miri64 Awaiting requested review from miri64

@Teufelchen1 Teufelchen1 Awaiting requested review from Teufelchen1

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Milestone
Release 2024.10
Development

Successfully merging this pull request may close these issues.

Wrong RSSI calculation for the nrf802154
4 participants
@mguetschow @riot-ci @benpicco @KSKNico