gnrc_lwmac: port to gnrc_netif2 by zhuoshuguo · Pull Request #7895 · RIOT-OS/RIOT

zhuoshuguo · 2017-10-27T16:21:04Z

Port LwMAC protocol to netif2.

zhuoshuguo · 2017-10-27T16:38:15Z

@miri64 Hi, Martine, can I bother you for help correcting the initialization? I get a wrong idea about it.

miri64

Hope this helps

miri64 · 2017-10-27T16:55:32Z

sys/include/net/gnrc/lwmac/lwmac.h

-                             const char *name, gnrc_netdev_t *dev);
+gnrc_netif2_t *gnrc_lwmac_init(char *stack, int stacksize, char priority,
+                                  const char *name, netdev_t *dev,
+                                  const gnrc_netif2_ops_t *ops);


The ops-parameter shouldn't be needed here (just use a static definition in your c-file)

miri64 · 2017-10-27T16:56:11Z

sys/include/net/gnrc/netif2/mac.h

+ * support on-chip CSMA and this flag is set for requiring CSMA transmission,
+ * then, the device will run software CSMA using `csma_sender` APIs.
+ */
+#define GNRC_NETDEV_MAC_INFO_CSMA_ENABLED       (0x0100U)


For all: GNRC_NETIF2 instead of GNRC_NETDEV ;-)

miri64 · 2017-10-27T16:56:46Z

sys/include/net/gnrc/netif2.h

+    netif->mac.mac_info &= ~GNRC_NETDEV_MAC_INFO_TX_FEEDBACK_MASK;
+    netif->mac.mac_info |= (uint16_t)(txf & GNRC_NETDEV_MAC_INFO_TX_FEEDBACK_MASK);
+}
+#endif


These function belong into the net/gnrc/mac/mac.h header.

miri64 · 2017-10-27T16:59:11Z

sys/net/gnrc/link_layer/lwmac/lwmac.c

+    _update_l2addr_from_dev(netif);
+}
+
+////////netif2


This all shouldn't be required.

miri64 · 2017-10-27T17:01:00Z

sys/net/gnrc/link_layer/lwmac/lwmac.c

+    netif->mac.lwmac.system_start_time_ticks = rtt_get_counter();
+    netif->mac.lwmac.last_radio_on_time_ticks = netif->mac.lwmac.system_start_time_ticks;
+    netif->mac.lwmac.awake_duration_sum_ticks = 0;
+    netif->mac.lwmac.lwmac_info |= GNRC_LWMAC_RADIO_IS_ON;


All of the lwmac-related code can go into gnrc_netif2_ops_t::init() so you don't need the thread anymore.

Hi, Martine, what do you mean I don't need the thread anymore? Do you mean, (after I implemented ops-parameters with LwMAC features), there will be no LwMAC thread anymore (and there will be only the thread that created in sys/net/gnrc/netif2/gnrc_netif2.c), ?

So, is the idea like this: LwMAC should be ported to the thread created in sys/net/gnrc/netif2/gnrc_netif2.c (the thread, maybe the real MAC thread then?), with replacement of the ops-parameters (like _init(), _send...) of my own LwMAC implementation?

Yes. That is one of the main features of gnrc_netif2, to simplify MAC porting and to avoid code duplication ;-).

(you've done this this is just not as visible in the diff so everything alright here ;-))

miri64 · 2017-10-27T17:08:56Z

sys/net/gnrc/link_layer/lwmac/lwmac.c

+    		            _lwmac_thread, (void *)netif, name);
+    (void)res;
+    assert(res > 0);
+    return netif;


Just call gnrc_netif2_create() with your ops struct here.

miri64 · 2017-10-27T17:09:30Z

sys/net/gnrc/netif2/gnrc_netif2_ieee802154.c

+    puts("init lwmac");
+    return gnrc_lwmac_init(stack, stacksize, priority, "at86rf2xx-lwmac", dev,
+                              &ieee802154_ops);
+#else


miri64 · 2017-10-27T17:09:43Z

sys/net/gnrc/netif2/gnrc_netif2_ieee802154.c

