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

drivers/at86rf215: implement MR-OFDM #14010

Merged
merged 4 commits into from Jun 3, 2020

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented May 4, 2020

Contribution description

This adds MR-OFDM modulation to the at86rf215 driver.

This also adds new commands to ifconfig to configure the new modulation parameters.

Testing procedure

Switch to the new PHY mode with ifconfig 7 set phy_mode mr-ofdm. Note that the L2-PDU has changed to 2022 bytes (802.15.4g can send 2047 byte frames).

2020-05-04 03:41:16,173 # Iface  7  HWaddr: 01:57  Channel: 0  NID: 0x23  PHY: MR-OFDM 
2020-05-04 03:41:16,180 #            Option: 1  MCS: 0 (BPSK, rate 1/2, 4x frequency repetition) 
2020-05-04 03:41:16,184 #           Long HWaddr: 1A:F9:8F:5F:30:5D:05:8E 
2020-05-04 03:41:16,191 #            TX-Power: 0dBm  State: IDLE  max. Retrans.: 3  CSMA Retries: 4 
2020-05-04 03:41:16,197 #           AUTOACK  ACK_REQ  CSMA  L2-PDU:2022 MTU:1280  HL:64  RTR  
2020-05-04 03:41:16,199 #           6LO  IPHC  
2020-05-04 03:41:16,202 #           Source address length: 8
2020-05-04 03:41:16,204 #           Link type: wireless
2020-05-04 03:41:16,210 #           inet6 addr: fe80::18f9:8f5f:305d:58e  scope: link  VAL
2020-05-04 03:41:16,213 #           inet6 group: ff02::2
2020-05-04 03:41:16,216 #           inet6 group: ff02::1
2020-05-04 03:41:16,219 #           inet6 group: ff02::1:ff5d:58e
2020-05-04 03:41:16,220 #           
2020-05-04 03:41:16,223 #           Statistics for Layer 2
2020-05-04 03:41:16,226 #             RX packets 0  bytes 0
2020-05-04 03:41:16,230 #             TX packets 4 (Multicast: 4)  bytes 172
2020-05-04 03:41:16,233 #             TX succeeded 4 errors 0
2020-05-04 03:41:16,236 #           Statistics for IPv6
2020-05-04 03:41:16,239 #             RX packets 0  bytes 0
2020-05-04 03:41:16,244 #             TX packets 4 (Multicast: 4)  bytes 256
2020-05-04 03:41:16,247 #             TX succeeded 4 errors 0
2020-05-04 03:41:16,247 # 
2020-05-04 03:41:30,034 #  ping6 fe80::2068:3123:59f5:d23a%7
2020-05-04 03:41:30,053 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=0 ttl=64 rssi=-41 dBm time=12.051 ms
2020-05-04 03:41:31,054 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=1 ttl=64 rssi=-40 dBm time=12.051 ms
2020-05-04 03:41:32,054 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=2 ttl=64 rssi=-41 dBm time=12.051 ms

You should be able to configure the OFDM option (ifconfig 7 set option [1…4]) as well as the Modulation and Coding Scheme (ifconfig 7 set mcs [0…6]):

2020-05-04 03:49:00,938 #  ifconfig 7 set mcs 2
2020-05-04 03:49:04,010 #  ifconfig 7 set option 4
2020-05-04 03:52:49,320 #  ping6 fe80::2068:3123:59f5:d23a%7
2020-05-04 03:52:49,348 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=0 ttl=64 rssi=-42 dBm time=19.828 ms
2020-05-04 03:52:50,348 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=1 ttl=64 rssi=-42 dBm time=19.828 ms
2020-05-04 03:52:51,348 # 12 bytes from fe80::2068:3123:59f5:d23a%7: icmp_seq=2 ttl=64 rssi=-42 dBm time=19.827 ms
2020-05-04 03:52:51,348 # 
2020-05-04 03:52:51,353 # --- fe80::2068:3123:59f5:d23a PING statistics ---
2020-05-04 03:52:51,358 # 3 packets transmitted, 3 packets received, 0% packet loss
2020-05-04 03:52:51,362 # round-trip min/avg/max = 19.827/19.827/19.828 ms

Nodes can talk to each other if they use the same option, but different Modulation & Coding Schemes.

Issues/PRs references

split off #12128
requires #13979 to send full 2k frames

@benpicco benpicco added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels May 4, 2020
@benpicco benpicco requested a review from fjmolinas May 4, 2020 01:38
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 4, 2020
@benpicco benpicco added this to Backlog / Proposals in RIOT Sprint Day via automation May 5, 2020
@benpicco
Copy link
Contributor Author

