Split RPL into core and mode related functions. #1167

fabratu · 2014-05-14T10:20:17Z

With this PR rpl.c only contains the core RPL-functions, which include basic initialization, mutex-management, basic receiving of RPL-messages and calling mode specific functions. Each mode is now moved to its own .c/.h pair. In addition rpl.h finally got documentation.

Note: This PR does not include the by now not acknowledged PRs #1080 and #930. Therefore some code is marked as obsolete and will be altered on merge/rebase of the other PRs.

fabratu · 2014-05-17T12:26:44Z

I guess this PR will have to wait until the BBQ? (no prob, just asking)

OlegHahm · 2014-05-17T18:09:52Z

Yes, you're right.

mehlis · 2014-05-21T07:45:01Z

sys/net/routing/rpl/rpl_sm.c

@@ -0,0 +1,863 @@
+/**
+ * RPL dodag implementation


RPL Storing mode impl?

fabratu · 2014-05-31T13:18:26Z

Fixed.

mehlis · 2014-06-12T07:58:19Z

@FabianBrandt please rebase and squash together

fabratu · 2014-06-12T22:55:18Z

Remind myself - never rebase on friday 13. Took some time - seems to be done.

Kijewski · 2014-06-12T23:30:24Z

Well, never rebase at 1 o'clock in the morning … ;)

mehlis · 2014-06-14T13:12:46Z

@FabianBrandt needs rebase again...

fabratu · 2014-06-17T10:38:15Z

Successfully rebased and updated refs/heads/rpl_base_split.

OlegHahm · 2014-06-17T21:24:08Z

Well, hm, seems to have been a very short-term success...

Kijewski · 2014-06-17T21:34:18Z

@FabianBrandt, you forgot to push the rebased branch, didn't you? :)

fabratu · 2014-06-18T10:07:53Z

Nope, pushed it ;) Maybe its about me and git. I will have a look.

Edit: found an error...

Kijewski · 2014-06-18T10:10:10Z

After your comment

Successfully rebased and updated refs/heads/rpl_base_split.

there is no new commit.

fabratu · 2014-06-18T10:56:19Z

There it is. Don´t really know what went wrong exactly. However I had to resolve a conflict, which is still stated in the commit but is not present anymore. From what I understand from git, this means, that there was a merge, which involved more than automatic resolving - which is not true actually.

OlegHahm · 2014-06-18T14:14:39Z

Maybe rename rpl_sm.c to rpl_storing.c or rpl_mop2.c? I think the current name makes it hard to guess, what's in there.

OlegHahm · 2014-06-18T14:17:00Z

sys/net/routing/rpl/rpl.c

@@ -31,155 +31,35 @@
 #include "sixlowpan.h"
 #include "net_help.h"

+/* You can only run Storing Mode by now. Other unsupported modes lead to default (Storing Mode) */
+#if RPL_DEFAULT_MOP == STORING_MODE_NO_MC


STORING_MODE_NO_MC is not defined, right?

I would propose to call it something like RPL_MOP_2 or RPL_STORING_MODE_NO_MC.

OlegHahm · 2014-06-18T14:18:49Z

It's partly overlapping with #891, right? Could you review (and ack this) first?

OlegHahm · 2014-06-18T14:20:09Z

sys/net/routing/rpl/rpl.h

@@ -10,12 +10,17 @@
 * @defgroup    net_rpl RPL
 * @ingroup     net
 * @brief       Routing Protocol for Low power and Lossy Networks
+ *
+ * Header which includes all core RPL-functions. This should not be modified if new modes or
+ * objective functions are implemented.


Maybe rephrase so something saying that it shouldn't be necessary to modify something in this file. Now, it sounds like: "Don't touch this file!".

OlegHahm · 2014-06-18T14:54:36Z

sys/net/routing/rpl/rpl_sm.h