-        if (netif->mac_info & GNRC_NETDEV_MAC_INFO_CSMA_ENABLED) {
-            res = csma_sender_csma_ca_send(dev, vector, n, &netif->csma_conf);
+        if (netif->mac.mac_info & GNRC_NETDEV_MAC_INFO_CSMA_ENABLED) {
+            res = csma_sender_csma_ca_send(dev, vector, n, &netif->mac.csma_conf);


miri64 · 2017-10-27T17:10:08Z

sys/net/gnrc/netif2/gnrc_netif2.c

 #endif
    if (netif->ops->init) {
        netif->ops->init(netif);
+        puts("inti device");


Sorry, this is just for debug, forget to remove it.. :-P Will remove!

miri64 · 2017-10-27T17:10:21Z

sys/net/gnrc/netif2/gnrc_netif2.c

                DEBUG("gnrc_netif2: GNRC_NETAPI_MSG_TYPE_GET received. opt=%s\n",
                      opt->opt);
 #endif
+                puts("get!!");


I think this is debug output ;-)

zhuoshuguo · 2017-10-27T21:01:48Z

Sorry, Martine, 😅 I know there are some mess in this PR . I just did a quick porting (and some debug runing) to see if my way of porting (and understanding) is corrent. Will clean out all the mess myself of cause. You don't have to focus on those debuging mess now. ;-)

zhuoshuguo · 2017-10-30T14:11:53Z

Hi, Martine, I don't know I understand it corrently, but do I still need to use #ifdef MODULE_GNRC_LWMAC and put my gnrc_lwmac_netif2_create() in here, namely, in sys/auto_init/netif/auto_init_at86rf2xx.c ?? Namely, to replace gnrc_netif2_ieee802154_create() when MODULE_GNRC_LWMAC is defined?

zhuoshuguo · 2017-10-30T21:21:35Z

Adapted, should be closer to the way it is expected to be.

miri64 · 2017-11-01T13:17:33Z

Hi, Martine, I don't know I understand it corrently, but do I still need to use #ifdef MODULE_GNRC_LWMAC and put my gnrc_lwmac_netif2_create() in here, namely, in sys/auto_init/netif/auto_init_at86rf2xx.c ?? Namely, to replace gnrc_netif2_ieee802154_create() when MODULE_GNRC_LWMAC is defined?

Do it with an #ifdef for now. We will do it with a more generic approach later.

zhuoshuguo · 2017-11-01T13:33:52Z

Do it with an #ifdef for now. We will do it with a more generic approach later.

Thanks, @miri64 , have done it, like this.

miri64

Tested successfully with tests/lwmac (I'm not sure examples/gnrc_networking_mac can work at the moment so I did not bother testing it for now), but some (minor) issues still left.

miri64 · 2017-11-01T13:20:23Z

sys/auto_init/netif/auto_init_at86rf2xx.c

+                                 AT86RF2XX_MAC_STACKSIZE,
+                                 AT86RF2XX_MAC_PRIO, "at86rf2xx-lwmac",
+                                 (netdev_t *)&at86rf2xx_devs[i]);
+#else


Move this into the #ifdef MODULE_GNRC_NETIF2 below, please

miri64 · 2017-11-01T13:20:39Z

sys/auto_init/netif/auto_init_at86rf2xx.c

 #else
        int res = gnrc_netdev_ieee802154_init(&gnrc_adpt[i],
                                              (netdev_ieee802154_t *)&at86rf2xx_devs[i]);
-


Please leave this be ;-)

miri64 · 2017-11-01T13:21:18Z

sys/include/net/gnrc/lwmac/lwmac.h

+ * @param[in] priority  Priority for the network interface's thread.
+ * @param[in] name      Name for the network interface. May be NULL.
+ * @param[in] dev       Device for the interface.
+ * @param[in] ops       Operations for the LWMAC network interface.


ops isn't a parameter below.

miri64 · 2017-11-01T13:21:41Z

sys/include/net/gnrc/lwmac/lwmac.h

- * @param[in] name          name of the thread housing the LWMAC instance
- * @param[in] dev           netdev device, needs to be already initialized
+ * @note If @ref DEVELHELP is defined netif_params_t::name is used as the
+ *       name of the network interface's thread.


What is netif_params_t?

miri64 · 2017-11-01T13:26:05Z

sys/net/gnrc/link_layer/lwmac/include/lwmac_internal.h

+ *          (e.g. empty payload with Ethernet).
+ * @return  Any negative error code reported by gnrc_netif2_t::dev.
+ */
+int _gnrc_lwmac_transmit(gnrc_netif2_t *netif, gnrc_pktsnip_t *pkt);


(optional) since the function name starts with a _: Maybe add @internal tag to the doc?

How about I just change this function to gnrc_lwmac_transmit()??

Added @internal tag to the doc.

miri64 · 2017-11-01T13:32:07Z

sys/net/gnrc/link_layer/lwmac/lwmac.c

+    netif->mac.lwmac.system_start_time_ticks = rtt_get_counter();
+    netif->mac.lwmac.last_radio_on_time_ticks = netif->mac.lwmac.system_start_time_ticks;
+    netif->mac.lwmac.awake_duration_sum_ticks = 0;
+    netif->mac.lwmac.lwmac_info |= GNRC_LWMAC_RADIO_IS_ON;


(you've done this this is just not as visible in the diff so everything alright here ;-))

zhuoshuguo · 2017-11-01T14:06:51Z

Addressed new comments~

miri64

Changes look all good now. This might be merged before the actual GNRC integration of gnrc_netif2 ^^: ACK

zhuoshuguo · 2017-11-01T14:15:38Z

OK~ Thanks! Martine!~ Could you wait a minute before trigger the merge button. I want to pope all the LWMAC codes through uncrustify, to tackle potential remaining coding style issues~ Also, I need to squash. ^^

miri64 · 2017-11-01T14:18:00Z

Sure! Squash is necessary anyway ;-)

miri64 · 2017-11-01T14:19:00Z

Also: checkout some whitespace issues, that Murdock found.

zhuoshuguo · 2017-11-01T14:19:56Z

Also: checkout some whitespace issues, that Murdock found.

OK, I suppose uncrustify will do the job. :-)

zhuoshuguo · 2017-11-01T14:38:04Z

Poped through uncrustify, squash immediately.

zhuoshuguo · 2017-11-01T14:43:07Z

Squashed!~ ;-)

zhuoshuguo · 2017-11-01T14:48:20Z

Everything looks OK now, let's go for merge?~ 😃

miri64 · 2017-11-01T15:11:00Z

Yapp

miri64 changed the base branch from master to gnrc_netif2_integration/master October 27, 2017 16:53

miri64 changed the title ~~Lwmac port to netif2~~ gnrc_lwmac: port to gnrc_netif2 Oct 27, 2017

miri64 added Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. GNRC Area: network Area: Networking labels Oct 27, 2017

miri64 requested changes Oct 27, 2017

View reviewed changes

miri64 self-assigned this Oct 28, 2017

miri64 requested changes Nov 1, 2017

View reviewed changes

miri64 approved these changes Nov 1, 2017

View reviewed changes

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 1, 2017

miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 1, 2017

gnrc_lwmac: port to gnrc_netif2.

c554d30

zhuoshuguo force-pushed the lwmac_port_to_netif2 branch from ce74a9b to c554d30 Compare November 1, 2017 14:42

miri64 merged commit ea27fe3 into RIOT-OS:gnrc_netif2_integration/master Nov 1, 2017

miri64 mentioned this pull request Nov 1, 2017

gnrc_netif2: gnrc_ipv6_nib: full integration into GNRC #7925

Merged

28 tasks

zhuoshuguo mentioned this pull request Nov 2, 2017

gnrc_gomach: a traffic adaptive MAC protocol for IEEE 802.15.4 networks (GoMacH). #5618

Merged

2 tasks

miri64 mentioned this pull request Nov 15, 2017

LWMAC: avoid receiving duplicate broadcast packet. #7442

Closed

aabadie added this to the Release 2018.01 milestone Jan 18, 2018

Conversation

zhuoshuguo commented Oct 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhuoshuguo commented Oct 27, 2017

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo Oct 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo commented Oct 27, 2017

Uh oh!

zhuoshuguo commented Oct 30, 2017

Uh oh!

zhuoshuguo commented Oct 30, 2017

Uh oh!

miri64 commented Nov 1, 2017

Uh oh!

zhuoshuguo commented Nov 1, 2017

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo commented Nov 1, 2017

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

zhuoshuguo commented Nov 1, 2017

zhuoshuguo commented Oct 27, 2017 •

edited

Loading

zhuoshuguo Oct 30, 2017 •

edited

Loading