@tinstructor I heard you like MR-OFDM, want to give this a try? 😉

@tinstructor
Copy link

@benpicco I'll run through your testing procedure quickly tomorrow morning. Mind you, due to Corona lockdown measures I currently don't have access to a signal analyzer should things need troubleshooting.

@tinstructor
Copy link

@benpicco Remind me again, which example application do you typically use to test drivers like this?

@benpicco
Copy link
Contributor Author

examples/gnrc_networking should work

@tinstructor
Copy link

@benpicco I've quickly done some testing with two OpenMote-B nodes and it seems to work just fine. Of course you'd probably want to run a few hundred pings for each interface / each combination of option + MCS to be sure

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.

Some nitpicks, otherwise code looks. Will test against different configurations.

sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
sys/shell/commands/sc_gnrc_netif.c Outdated Show resolved Hide resolved
drivers/include/at86rf215.h Outdated Show resolved Hide resolved
drivers/at86rf215/at86rf215_ofdm.c Show resolved Hide resolved
@fjmolinas
Copy link
Contributor

@benpicco I've quickly done some testing with two OpenMote-B nodes and it seems to work just fine. Of course you'd probably want to run a few hundred pings for each interface / each combination of option + MCS to be sure

Your application that looped over the different configurations would be very usefull here :)

@benpicco
Copy link
Contributor Author

benpicco commented Jun 2, 2020

Your application that looped over the different configurations would be very usefull here :)

That one?
Probably needs an update to match the upstream API now.

@fjmolinas
Copy link
Contributor

Your application that looped over the different configurations would be very usefull here :)

That one?
Probably needs an update to match the upstream API now.

Did some testing mixing up configurations and it all seems to work. trying to see if I can cover them all automaticaly.

@fjmolinas
Copy link
Contributor

fjmolinas commented Jun 2, 2020

Your application that looped over the different configurations would be very usefull here :)

That one?
Probably needs an update to match the upstream API now.

Did some testing mixing up configurations and it all seems to work. trying to see if I can cover them all automatically.

I used #13612 to script testing over all possible combinations. Worked for all pining both ways with -c 100 -s 100 -i 100. For the options/mcs allowing higher datarates I was able to ping at much higher rates.

@fjmolinas fjmolinas added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jun 2, 2020
@fjmolinas
Copy link
Contributor

@benpicco would you mind uncrustifying the files?

@fjmolinas fjmolinas added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines labels Jun 2, 2020
@benpicco
Copy link
Contributor Author

benpicco commented Jun 2, 2020

I removed some of the crust, although IMHO that added to the taste.
Or do you mean also uncrustify the existing code?

@fjmolinas
Copy link
Contributor

I removed some of the crust, although IMHO that added to the taste.
Or do you mean also uncrustify the existing code?

The new code, I noticed that uncrustify -c uncrustify-riot.cfg drivers/at86rf215/at86rf215_ofdm.c suggested many changes :)

@fjmolinas fjmolinas moved this from Backlog / Proposals to Under Review in RIOT Sprint Day Jun 3, 2020
@fjmolinas fjmolinas self-assigned this Jun 3, 2020
@fjmolinas
Copy link
Contributor

Please squash @benpicco (BTW I don't get nitified when you push a fix, so please ping me when you do so I notice :) )

@fjmolinas fjmolinas added the Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines label Jun 3, 2020
_TXDFE_SR(option) | _TXDFE_RCUT(option));

/* set channel spacing */
at86rf215_reg_write(dev, dev->RF->RG_CS, _channel_spacing_kHz(option) / 25);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed this 25, why is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah 25Khz resolution

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment like for o-qpsk?

/* set channel spacing with 25 kHz resolution */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! And I also undid some of the damage done by uncrustify in the next line.

Define options for IEEE 802.15.4g MR-OFDM as well as shell commands
to set them via ifconfig.
With the added features, the driver doesn't fit onto those
ATmega boards anymore.
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

RIOT Sprint Day automation moved this from Under Review to Waiting For Murdock Jun 3, 2020
@benpicco benpicco merged commit 3a1ee49 into RIOT-OS:master Jun 3, 2020
RIOT Sprint Day automation moved this from Waiting For Murdock to Done Jun 3, 2020
@benpicco benpicco deleted the at86rf215-mr-ofdm branch June 3, 2020 14:39
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants