cc2538_rf: implement Radio HAL #14791

jia200x · 2020-08-19T12:13:13Z

Contribution description

This PR adds an ieee802154_hal implementation for the CC2538 radios. It's based on the work of #14655. It also includes the definition of an ieee802154_radio_hal module to be able to switch between netdev and ieee802154_hal implementations (they are mutually exclusive).

The test application was adapted from #14655 for testing this radio. When more radios are there, the test application will be expanded.

Testing procedure

Run tests/ieee802154_hal test. Ideally this should be tested between 2 CC2538 radios. If that's not possible, it should be enough to use an AT86RF2XX or NRF52840 with #14655.

Test setting channel and TX power with the config_phy command:

2020-08-12 18:24:49,390 #  config_phy
2020-08-12 18:24:49,393 # Usage: config_phy <channel> <tx_pow>

An out of range value for TX power should give an error

Send data between nodes using the txtsnd command.

Usage: txtsnd <long_addr> <len>

It's possible to get the long address with the print_addr command.

Perform CCa with the cca command:

2020-08-12 18:28:57,628 #  cca
2020-08-12 18:28:57,629 # CLEAR
> 2020-08-12 18:28:57,827 #  cca
2020-08-12 18:28:57,828 # CLEAR
> 2020-08-12 18:28:58,157 #  cca
2020-08-12 18:28:58,158 # CLEAR
> 2020-08-12 18:28:58,334 #  cca
2020-08-12 18:28:58,335 # BUSY
> 2020-08-12 18:28:58,473 #  cca
2020-08-12 18:28:58,474 # CLEAR

Change the CCA mode and the threshold with the config_cca command:

config_cca <ed|cs|or|and> [ED threshold]>

Note the ED is in dBm. The ED threshold doesn't affect the Carrier Sense mode (CCA mode 2).

Try different TX modes (CSMA-CA, CCA, direct transmission) using the tx_mode command:

Usage: tx_mode <csma_ca|cca|direct>

Try different RX states (ACK on, ACK disabled, ACK with Frame Pending or Promiscuous mode) with the rx_mode command:

Usage: rx_mode <on|off|pend|promisc>

Use a sniffer to check if the receiver sends ACK. As expected, a radio with Promiscuous Mode will receive all kind of packets with correct FCS.

Use the caps command to get the available caps for a given device:

2020-08-19 14:24:46,502 #  caps
2020-08-19 14:24:46,503 #                CAPS              
2020-08-19 14:24:46,505 # =================================
2020-08-19 14:24:46,507 # - Frame Retransmissions:        n
2020-08-19 14:24:46,509 #     * Info retries:             n
2020-08-19 14:24:46,511 # - Auto CSMA-CA:                 y
2020-08-19 14:24:46,513 # - 2.4 GHz band:                 y
2020-08-19 14:24:46,515 # - SubGHz band:                  n
2020-08-19 14:24:46,516 # - TX DONE indication:           y
2020-08-19 14:24:46,518 #     * ACK Timeout indication:   n
2020-08-19 14:24:46,519 # - RX_START indication:          n
2020-08-19 14:24:46,521 # - CCA Done indication:          y

Use the test_states command to test state transitions and measure transition time. The important thing here is the TX-RX turnaround time (it should be equal or less than 12 symbols, 192 us, in order to comply with LIFS frames like the ACK):

2020-08-19 14:09:52,563 # test_states
2020-08-19 14:09:52,564 # Setting state to: TRX_OFF
2020-08-19 14:09:52,565 # 	Transition took 7 usecs
2020-08-19 14:09:52,566 # TRX_OFF-> RX
2020-08-19 14:09:52,578 # 	Transition took 194 usecs
2020-08-19 14:09:52,578 # RX-> TRX_OFF
2020-08-19 14:09:52,579 # 	Transition took 6 usecs
2020-08-19 14:09:52,580 # TRX_OFF-> TX
2020-08-19 14:09:52,581 # 	Transition took 7 usecs
2020-08-19 14:09:52,583 # TX-> TRX_OFF
2020-08-19 14:09:52,584 # 	Transition took 6 usecs
2020-08-19 14:09:52,584 # 
2020-08-19 14:09:52,585 # Setting state to: RX
2020-08-19 14:09:52,586 # 	Transition took 193 usecs
2020-08-19 14:09:52,587 # RX-> TRX_OFF
2020-08-19 14:09:52,588 # 	Transition took 7 usecs
2020-08-19 14:09:52,588 # TRX_OFF-> RX
2020-08-19 14:09:52,591 # 	Transition took 193 usecs
2020-08-19 14:09:52,591 # RX-> TX
2020-08-19 14:09:52,593 # 	Transition took 6 usecs
2020-08-19 14:09:52,593 # TX-> RX
2020-08-19 14:09:52,605 # 	Transition took 194 usecs
2020-08-19 14:09:52,606 # 
2020-08-19 14:09:52,607 # Setting state to: TX
2020-08-19 14:09:52,608 # 	Transition took 7 usecs
2020-08-19 14:09:52,609 # TX-> TRX_OFF
2020-08-19 14:09:52,610 # 	Transition took 6 usecs
2020-08-19 14:09:52,610 # TRX_OFF-> TX
2020-08-19 14:09:52,611 # 	Transition took 7 usecs
2020-08-19 14:09:52,612 # TX-> RX
2020-08-19 14:09:52,613 # 	Transition took 193 usecs
2020-08-19 14:09:52,613 # RX-> TX
2020-08-19 14:09:52,614 # 	Transition took 6 usecs
2020-08-19 14:09:52,614 # 
2020-08-19 14:09:52,616 # 	Transition took 193 usecs

Note this radio takes ~193 us to switch between RX-TX if the radio is not programmed to automatically go from TX to RX. However, since the symbol time is 16 us, this is not a problem for detecting the IEEE 802.15.4 preamble.

#14787 proves this :)

Issues/PRs references

#13943 and friends

benpicco · 2020-08-20T21:20:31Z

I wanted to test examples/gnrc_networking with the new HAL, so I added

USEMODULE += ieee802154
USEMODULE += ieee802154_radio_hal
USEMODULE += cc2538_rf

but I would only get

/usr/lib/gcc/arm-none-eabi/9.2.1/../../../arm-none-eabi/bin/ld: /home/benpicco/dev/RIOT/examples/gnrc_networking/bin/openmote-b/cc2538_rf/cc2538_rf.o: in function `cc2538_setup':
/home/benpicco/dev/RIOT/cpu/cc2538/radio/cc2538_rf.c:203: undefined reference to `cc2538_rf_driver'
collect2: error: ld returned 1 exit status
make: *** [/home/benpicco/dev/RIOT/examples/gnrc_networking/../../Makefile.include:587: /home/benpicco/dev/RIOT/examples/gnrc_networking/bin/openmote-b/gnrc_networking.elf] Error 1

is this expected?

benpicco

Ah this is only the lowest layer.
Well I can confirm that the old netdev part still works with examples/gnrc_networking.

With the test I was wondering 'why is this implemented in the test?'

Is the plan to move this to the submac layer eventually?

Looking good btw!

cpu/cc2538/include/cc2538_rf.h

cpu/cc2538/radio/cc2538_rf.c

benpicco · 2020-08-20T21:52:15Z

cpu/cc2538/radio/cc2538_rf.c

+    RFCORE_SFR_MTM0 |= TIMER_PERIOD_LSB;
+    RFCORE_SFR_MTM1 |= TIMER_PERIOD_MSB;
+
+    RFCORE_SFR_MTMSEL &= ~CC2538_SFR_MTMSEL_MASK;


Didn't you already clear that in line 146?

Yes, but because I had to set the 3 LSBs of RFCORE_SFR_MTMSEL to 001 (hence 146 and 148). For the next operation, I have to set those bits to 000 (so I have to clear it again).
If I do it the other way around the timer will start with the wrong configuration.

should I mark is as resolved?

Better add a comment.
If something turned out to need clarification during review, better add the explanation to the code so we don't lose that information. And future readers of the code who might have the same question shouldn't have to dig through old PRs in search of answers ;)

cpu/cc2538/radio/cc2538_rf_radio_ops.c

tests/ieee802154_hal/main.c

jia200x · 2020-08-31T08:16:18Z

Is the plan to move this to the submac layer eventually?

Yes, exactly! Most of the stuff here are handled by the SubMAC :)

cpu/cc2538/radio/cc2538_rf_radio_ops.c

jia200x · 2020-08-31T12:58:45Z

I think I addressed all comments

jia200x · 2020-08-31T13:04:51Z

I also added the Kconfig symbols for the CSMA-CA params.

cpu/cc2538/radio/cc2538_rf_radio_ops.c

jia200x · 2020-09-03T16:06:48Z

@benpicco done! :)

benpicco · 2020-09-03T16:15:12Z

Please squash! (& rebase)

benpicco

I did some regression tests and the old netdev code path is still working.
So let's move on with this!

jia200x · 2020-09-03T16:22:31Z

rebased and squashed!

benpicco · 2020-09-03T16:45:17Z

Murdock is not happy

jia200x · 2020-09-04T08:38:45Z

Fixed.

I also adapted to the changes from #14938 and removed references to other radios in the Makefile (only the cc2538_rf is supported so far, when the other radios are there we can update the Makefile)

jia200x · 2020-09-04T10:07:20Z

thanks for the review!

jia200x requested review from aabadie, fjmolinas, leandrolanzieri, MichelRottleuthner and smlng as code owners August 19, 2020 12:13

jia200x changed the title ~~Pr/cc2538 radio hal~~ cc2538_rf: implement Radio HAL Aug 19, 2020

jia200x requested review from benpicco and maribu August 19, 2020 12:14

jia200x added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers Area: network Area: Networking labels Aug 19, 2020

jia200x mentioned this pull request Aug 19, 2020

[tracker] ieee802154_hal: implementation status #14792

Open

8 tasks

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 19, 2020

jia200x force-pushed the pr/cc2538_radio_hal branch from 0b74149 to 7bd8ba3 Compare August 20, 2020 08:38

jia200x mentioned this pull request Aug 20, 2020

nrf52840: implement Radio HAL #14802

Merged

benpicco reviewed Aug 20, 2020

View reviewed changes

fjmolinas reviewed Aug 31, 2020

View reviewed changes

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

fjmolinas reviewed Aug 31, 2020

View reviewed changes

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

jia200x requested a review from cgundogan as a code owner August 31, 2020 13:04

jia200x mentioned this pull request Sep 1, 2020

netdev: use thread_flags #5263

Closed

benpicco reviewed Sep 3, 2020

View reviewed changes

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

benpicco reviewed Sep 3, 2020

View reviewed changes

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

benpicco approved these changes Sep 3, 2020

View reviewed changes

jia200x force-pushed the pr/cc2538_radio_hal branch from 1f1fec0 to 107c607 Compare September 3, 2020 16:18

jia200x added 7 commits September 3, 2020 18:20

ieee802154: add RSSI offset macro

b582d5e

makefiles/pseudomodules: add ieee802154_radio_hal pseudomodule

8b82182

cc2538: move IRQ clear to netdev irq handler

4cbd3a9

cc2538: move chunks of init function to netdev

2917879

cc2538_netdev: disable netdev if radio hal is present

2d7c316

cc2538: add IRQ bit definitions

18d188c

net/ieee802154: add default CSMA-CA params

b7929bf

jia200x force-pushed the pr/cc2538_radio_hal branch from 107c607 to 0fa291d Compare September 3, 2020 16:22

jia200x force-pushed the pr/cc2538_radio_hal branch 2 times, most recently from 6b64fd0 to bb64ceb Compare September 4, 2020 08:37

jia200x added Area: arduino API Area: Arduino wrapper API and removed Area: arduino API Area: Arduino wrapper API labels Sep 4, 2020

jia200x added 2 commits September 4, 2020 11:09

cc2538: implement radio hal

1401635

tests/ieee802154_hal: add test application

387deec

jia200x force-pushed the pr/cc2538_radio_hal branch from bb64ceb to 387deec Compare September 4, 2020 09:09

benpicco added State: waiting for CI update State: The PR requires an Update to CI to be performed first and removed State: waiting for CI update State: The PR requires an Update to CI to be performed first labels Sep 4, 2020

benpicco merged commit 6fcf60c into RIOT-OS:master Sep 4, 2020

jia200x deleted the pr/cc2538_radio_hal branch September 4, 2020 10:07

fjmolinas mentioned this pull request Dec 11, 2020

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cc2538_rf: implement Radio HAL #14791

cc2538_rf: implement Radio HAL #14791

jia200x commented Aug 19, 2020 •

edited

Loading

benpicco commented Aug 20, 2020

benpicco left a comment

benpicco Aug 20, 2020

jia200x Aug 31, 2020

jia200x Sep 1, 2020

benpicco Sep 3, 2020 •

edited

Loading

jia200x commented Aug 31, 2020

jia200x commented Aug 31, 2020

jia200x commented Aug 31, 2020

jia200x commented Sep 3, 2020

benpicco commented Sep 3, 2020 •

edited

Loading

benpicco left a comment

jia200x commented Sep 3, 2020

benpicco commented Sep 3, 2020

jia200x commented Sep 4, 2020

jia200x commented Sep 4, 2020

cc2538_rf: implement Radio HAL #14791

cc2538_rf: implement Radio HAL #14791

Conversation

jia200x commented Aug 19, 2020 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

benpicco commented Aug 20, 2020

benpicco left a comment

Choose a reason for hiding this comment

benpicco Aug 20, 2020

Choose a reason for hiding this comment

jia200x Aug 31, 2020

Choose a reason for hiding this comment

jia200x Sep 1, 2020

Choose a reason for hiding this comment

benpicco Sep 3, 2020 • edited Loading

Choose a reason for hiding this comment

jia200x commented Aug 31, 2020

jia200x commented Aug 31, 2020

jia200x commented Aug 31, 2020

jia200x commented Sep 3, 2020

benpicco commented Sep 3, 2020 • edited Loading

benpicco left a comment

Choose a reason for hiding this comment

jia200x commented Sep 3, 2020

benpicco commented Sep 3, 2020

jia200x commented Sep 4, 2020

jia200x commented Sep 4, 2020

jia200x commented Aug 19, 2020 •

edited

Loading

benpicco Sep 3, 2020 •

edited

Loading

benpicco commented Sep 3, 2020 •

edited

Loading