+/**
+ * @defgroup    net_rpl RPL
+ * @ingroup     net
+ * @brief       Routing Protocol for Low power and Lossy Networks


Copy and paste from rpl.h?

OlegHahm · 2014-06-18T14:58:17Z

As far as I can see all function declarations in rpl_sm.h would be the same for all other modes (apart from documentation). Maybe move these declarations to something like rpl_mode.h?

fabratu · 2014-06-18T16:25:09Z

That are really some comments ... I try to look into all of it until tomorrow.

fabratu · 2014-06-18T18:57:42Z

As far as I can see all function declarations in rpl_sm.h would be the same for all other modes (apart from documentation)

@OlegHahm Depends - P2P should have other + there maybe some alterations in future. I would agree, that a certain amount of functions are common for storing/non-storing. Maybe thats a idea to split them into another file and future additions can than lead to a special rpl_[mode-name].h file.

fabratu · 2014-06-18T19:25:33Z

Based on the comments I´ve done the following:

renaming of rpl_sm.* to rpl_storing.*
several spelling and date fixes
documentation fixes
added [in] and [out] for doxygen
global variables from rpl.c got rpl_ as prefix and their declaration moved to rpl.h
added functionality from net: some debug code #891 from rpl.c and rpl.h to their new homes in rpl.c, rpl.h and rpl_storing.c for easy merging after net: some debug code #891 gets acked.

What is not done:

Functions from rpl_storing.h former rpl_sm.h are not moved into a new common mode-headerfile because the initialization in rpl.c relies on different file names for different modes. Therefore it at least needs a rpl_storing.h-headerfile. However when moving all general functions this file would be empty and only lead to confusion. After implementation of non-storing mode is finished, I will look again into this.

OlegHahm · 2014-06-21T16:09:21Z

sys/net/routing/rpl/rpl.c


+    mutex_lock(&rpl_send_mutex);
+    send_DIO_mode(destination);
+    mutex_unlock(&rpl_send_mutex);


Could elaborate/document why these mutexes are necessary?

mehlis · 2014-06-24T07:59:14Z

@FabianBrandt PR needs rebase

fabratu · 2014-06-25T10:06:04Z

Fixed - suggestions inserted.

mehlis · 2014-06-25T13:41:31Z

@OlegHahm can you elaborate , I think @FabianBrandt next steps are based on this PR...

OlegHahm · 2014-06-25T17:29:17Z

Elaborate on what?

I would say ACK.

fabratu · 2014-06-27T18:08:37Z

Don´t ACK. I just recognized a problem with the buffers. I´m making a new commit this weekend.

fabratu · 2014-07-02T10:16:51Z

Buffer issue fixed. If ACK, I will rebase/squash.

OlegHahm · 2014-07-02T13:23:16Z

Can you rebase first?

fabratu · 2014-07-02T18:31:05Z

Sure. Sorry wrong button -.-

OlegHahm · 2014-07-02T20:40:50Z

sys/net/routing/rpl/rpl_storing.c

+#include "sixlowpan.h"
+#include "net_help.h"
+
+#define ENABLE_DEBUG    (1)


Debug should be disabled by default.

OlegHahm · 2014-07-02T20:42:13Z

Apart from the ENABLE_DEBUG setting: ACK - ready to be merged.

fabratu · 2014-07-03T18:25:34Z

Of course this should be zero. Also packet_length is no more.

OlegHahm · 2014-07-04T09:20:44Z

Then let's go!

Split RPL into core and mode related functions.

LudwigKnuepfer added enhancement labels May 14, 2014

OlegHahm added this to the Release NEXT MAJOR milestone May 16, 2014

OlegHahm self-assigned this May 16, 2014

mehlis reviewed May 21, 2014
View reviewed changes

sys/net/routing/rpl/rpl_sm.c

@@ -0,0 +1,863 @@

/**

* RPL dodag implementation

Copy link

Contributor

mehlis May 21, 2014

Choose a reason for hiding this comment

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

RPL Storing mode impl?

OlegHahm added the major label Jun 3, 2014

OlegHahm reviewed Jun 18, 2014
View reviewed changes

OlegHahm reviewed Jun 21, 2014
View reviewed changes

OlegHahm assigned emmanuelsearch and unassigned OlegHahm Jun 25, 2014

fabratu closed this Jul 2, 2014

fabratu reopened this Jul 2, 2014

OlegHahm reviewed Jul 2, 2014
View reviewed changes

fabratu mentioned this pull request Jul 3, 2014

RPL: superfluous packet_len variable #1361

Closed

Split RPL into core and mode related functions.

7fb462c

OlegHahm added a commit that referenced this pull request Jul 4, 2014

Merge pull request #1167 from fabianbrandt/rpl_base_split

53f7bea

Split RPL into core and mode related functions.

OlegHahm merged commit 53f7bea into RIOT-OS:master Jul 4, 2014

Split RPL into core and mode related functions. #1167

Split RPL into core and mode related functions. #1167

Uh oh!

Conversation

fabratu commented May 14, 2014

Uh oh!

fabratu commented May 17, 2014

Uh oh!

OlegHahm commented May 17, 2014

Uh oh!

mehlis May 21, 2014

Choose a reason for hiding this comment

Uh oh!

fabratu commented May 31, 2014

Uh oh!

mehlis commented Jun 12, 2014

Uh oh!

fabratu commented Jun 12, 2014

Uh oh!

Kijewski commented Jun 12, 2014

Uh oh!

mehlis commented Jun 14, 2014

Uh oh!

fabratu commented Jun 17, 2014

Uh oh!

OlegHahm commented Jun 17, 2014

Uh oh!

Kijewski commented Jun 17, 2014

Uh oh!

fabratu commented Jun 18, 2014

Uh oh!

Kijewski commented Jun 18, 2014

Uh oh!

fabratu commented Jun 18, 2014

Uh oh!

OlegHahm commented Jun 18, 2014

Uh oh!

OlegHahm Jun 18, 2014

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Jun 18, 2014

Uh oh!

OlegHahm Jun 18, 2014

Choose a reason for hiding this comment

Uh oh!

OlegHahm Jun 18, 2014

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Jun 18, 2014

Uh oh!

fabratu commented Jun 18, 2014

Uh oh!

fabratu commented Jun 18, 2014

Uh oh!

fabratu commented Jun 18, 2014

Uh oh!

OlegHahm Jun 21, 2014

Choose a reason for hiding this comment

Uh oh!

mehlis commented Jun 24, 2014

Uh oh!

fabratu commented Jun 25, 2014

Uh oh!

mehlis commented Jun 25, 2014

Uh oh!

OlegHahm commented Jun 25, 2014

Uh oh!

fabratu commented Jun 27, 2014

Uh oh!

fabratu commented Jul 2, 2014

Uh oh!

OlegHahm commented Jul 2, 2014

Uh oh!

fabratu commented Jul 2, 2014

Uh oh!

OlegHahm Jul 2, 2014

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Jul 2, 2014

Uh oh!