drivers: new driver for at86rf2xx radios #2825

haukepetersen · 2015-04-16T18:21:29Z

this PR is the continuation of @thomaseichinger's work started in #2650.

~~Tested (and working) with the iot-lab_M3 (at86rf321). With the iot-lab_M3 it should be possible with this PR at the current state to run the new network stack -> happy testing :-)~~

Update: driver should be stable with iot-lab_M3 and samr21-xpro - also tested successfully against the Xbee S1 and validated packets against Wireshark.

~~Receiving also works with the samr32-xpro (at86rf233), here the sending still has problems -> will fix this next week.~~

I heavily restructured the code again -> the goal was to separate the drivers internals from the 'netdev' adaption to a large extend. This is not completely clean -> especially when receiving data there are still some fixed references into netdev...

Some more known issues:

the RSSI seems to be faulty
~~enabling/disabling of AUTOACK, CSMA, PROMISCUOUS-Mode is missing~~
~~a lot of doxygen documentation is missing~~
~~some doxygen is still wrong (copy/paste)~~
the IEEE802.15.4 frame header creation/parsing is not cleanly separated from the driver, yet
~~probably more...~~

Kijewski · 2015-04-16T19:20:35Z

Makefile.dep

+endif
+ifneq (,$(filter ng_at86rf212b,$(USEMODULE)))
+  USEMODULE += ng_at86rf2xx
+endif


Pattern matching: ifneq (,$(filter ng_at86rf2%,$(USEMODULE)))

biboc · 2015-04-17T07:09:43Z

@haukepetersen, Great job!
I will try it soon.

How do you add each driver "specialities"? By some #ifdef inside the code?

EDIT: I see you added some MODULE_NG_AT86RF233

haukepetersen · 2015-04-17T07:55:26Z

@Kijewski good point, will fix, thx

@bapclenet Actually I am not sure, yet. @thomaseichinger and my initial idea was actually to find a way, so you could run say an at86rf231 and an at86rf212b simultaneously. But when working on it yesterday I found the ifdef solution to be far easier... So any ideas are welcome - until then I would say we go with the iedef variant

biboc · 2015-04-17T09:47:44Z

I see. If we use both transceiver at the same time, we will have two net interfaces,won't we? And then send a message will require to select the interface.
I don't know much about the new network stack, do you have a diagram somewhere to get an overview?

Why do we need PSEUDOMODULES if each driver is considered as a MODULE?

kaspar030 · 2015-04-17T12:28:01Z

For my network driver interface, I usually have a *_setup and a *_init function. *_setup can hav any signature and is totally driver/system dependand. I use it to e.g., setup the SPI pins. It could be used to switch the driver into the right device mode for the corresponding instance.

But for initial merge, I'd say compile-time device selection is fine.

haukepetersen · 2015-04-17T13:02:48Z

minor update, now communication between the iot-lab_M3 and the samr21-xpro using short addresses works.

biboc · 2015-04-17T14:48:15Z

Between two samr21 (AT86RF233), I can transmit packets by short addresses but not with long ones.

haukepetersen · 2015-04-20T08:08:24Z

yapp, I have the same behavior here, will look into it today.

kaspar030 · 2015-04-20T10:44:25Z

drivers/ng_at86rf2xx/ng_at86rf2xx_netdev.c

+        buf[pos++] = (uint8_t)((dev->addr_long) >> 8);
+        buf[pos++] = (uint8_t)((dev->addr_long) & 0xff);
+    }
+    /* unsupported address length */


function doesn't handle broadcast case (hdr->dst_l2addr_len==0 && (hdr->flags & NG_NETIF_HDR_FLAGS_BROADCAST)).

(should also (hdr->flags & NG_NETIF_HDR_FLAGS_MULTICAST) in the same manner)

this is on the TODO list :-)

It's just a

if (hdr->flags == (NG_NETIF_HDR_FLAGS_BROADCAST | NG_NETIF_HDR_FLAGS_MULTICAST))) { /* set FCF */ buf[pos++] = 0xff; buf[pos++] = 0xff; }

;-)

one question: is the broadcast address always the short address?

The 802.15.4 standard only defines the short address (0xffff) as broadcast address, so yes.

