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.

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

at86rf2xx: implement basic mode (v2) #13798

Merged
merged 3 commits into from
Apr 9, 2020

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Apr 3, 2020

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

@jia200x jia200x added the Area: drivers Area: Device drivers label Apr 3, 2020
@jia200x jia200x force-pushed the pr/at86rf2x_basic_mode branch 2 times, most recently from 70b9232 to 4aec29d Compare April 3, 2020 08:47
@fjmolinas
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

jia200x commented Apr 6, 2020

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
Copy link
Member Author

jia200x commented Apr 6, 2020

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
Copy link
Member Author

jia200x commented Apr 6, 2020

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

@jia200x
Copy link
Member Author

jia200x commented Apr 6, 2020

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)

@fjmolinas
Copy link
Contributor

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!

fjmolinas pushed a commit to fjmolinas/RIOT that referenced this pull request Apr 6, 2020
@jia200x
Copy link
Member Author

jia200x commented Apr 6, 2020

I think I addresses all comments

@fjmolinas
Copy link
Contributor

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 )
> 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!
> 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
> [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
Copy link
Contributor

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
Copy link
Member Author

jia200x commented Apr 8, 2020

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
Copy link
Member Author

jia200x commented Apr 8, 2020

squashed and rebased!

@fjmolinas
Copy link
Contributor

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

@jia200x
Copy link
Member Author

jia200x commented Apr 8, 2020

done!

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK!

@jia200x 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
@fjmolinas
Copy link
Contributor

GO!

@fjmolinas fjmolinas merged commit 3177784 into RIOT-OS:master Apr 9, 2020
@PeterKietzmann
Copy link
Member

Congratulations!

@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Apr 9, 2020
@leandrolanzieri leandrolanzieri added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants