ieee802154/hal: adapt to latest changes of #13943

[jia200x]

Contribution description

This PR implements the latest proposals of the Radio HAL described in #13943 (comment).

• A request_op and confirm_op mechanism in order to simplify the TX logic
  • xxx_transmit are not needed anymore
  • CCA and set_trx_state functions were merged into these functions
  • TX_ON state was removed and TRX_OFF was renamed to IDLE.
• A mechanism to force transition to IDLE (TRX_OFF).
• Assume the transceiver is turned off on reception

This changes make drivers easier to implemented and provide extra guarantees to both the driver and upper layer. I tested this one with the ongoing port of OpenDSME and it seems to be compatible.

Testing procedure

A stress ping test would be optimal. Also, make sure tests/ieee802154_hal work as expected.

Here are some stress ping results:

cc2538_rf:

2021-10-04 12:15:08,104 # --- fe80::204:2519:1801:bddb PING statistics ---
2021-10-04 12:15:08,110 # 10000 packets transmitted, 9974 packets received, 0% packet loss
2021-10-04 12:15:08,115 # round-trip min/avg/max = 132.906/146.836/174.760 ms

nrf802154:

2021-09-27 20:12:58,881 # --- fe80::204:2519:1801:bddb PING statistics ---
2021-09-27 20:12:58,887 # 60000 packets transmitted, 59192 packets received, 1% packet loss
2021-09-27 20:12:58,892 # round-trip min/avg/max = 128.335/148.738/172.137 ms

Issues/PRs references

#13943

Although on-going Radio HAL PRs (e.g #16535 or #13943 would require a minor API change, this change should be straightforward.

Depends on #16821

[benpicco]

needs a rebase 😉

[jia200x]

done!

[benpicco]

Looks good, but Murdock is not happy

[benpicco]

ping?

[jia200x]

addressed all comments

[benpicco]

Murdock still has some comments

[jia200x]

fixed it. 43e40f0 was wrong

[benpicco]

Murdock is still not happy

[fjmolinas]

ping @jia200x

[jia200x]

it should work now

[jia200x]

oh... I totally forgot the OpenWSN functions :/... I need to update that first

[jia200x]

@fjmolinas I updated the HAL port of OpenWSN. Could you please check if it still works as expected?

[benpicco]

looks like socket_zep needs an update 😇

[jia200x]

looks like socket_zep needs an update innocent

yup... luckily only an arrange of functions :)

[jia200x]

@benpicco I adapted socket_zep but make test fails:

RIOT/tests/socket_zep/bin/native/tests_socket_zep.elf -z [::]:12345,[::1]:17754 /dev/ttyACM0 
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: 2022.01-devel-1317-g2422f-pr/hal/v_10_04)
Socket ZEP device driver test
Initializing socket ZEP with (local: [::]:12345, remote: [::1]:17754)
(Hwaddrs: 4c20, bab0279b993ecc20)
Send zero-length packet
Send 'Hello\0World\0'
Waiting for an incoming message (use `make test`)

Traceback (most recent call last):
  File "RIOT/tests/socket_zep/tests/01-run.py", line 62, in <module>
    res = run(testfunc, timeout=1, echo=True, traceback=True)
  File "RIOT/dist/pythonlibs/testrunner/__init__.py", line 30, in run
    testfunc(child)
  File "RIOT/tests/socket_zep/tests/01-run.py", line 40, in testfunc
    assert(len(data) == (ZEP_DATA_HEADER_SIZE + FCS_LEN))
AssertionError
make: *** [RIOT/makefiles/tests/tests.inc.mk:22: test] Error 1

Any idea?

[benpicco]

This is what fails.

Looks like the HAL does not send packets with an empty payload.

The RX test appears to fail because the address does not match anymore.

socket_zep::_socket_isr: bytes on 8
socket_zep::len 66 bytes on 8
socket_zep::len 66 bytes on 8
socket_zep::read: reading up to 127 bytes into 0x805b320
socket_zep::read: got 66 bytes
socket_zep::read: dst not me

Unfortunately the test is not run by CI because Murdock does not support IPv6, so it was not discovered earlier. This should be fixed with

this patch

diff --git a/tests/socket_zep/Makefile b/tests/socket_zep/Makefile
index 0073b01ed6..b7903776d3 100644
--- a/tests/socket_zep/Makefile
+++ b/tests/socket_zep/Makefile
@@ -2,14 +2,10 @@ include ../Makefile.tests_common
 
 BOARD_WHITELIST = native    # socket_zep is only available on native
 
-# Cannot run the test on `murdock`
-#   ZEP: Unable to connect socket: Cannot assign requested address
-TEST_ON_CI_BLACKLIST += native
-
 USEMODULE += od
 USEMODULE += socket_zep
 USEMODULE += netdev
 
-TERMFLAGS ?= -z [::1]:17754
+TERMFLAGS ?= -z [127.0.0.1]:17754
 
 include $(RIOTBASE)/Makefile.include
diff --git a/tests/socket_zep/tests/01-run.py b/tests/socket_zep/tests/01-run.py
index 843f3421b7..6196394723 100755
--- a/tests/socket_zep/tests/01-run.py
+++ b/tests/socket_zep/tests/01-run.py
@@ -17,9 +17,9 @@ ZEP_DATA_HEADER_SIZE = 32
 FCS_LEN = 2
 RCVBUF_LEN = IEEE802154_FRAME_LEN_MAX + ZEP_DATA_HEADER_SIZE + FCS_LEN
 zep_params = {
-        "local_addr": "::",
+        "local_addr": "127.0.0.1",
         "local_port": 12345,
-        "remote_addr": "::1",
+        "remote_addr": "127.0.0.1",
         "remote_port": 17754,
     }
 s = None
@@ -28,8 +28,8 @@ s = None
 def testfunc(child):
     child.expect_exact("Socket ZEP device driver test")
     child.expect(r"Initializing socket ZEP with " +
-                 r"\(local: \[(?P<local_addr>[:0-9a-f]+)\]:(?P<local_port>\d+), " +
-                 r"remote: \[(?P<remote_addr>[:0-9a-f]+)\]:(?P<remote_port>\d+)\)")
+                 r"\(local: \[(?P<local_addr>[.0-9a-f]+)\]:(?P<local_port>\d+), " +
+                 r"remote: \[(?P<remote_addr>[.0-9a-f]+)\]:(?P<remote_port>\d+)\)")
     assert(child.match.group('local_addr') == zep_params['local_addr'])
     assert(int(child.match.group('local_port')) == zep_params['local_port'])
     assert(child.match.group('remote_addr') == zep_params['remote_addr'])

Then specify the expected MAC address with TERMFLAGS += --eui64=$(ZEP_MAC) in the Makefile.

[benpicco]

Please squash, the test should be fixed now.

[jia200x]

Looks like the HAL does not send packets with an empty payload.

Hmmm the HAL documentation states that the frame is a valid PSDU (full MAC frame). I'm not sure if an empty PSDU is a valid frame, but what I find strange is that the implementation should have (accidentally) handled that.
Anyway, I guess it should be fixed with the last patch.

[jia200x]

squashed!

[jia200x]

awesome! thank you for the review!