The Destination Address field, when present, specifies the address of the intended recipient of the frame. A value of 0xffff in this field shall represent the broadcast short address, which shall be accepted as a valid address by all devices currently listening to the channel.

IETF documents also only speak about 0xffff as broadcast address for IEEE 802.15.4.

I'm not even sure there is any EUI-64 defined for broadcast. Only thing in that direction I know of is, that the least significant bit of the first byte (0x01) signifies unicast/multicast.

(but since IEEE 802.15.4 does not support multicast, this is out of scope ;-))

kaspar030 · 2015-04-20T15:12:11Z

Is this driver supposed to work with the 212b?

haukepetersen · 2015-04-20T16:09:28Z

updated driver, handling of long addresses should work now (tested with 231, 233 and against xbee device).

biboc · 2015-04-20T16:10:28Z

The old driver worked with 212b, so this one should work (this is the aim of the driver).
BUT, you won't be able to set the right emission frequency yet because this is a specificity of the 212b.
Later this driver will work with the 230, 231, 233 and 212b as Hauke mentioned.

haukepetersen · 2015-04-20T16:10:54Z

@kaspar030: handling of broadcast addresses is not implemented, yet. In general the driver is also supposed to work with the at86rf212b, but I have not tested this (as I have no device available). But I think there might be some slight alterations needed for getting it to work

miri64 · 2015-04-20T17:00:01Z

drivers/ng_at86rf2xx/ng_at86rf2xx_netdev.c

+    /* default to compress pan TODO: provide means to specify src PAN*/
+    buf[0] |= NG_IEEE802154_FCF_PAN_COMP;
+    if (hdr->dst_l2addr_len == 2) {
+        /* default to 2 byte addresses for src and dst */


The idea was to take NETCONF_OPT_SRC_LEN for the length of the source address.

jonas-rem · 2015-04-20T21:07:30Z

When executing ifconfig on the samr21-xpro board the program crashed (isr_hard_fault).

The reason was the uninitialized netif. From where should ng_netif_init() be called? Would this be /tests/driver_at86rf2xx/main.c in this case?

When calling ng_netif_init() before ng_netif_add(dev->mac_pid), ifconfig returnes valid prints, otherwise isr_hard_fault.

Will test against linux tomorrow.

biboc · 2015-04-20T21:29:28Z

@Jremmert-phytec-iot, which example did youuse to make the ´samr21-xpro´ go to ´isr_hard_fault()´ ?
The example provided by Hauke in tests/driver_at86rf2xx should work

jonas-rem · 2015-04-20T22:01:28Z

@bapclenet, I used the one hauke provided (tests/driver_at86rf2xx) goes to isr_hard_fault. If the mac layer wants to register its kernel_pid ng_netif_add(dev->mac_pid) (ng_netif.c), the function is executed but nothing will be registered.

int ng_netif_add(kernel_pid_t pid)
{
    for (int i = 0; i < NG_NETIF_NUMOF; i++) {
        if (ifs[i] == pid) {    /* avoid duplicates */
            return 0;
        }   

/* registering fails here because ifs[i] !== KERNEL_PID_UNDEF, although it should be.
 * So nothing will be assigned to ifs[i]. In my case it remains at 14578 or so ;). 
 * When I add a call to the initialization function, the correct MAC kernel_pid (5) 
 * gets assigned. */
        else if (ifs[i] == KERNEL_PID_UNDEF) { 
            ifs[i] = pid;

            for (int j = 0; if_handler[j].add != NULL; j++) {
                if_handler[j].add(pid);
            }

            return 0;
        }
    }

    return -ENOMEM;
}

I have no idea why it worked for you without the initialization. Maybe the uninitialized value of ifs[i] was 0 ( KERNEL_PID_UNDEF)? Maybe I have missed something fundamental?

biboc · 2015-04-21T08:07:10Z

@jremmert-phytec-iot, I've got the same configuration, and pid is 5 for me.
I think you're right, ng_netif_init() should be called. @haukepetersen ?
Btw, why ng_netreg_init is not called as well?

miri64 · 2015-04-21T08:25:33Z

@jremmert-phytec-iot it seems that you have another interface registered. Does it work if you set a higher NG_NETIF_NUMOF?

@bapclenet It should work without ng_netif_init(). The C standard states that all static memory should be set to 0, if no other value is provided. ng_netif_init() was added for testing purposes only (it might even be set into TEST_SUITES #ifdefs)

miri64 · 2015-04-21T08:26:16Z

At the very least: ng_netreg_init() should not be called in this driver, but by auto_init.

jonas-rem · 2015-04-21T10:00:31Z

Ok, it is clear that ng_netref_init() should not be called in the driver since it would delete all other registered interfaces.

As test setup I just checked out the latest master, applied this PR and flashed this test application. So, there should not be any other interfaces. Also ng_netif_add() is called the first time from ng_nomac.

When setting a breakpoint to ng_netif_add and printing the variables on the first (and only) call:

(gdb) print ifs[0]
$3 = 14976
(gdb) print pid
$4 = 5

I investigated a bit more. The memory address of ifs[0] is written to 0 in reset_handler(). Later, kernel_init() prints some stuff on the shell before it starts the idle and main thread. This print changes the content of the memory address of ifs[0]. If I uncomment these printf calls, everything works, without ng_netif_init.

I assume that ifs[0] is initialized after all this, in main thread-context, but is it initialized with 0 then? So, initialize netif once after the startup procedure would solve this problem.

OlegHahm · 2015-04-27T08:47:19Z

Check for make doc 2>&1 | grep '.*warning' | grep ng_at86rf2xx and similar for ng_ieee802154.h.

biboc · 2015-04-27T09:02:46Z

sys/include/net/ng_ieee802154.h

+#ifdef __cplusplus
+}
+#endif
+


Travis complains here
Missed something like
/**
* @brief IEEE 802.14.4 header definitions
* @{
*/
/ * * @} */

ups, must have overlooked that one... fixed.

haukepetersen · 2015-04-27T09:16:54Z

fixed named issues and rebased and squashed. Travis should now be happy.

biboc · 2015-04-27T10:02:04Z

Great!
Here is Travis fail:
The command "sudo apt-get update" failed and exited with 100 during .
@authmillenon

haukepetersen · 2015-04-27T12:58:51Z

Seems Travis is broken once more...

miri64 · 2015-04-27T13:09:45Z

@haukepetersen please rebase. #2873 fixed the issue.

haukepetersen · 2015-04-27T16:56:51Z

rebased.

daniel-k · 2015-04-27T18:02:19Z

drivers/ng_at86rf2xx/ng_at86rf2xx.c

+    }
+}
+
+size_t ng_at86rf2xx_rx_len(ng_at86rf2xx_t *dev)


Page 946 of the datasheet (SAM R21) states, that the MSB should be masked for 802.15.4 compliance. Shouldn't this be done here?

On reception, the PHR is returned as the first octet during Frame Buffer read access. While the IEEE 802.15.4-2006 standard declares bit seven of the PHR octet as being reserved, the AT86RF233 preserves this bit upon transmission and reception so it can be used to carry additional information within proprietary networks. Nevertheless, this bit is not considered to be a part of the frame length, so only frames between one and 127 octets are possible. For IEEE 802.15.4 compliant operation bit [7] has to be masked by software.

EDIT: My comment refers to size_t ng_at86rf2xx_rx_len(ng_at86rf2xx_t *dev) of course. I wasn't aware of how GitHub is displaying line comments.

That paragraph I have not read before, so you are right that masking the bit is a good idea. As long as we have RIOT devices only (with the same driver), it should not be an issue for now, though. So again: Lets fix this after this PR is merged.

biboc · 2015-04-28T06:43:25Z

Travis complained about the label need squashing. Let Travis work and this PR should be fine for merging.

miri64 · 2015-04-28T09:41:25Z

Can't restart. Let's wait until the H&A if something happens, otherwise let's merge it than

haukepetersen · 2015-04-28T09:44:19Z

Ok, if the label was the only thing Travis complained about, I say: Let's merge this now!

miri64 · 2015-04-28T11:12:08Z

Yeah, let's merge it.

drivers: new driver for at86rf2xx radios

biboc · 2015-04-28T11:44:05Z

Great! :-)
Good job @haukepetersen

PeterKietzmann · 2015-05-13T22:28:02Z

drivers/ng_at86rf2xx/ng_at86rf2xx_netdev.c

+            if (max_len < sizeof(int16_t)) {
+                return -EOVERFLOW;
+            }
+            *((uint16_t *)val) = NG_AT86RF2XX_MAX_PKT_LENGTH;


You are returning the full 802.15.4 packet length here without taking care of the 802.15.4 header that also has to be appended.

ping @haukepetersen

@bapclenet I opened issue #3086 for that discussion

haukepetersen added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: drivers Area: Device drivers NSTF labels Apr 16, 2015

haukepetersen assigned biboc Apr 16, 2015

haukepetersen added this to the Network Stack Task Force milestone Apr 16, 2015

haukepetersen mentioned this pull request Apr 16, 2015

drivers/ng_at86rf2xx: initial commit of refactoring #2650

Closed

Kijewski reviewed Apr 16, 2015
View reviewed changes

PeterKietzmann mentioned this pull request Apr 17, 2015

Extended radio api and AT86RF231 implementation #2562

Closed

OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 20, 2015

kaspar030 reviewed Apr 20, 2015
View reviewed changes

miri64 reviewed Apr 20, 2015
View reviewed changes

biboc reviewed Apr 27, 2015
View reviewed changes

haukepetersen force-pushed the opt_driver_rf2xx branch from c9e85c7 to 9390c86 Compare April 27, 2015 09:16

haukepetersen force-pushed the opt_driver_rf2xx branch from 9390c86 to ca75e5b Compare April 27, 2015 09:18

miri64 mentioned this pull request Apr 27, 2015

net: initial import of the ZEP protocol #2627

Merged

haukepetersen force-pushed the opt_driver_rf2xx branch from ca75e5b to 4e121e1 Compare April 27, 2015 16:56

haukepetersen added 3 commits April 27, 2015 18:56

drivers: new driver for at86rf2xx family radios

fefac35

net: added ng header for ieee802154

b15b291

tests: added test app for the at86rf2xx driver

4e121e1

daniel-k reviewed Apr 27, 2015
View reviewed changes

biboc removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 28, 2015

miri64 added a commit that referenced this pull request Apr 28, 2015

Merge pull request #2825 from haukepetersen/opt_driver_rf2xx

b829dfa

drivers: new driver for at86rf2xx radios

miri64 merged commit b829dfa into RIOT-OS:master Apr 28, 2015

haukepetersen deleted the opt_driver_rf2xx branch April 28, 2015 15:44

daniel-k mentioned this pull request Apr 29, 2015

ng_at86rf2xx: fix wrong LQI and ensure 802.15.4 compliance with packet length field #2884

Closed

PeterKietzmann reviewed May 13, 2015
View reviewed changes

miri64 added the Area: network Area: Networking label Sep 30, 2018

drivers: new driver for at86rf2xx radios #2825

drivers: new driver for at86rf2xx radios #2825

Conversation

haukepetersen commented Apr 16, 2015

Choose a reason for hiding this comment

biboc commented Apr 17, 2015

haukepetersen commented Apr 17, 2015

biboc commented Apr 17, 2015

kaspar030 commented Apr 17, 2015

haukepetersen commented Apr 17, 2015

biboc commented Apr 17, 2015

haukepetersen commented Apr 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaspar030 commented Apr 20, 2015

haukepetersen commented Apr 20, 2015

biboc commented Apr 20, 2015

haukepetersen commented Apr 20, 2015

Choose a reason for hiding this comment

jonas-rem commented Apr 20, 2015

biboc commented Apr 20, 2015

jonas-rem commented Apr 20, 2015

biboc commented Apr 21, 2015

miri64 commented Apr 21, 2015

miri64 commented Apr 21, 2015

jonas-rem commented Apr 21, 2015

OlegHahm commented Apr 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haukepetersen commented Apr 27, 2015

biboc commented Apr 27, 2015

haukepetersen commented Apr 27, 2015

miri64 commented Apr 27, 2015

haukepetersen commented Apr 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biboc commented Apr 28, 2015

miri64 commented Apr 28, 2015

haukepetersen commented Apr 28, 2015

miri64 commented Apr 28, 2015

biboc commented Apr 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment