at86rf2xx: implement basic mode (v2) by jia200x · Pull Request #13798 · RIOT-OS/RIOT

jia200x · 2020-04-03T08:38:59Z

Contribution description

This PR implements basic mode for the AT86RF2XX radios.

In basic mode, the radio doesn't perform any kind of CSMA-CA, retransmissions and Auto ACK.
Although the Address Matching is still available, I'm not adding it because the AT86RF233 doesn't match ACK frames, which might be problematic for users of the basic mode (e.g OpenWSN)

I added CRC error report via a netdev event. The radio will release frame buffer protection when the CRC check fails. Since the extended mode doesn't trigger the TRX_END if CRC fails, this event is only available in basic mode.

Testing procedure

It's possible to test with one radio in extended mode (default configuration) and another radio running in Basic Mode:
CFLAGS=-DAT86RF2XX_BASIC_MODE make flash term

You should be able to see the retransmissions + received ACKs.

Check that the NETDEV_EVENT_CRC_ERROR is called (e.g with an failed assertion).

Issues/PRs references

#8213

drivers/at86rf2xx/at86rf2xx_getset.c

drivers/include/at86rf2xx.h

fjmolinas · 2020-04-06T11:51:34Z

Does it make sense generalizing this basic mode? AFAIK this is a term used by atmel radios, but it basically implies auto ACK, CSMA-CA and retransmission's, which are configurations most radios will understand. CFLAGS=-DAT86RF2XX_BASIC_MODE could be CFLAGS=-DIEE802154_BASIC_MODE or something else?

fjmolinas · 2020-04-06T11:58:11Z

Does it make sense generalizing this basic mode? AFAIK this is a term used by atmel radios, but it basically implies auto ACK, CSMA-CA and retransmission's, which are configurations most radios will understand. CFLAGS=-DAT86RF2XX_BASIC_MODE could be CFLAGS=-DIEE802154_BASIC_MODE or something else?

I know this should somehow come more easily after the netdev rework. But I think that in the current state maybe we can make explicit which radios provide this. It could possibly be a FEATURE provided by the driver, which currently only some radios might support, but with the netdev rework it would be easily generalized. But for now you could have FEATURES_PROVIDED += iee802154_basic_mode, and require it when needed. (I know this is a little out of scope)

jia200x · 2020-04-06T13:16:07Z

Does it make sense generalizing this basic mode? AFAIK this is a term used by atmel radios, but it basically implies auto ACK, CSMA-CA and retransmission's, which are configurations most radios will understand. CFLAGS=-DAT86RF2XX_BASIC_MODE could be CFLAGS=-DIEE802154_BASIC_MODE or something else?

The basic mode is specific to the AT86RF2XX.
What would be indeed possible is to define stuff like:

IEEE802154_AUTO_ACK
IEEE802154_FRAME_RETRANS
etc

So we could configure a "basic mode" setting the right parameters

jia200x · 2020-04-06T13:25:42Z

What would be indeed possible is to define stuff like:

IEEE802154_AUTO_ACK
IEEE802154_FRAME_RETRANS
etc

I typed without thinking, but actually this is indeed radio specific (unless we disable ACK or frame retransmissions for the (Sub)MAC layer, which makes no sense unless you want to simulate test scenarios).

The netdev rework is including the concept of radio caps (#11473), and these can be exposed from the device driver depending on the compile time configurations. For instance

switch(cap) {
#if AT86RF2XX_AUTO_ACK
    return RADIO_HAS_AUTO_ACK;
#endif 
}

}

Then an upper layer has uniform way to extract the caps.

jia200x · 2020-04-06T13:26:31Z

(in other words, setting IEEE802154_FRAME_RETRANS to 0 won't have any effect in CC radios, NRF52, etc)

jia200x · 2020-04-06T13:28:57Z

Or alternatively, we can model these caps for now using compile time features (e.g FEATURES_PROVIDED=IEEE802154_FRAME_RETRANSMISSION). But then we would need to manually take care of the cases where we want to use the radio without the feature (e.g OpenWSN)

drivers/at86rf2xx/at86rf2xx_netdev.c

fjmolinas · 2020-04-06T14:39:49Z

IEEE802154_AUTO_ACK
IEEE802154_FRAME_RETRANS
etc

I think that could make sense, you could have drivers providing those features, and then drivers requiring those features in Makefile.dep (when the state machine doesn't work without AACK). That way, you could blacklist features as well as not requiring them, but I guess this is maybe getting off topic, as it also addresses part of the discussion in #12910. For now lets keep the configuration flag!

jia200x · 2020-04-06T17:26:20Z

I think I addresses all comments

fjmolinas · 2020-04-07T08:56:05Z

I tested pinging between nodes, it sill works and from ifconfig I can see the right configuration is set.

pinging between enhanced nodes, (interestingly I had to rebase or this wouldn't work can you reproduce @jia200x )

Details

> ifconfig
ifconfig
Iface  7  HWaddr: 68:B5  Channel: 26  Page: 0  NID: 0x23
          Long HWaddr: 32:8F:F8:65:10:6B:11:14 
           TX-Power: 0dBm  State: IDLE  max. Retrans.: 3  CSMA Retries: 4 
          AUTOACK  ACK_REQ  CSMA  L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::308f:f865:106b:1114  scope: link  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff6b:1114
          
          Statistics for Layer 2
            RX packets 112  bytes 5028
            TX packets 39 (Multicast: 27)  bytes 1593
            TX succeeded 33 errors 0
          Statistics for IPv6
            RX packets 106  bytes 6664
            TX packets 39 (Multicast: 27)  bytes 2352
            TX succeeded 39 errors 0

Iface  7  HWaddr: 54:86  Channel: 26  Page: 0  NID: 0x23
          Long HWaddr: 02:BC:F6:65:10:6B:11:14 
           TX-Power: 0dBm  State: IDLE  max. Retrans.: 3  CSMA Retries: 4 
          AUTOACK  ACK_REQ  CSMA  L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::bc:f665:106b:1114  scope: link  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff6b:1114
          
          Statistics for Layer 2
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 86
            TX succeeded 2 errors 0
          Statistics for IPv6
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 128
            TX succeeded 2 errors 0

> ping6 fe80::308f:f865:106b:1114
ping6 fe80::308f:f865:106b:1114
12 bytes from fe80::308f:f865:106b:1114%7: icmp_seq=0 ttl=64 rssi=-58 dBm time=14.485 ms
12 bytes from fe80::308f:f865:106b:1114%7: icmp_seq=1 ttl=64 rssi=-58 dBm time=13.213 ms
12 bytes from fe80::308f:f865:106b:1114%7: icmp_seq=2 ttl=64 rssi=-58 dBm time=12.893 ms

--- fe80::308f:f865:106b:1114 PING statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 12.893/13.530/14.485 ms

pinging between basic and enhanced nodes, as there are no AACK, there are many duplicates!

Details

> ifconfig
ifconfig
Iface  7  HWaddr: 0A:92  Channel: 26  Page: 0  NID: 0x23
          Long HWaddr: 52:A8:FA:65:10:6B:11:14 
           TX-Power: 0dBm  State: IDLE  max. Retrans.: 3  CSMA Retries: 4 
          AUTOACK  ACK_REQ  CSMA  L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::50a8:fa65:106b:1114  scope: link  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff6b:1114
          
          Statistics for Layer 2
            RX packets 14  bytes 560
            TX packets 18 (Multicast: 6)  bytes 1182
            TX succeeded 18 errors 0
          Statistics for IPv6
            RX packets 14  bytes 824
            TX packets 12 (Multicast: 6)  bytes 1224
            TX succeeded 12 errors 0

> ping6 fe80::bc:f665:106b:1114 
ping6 fe80::bc:f665:106b:1114 
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=0 ttl=64 rssi=-59 dBm time=5.401 ms
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=0 ttl=64 rssi=-58 dBm time=8.142 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=0 ttl=64 rssi=-59 dBm time=11.203 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=0 ttl=64 rssi=-58 dBm time=14.904 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=1 ttl=64 rssi=-59 dBm time=5.716 ms
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=1 ttl=64 rssi=-59 dBm time=9.097 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=1 ttl=64 rssi=-59 dBm time=13.438 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=1 ttl=64 rssi=-59 dBm time=17.139 ms (DUP!)
12 bytes from fe80::bc:f665:106b:1114%7: icmp_seq=2 ttl=64 rssi=-58 dBm time=5.715 ms

Tested on pkg/openwsn: re-integrate the network stack as a package #13824 rebased on this PR, still works for me.

Details

> [IEE20154E]: synchronized
[neighbors]: new neighbor rssi: -67
[cjoin]: send join request
[cjoin]: success
> [icmpv6rpl]: found better parent
ifconfig
ifconfig
inet6 bbbb::684:f665:106b:1114
hwaddr short: 11:14    long: 06:84:F6:65:10:6B:11:14
panid: CA:FE

IEEE802154E sync: 1

6tsch joined: 1

RPL rank: 2816
RPL parent: 2A:BA:F7:65:10:6B:11:14
RPL DODAG ID: bbbb::2aba:f765:106b:1114

OpenVisualizer  (type "help" for commands)
> root /dev/ttyUSB1
> 10:50:48 ERROR 1114 [IEEE802154E] wrong state 1 in startSlot, at slotOffset 1

received RPL DAO from bbbb:0:0:0:684:f665:106b:1114
- parents:
   bbbb:0:0:0:2aba:f765:106b:1114
- children:
   bbbb:0:0:0:ab8:fc65:106b:1114

PING bbbb::684:f665:106b:1114(bbbb::684:f665:106b:1114) 56 data bytes
64 bytes from bbbb::684:f665:106b:1114: icmp_seq=1 ttl=64 time=3503 ms
64 bytes from bbbb::684:f665:106b:1114: icmp_seq=2 ttl=64 time=2477 ms
64 bytes from bbbb::684:f665:106b:1114: icmp_seq=3 ttl=64 time=3483 ms
64 bytes from bbbb::684:f665:106b:1114: icmp_seq=4 ttl=64 time=2595 ms

fjmolinas · 2020-04-07T08:57:08Z

This doesn't change the base case operation case, and basic mode now works. Please squash @jia200x (and rebase as well, I want to re-check enhanced mode oepration).

jia200x · 2020-04-08T08:28:52Z

pinging between enhanced nodes, (interestingly I had to rebase or this wouldn't work can you reproduce @jia200x )

Strange. Which was your base commit there?

jia200x · 2020-04-08T08:29:04Z

squashed and rebased!

fjmolinas · 2020-04-08T08:55:05Z

@jia200x Travis is complaining about missing documentation about those new macros. You can squash right away once addressed!

jia200x · 2020-04-08T12:08:37Z

done!

fjmolinas

ACK!

fjmolinas · 2020-04-09T07:21:11Z

GO!

PeterKietzmann · 2020-04-09T07:37:12Z

Congratulations!

jia200x requested review from PeterKietzmann, benpicco and fjmolinas April 3, 2020 08:38

jia200x requested review from daniel-k, miri64 and smlng as code owners April 3, 2020 08:39

jia200x added the Area: drivers Area: Device drivers label Apr 3, 2020

jia200x force-pushed the pr/at86rf2x_basic_mode branch 2 times, most recently from 70b9232 to 4aec29d Compare April 3, 2020 08:47

fjmolinas reviewed Apr 6, 2020

View reviewed changes

drivers/at86rf2xx/at86rf2xx_getset.c Show resolved Hide resolved

fjmolinas reviewed Apr 6, 2020

View reviewed changes

drivers/include/at86rf2xx.h Outdated Show resolved Hide resolved

fjmolinas reviewed Apr 6, 2020

View reviewed changes

drivers/at86rf2xx/at86rf2xx_netdev.c Show resolved Hide resolved

fjmolinas pushed a commit to fjmolinas/RIOT that referenced this pull request Apr 6, 2020

MERGECOMMIT: RIOT-OS#13798

17a55b6

fjmolinas mentioned this pull request Apr 6, 2020

pkg/openwsn: re-integrate the network stack as a package #13824

Merged

11 tasks

fjmolinas self-assigned this Apr 7, 2020

jia200x force-pushed the pr/at86rf2x_basic_mode branch from cb3fa4a to 0168b2b Compare April 8, 2020 08:27

jia200x force-pushed the pr/at86rf2x_basic_mode branch from 0168b2b to 69dbf17 Compare April 8, 2020 12:07

fjmolinas approved these changes Apr 8, 2020

View reviewed changes

jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 8, 2020

jia200x added 3 commits April 8, 2020 19:14

at86rf2xx: add basic mode PHY state

69eb3c7

at86rf2xx: implement basic mode

d4b0aff

at86rf2xx: report CRC error in basic mode

ab5418e

jia200x force-pushed the pr/at86rf2x_basic_mode branch from 69dbf17 to ab5418e Compare April 8, 2020 17:14

fjmolinas merged commit 3177784 into RIOT-OS:master Apr 9, 2020

PeterKietzmann mentioned this pull request Apr 9, 2020

at86rf2xx: Basic mode and NETOPT_AUTOACK #8213

Closed

leandrolanzieri added this to the Release 2020.04 milestone Apr 9, 2020

leandrolanzieri added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Apr 9, 2020

jia200x mentioned this pull request Jun 7, 2021

[RFC] at86rf2xx: implement radio HAL #16535

Draft

4 tasks

Conversation

jia200x commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented Apr 6, 2020

Uh oh!

fjmolinas commented Apr 6, 2020

Uh oh!

jia200x commented Apr 6, 2020

Uh oh!

jia200x commented Apr 6, 2020

Uh oh!

jia200x commented Apr 6, 2020

Uh oh!

jia200x commented Apr 6, 2020

Uh oh!

Uh oh!

fjmolinas commented Apr 6, 2020

Uh oh!

jia200x commented Apr 6, 2020

Uh oh!

fjmolinas commented Apr 7, 2020

Uh oh!

fjmolinas commented Apr 7, 2020

Uh oh!

jia200x commented Apr 8, 2020

Uh oh!

jia200x commented Apr 8, 2020

Uh oh!

fjmolinas commented Apr 8, 2020

Uh oh!

jia200x commented Apr 8, 2020

Uh oh!

fjmolinas left a comment

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Apr 9, 2020

Uh oh!

PeterKietzmann commented Apr 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jia200x commented Apr 3, 2020 •

edited

Loading