LR-WPAN ContikiMAC

LR-WPAN ContikiMAC

LR-WPAN MAC base class
- lr-wpan-mac.h

802.15.4 MAC class, extending the LrWpanMac base class
- lr-wpan-nullmac.h
- lr-wpan-nullmac.cc

ContikiMAC model, extending the LrWpanMac base class
- lr-wpan-contikimac.h
- lr-wpan-contikimac.cc

- lr-wpan-net-device.cc
Configured to use nullmac, can set ContikiMAC using existing SetMac function

- lr-wpan-contikimac-test.cc
3 node scenario:
Node 1&2 sleep time = 150 ms
Node 3 sleep time = 125 ms

First node 1 sends to node 2, Node 3 wakes up first and goes  back to sleep after detecting that packet is for another node. Node 2 wakes up and receives packet.

Then node 1 sends to node 3 and fails as sleep time node 3 > retransmissions done by node 1

Node 3 broadcasts packet- both node 1 and 2 wake up and receive as per their wake-up schedule

Node 3 sends packet (without ack bit set) and goes back to sleep immediately

[Tommaso Pecorella, 2015-07-22 15:46:38]

First bunch of comments. The MAC files review is not yet finished.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
File src/lr-wpan/examples/lr-wpan-contikimac-test.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
src/lr-wpan/examples/lr-wpan-contikimac-test.cc:18: * Author:  Tom Henderson
<thomas.r.henderson@boeing.com>
I don't think you should add Tom, even if you "borrowed" most of his code.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
File src/lr-wpan/examples/lr-wpan-data.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
src/lr-wpan/examples/lr-wpan-data.cc:48: std::cout << "Received packet of size "
<< p->GetSize () << "\n";
Remember to revert to NS_LOG_UNCOND before the final release.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
File src/lr-wpan/model/lr-wpan-contikimac.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
src/lr-wpan/model/lr-wpan-contikimac.cc:24: *  Vishwesh Rege
<vrege2012@gmail.com>
Only the right authors (you)

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
File src/lr-wpan/model/lr-wpan-contikimac.h (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
src/lr-wpan/model/lr-wpan-contikimac.h:24: */
see the comment on the .cc - just one author in this case

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac.h
File src/lr-wpan/model/lr-wpan-mac.h (left):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac...
src/lr-wpan/model/lr-wpan-mac.h:212: static TypeId GetTypeId (void);
Do not remove GetTypeId, even if this is an abstract class.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac...
src/lr-wpan/model/lr-wpan-mac.h:218: static const uint32_t aMinMPDUOverhead;
Do not remove things that are defined in the standard from the base class.
This is valid also for a lot of variables below. Even if you are not going to
use them (because they are relevant only for tree-based mode), leave them in the
base class.
The rationale is: a different MAC could need them, and we'd end up moving hem
back in the base class or in duplicating the code.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac...
src/lr-wpan/model/lr-wpan-mac.h:238: void SetRxOnWhenIdle (bool rxOnWhenIdle);
Even if ContikiMac is explicitly made to put the radio in offline mode, I guess
that this could be useful.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy.h
File src/lr-wpan/model/lr-wpan-phy.h (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy...
src/lr-wpan/model/lr-wpan-phy.h:241: virtual void NotifyTransition (int) = 0;
Not an int - an enum perhaps

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy...
src/lr-wpan/model/lr-wpan-phy.h:285: Time m_rxSetupTime;
I'd use a different name, e.g., m_stateChangeTime.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy...
src/lr-wpan/model/lr-wpan-phy.h:285: Time m_rxSetupTime;
Does it have any connection with aTurnAroundTime ? I.e., is there any constraint
like m_stateChangeTime > aTurnAroundTime ?

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
File src/lr-wpan/model/lr-wpan-radio-energy-model.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
src/lr-wpan/model/lr-wpan-radio-energy-model.cc:281: switch (m_nextState)
This is wrong.
When you have a transition, the current drawn is dependent on the next state and
the previous state, but it is lower than that current of the final state.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
File src/lr-wpan/model/lr-wpan-radio-energy-model.h (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
src/lr-wpan/model/lr-wpan-radio-energy-model.h:186: // The Energy Source and PHY
associated with this model
beware of Doxygen - document also the private part

[vrege, 2015-07-25 10:22:48]

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
File src/lr-wpan/examples/lr-wpan-contikimac-test.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
src/lr-wpan/examples/lr-wpan-contikimac-test.cc:18: * Author:  Tom Henderson
<thomas.r.henderson@boeing.com>
On 2015/07/22 15:46:37, Tommaso Pecorella wrote:
> I don't think you should add Tom, even if you "borrowed" most of his code.

Done.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
File src/lr-wpan/examples/lr-wpan-data.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/examples/lr-wpan-...
src/lr-wpan/examples/lr-wpan-data.cc:48: std::cout << "Received packet of size "
<< p->GetSize () << "\n";
On 2015/07/22 15:46:37, Tommaso Pecorella wrote:
> Remember to revert to NS_LOG_UNCOND before the final release.

Done.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
File src/lr-wpan/model/lr-wpan-contikimac.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
src/lr-wpan/model/lr-wpan-contikimac.cc:24: *  Vishwesh Rege
<vrege2012@gmail.com>
On 2015/07/22 15:46:37, Tommaso Pecorella wrote:
> Only the right authors (you)

Done.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
File src/lr-wpan/model/lr-wpan-contikimac.h (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-con...
src/lr-wpan/model/lr-wpan-contikimac.h:24: */
On 2015/07/22 15:46:37, Tommaso Pecorella wrote:
> see the comment on the .cc - just one author in this case

Done.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac.h
File src/lr-wpan/model/lr-wpan-mac.h (left):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac...
src/lr-wpan/model/lr-wpan-mac.h:212: static TypeId GetTypeId (void);
On 2015/07/22 15:46:37, Tommaso Pecorella wrote:
> Do not remove GetTypeId, even if this is an abstract class.

Keeping GetTypeId in the abstract class results in errors... Is there anything
else I missed?

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-mac...
src/lr-wpan/model/lr-wpan-mac.h:218: static const uint32_t aMinMPDUOverhead;
On 2015/07/22 15:46:38, Tommaso Pecorella wrote:
> Do not remove things that are defined in the standard from the base class.
> This is valid also for a lot of variables below. Even if you are not going to
> use them (because they are relevant only for tree-based mode), leave them in
the
> base class.
> The rationale is: a different MAC could need them, and we'd end up moving hem
> back in the base class or in duplicating the code. 

The reason I had to move this declaration from the base class to the derived
classes is because the compiler complains about 'multiple definitions of
LrWpanMac::aMinMPDUOverhead' in lr-wpan-nullmac.cc and lr-wpan-contikimac.cc in
case this declaration is kept in the base class and defined separately in the
derived classes.

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy.h
File src/lr-wpan/model/lr-wpan-phy.h (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy...
src/lr-wpan/model/lr-wpan-phy.h:241: virtual void NotifyTransition (int) = 0;
On 2015/07/22 15:46:38, Tommaso Pecorella wrote:
> Not an int - an enum perhaps

Specifying the argument as enum - LrWpanPhy::State instead of int would cause
problems since LrWpanPhy::State isn't declared yet. I can't declare this class
after LrWpanPhy since the Phy class maintains a list of listeners which requires
that LrWpanPhyListener be defined before. Any workaround?

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-phy...
src/lr-wpan/model/lr-wpan-phy.h:285: Time m_rxSetupTime;
On 2015/07/22 15:46:38, Tommaso Pecorella wrote:
> Does it have any connection with aTurnAroundTime ? I.e., is there any
constraint
> like m_stateChangeTime > aTurnAroundTime ?

This is the time to switch between ON (RX_ON or TX_ON) and OFF (TRX_OFF) states.
It isn't related to aTurnAroundTime (TX/RX switching time). The value isn't well
defined though.

Reference:
http://www.anacom.pt/streaming/Impact_Switching_7congressURSI.pdf?contentId=1...

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
File src/lr-wpan/model/lr-wpan-radio-energy-model.cc (right):

https://codereview.appspot.com/254120043/diff/1/src/lr-wpan/model/lr-wpan-rad...
src/lr-wpan/model/lr-wpan-radio-energy-model.cc:281: switch (m_nextState)
On 2015/07/22 15:46:38, Tommaso Pecorella wrote:
> This is wrong.
> When you have a transition, the current drawn is dependent on the next state
and
> the previous state, but it is lower than that current of the final state.

I have found some models prefer to use next state, some use the worst case
current draw, but I couldn't find any reference to the actual current draw while
switching states. Maybe I could use the average current draw of the two states?