Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(13)

Issue 254120043: LR-WPAN ContikiMAC

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years ago by vrege
Modified:
3 years, 11 months ago
Reviewers:
Tommaso Pecorella
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 21
Unified diffs Side-by-side diffs Delta from patch set Stats (+4651 lines, -1449 lines) Patch
M src/energy/model/basic-energy-source.cc View 1 chunk +9 lines, -1 line 0 comments Download
M src/lr-wpan/doc/lr-wpan.rst View 1 chunk +13 lines, -0 lines 0 comments Download
A src/lr-wpan/examples/lr-wpan-contikimac-test.cc View 1 chunk +184 lines, -0 lines 2 comments Download
M src/lr-wpan/examples/lr-wpan-data.cc View 6 chunks +43 lines, -7 lines 2 comments Download
M src/lr-wpan/helper/lr-wpan-helper.cc View 1 chunk +3 lines, -1 line 0 comments Download
A src/lr-wpan/model/lr-wpan-contikimac.h View 1 chunk +638 lines, -0 lines 2 comments Download
A src/lr-wpan/model/lr-wpan-contikimac.cc View 1 chunk +1260 lines, -0 lines 2 comments Download
M src/lr-wpan/model/lr-wpan-mac.h View 14 chunks +29 lines, -373 lines 5 comments Download
R src/lr-wpan/model/lr-wpan-mac.cc View 1 chunk +0 lines, -1041 lines 0 comments Download
M src/lr-wpan/model/lr-wpan-net-device.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/lr-wpan/model/lr-wpan-net-device.cc View 6 chunks +16 lines, -9 lines 0 comments Download
A src/lr-wpan/model/lr-wpan-nullmac.h View 1 chunk +599 lines, -0 lines 0 comments Download
A src/lr-wpan/model/lr-wpan-nullmac.cc View 1 chunk +1041 lines, -0 lines 0 comments Download
M src/lr-wpan/model/lr-wpan-phy.h View 6 chunks +69 lines, -0 lines 5 comments Download
M src/lr-wpan/model/lr-wpan-phy.cc View 12 chunks +150 lines, -15 lines 0 comments Download
A src/lr-wpan/model/lr-wpan-radio-energy-model.h View 1 chunk +213 lines, -0 lines 1 comment Download
A src/lr-wpan/model/lr-wpan-radio-energy-model.cc View 1 chunk +376 lines, -0 lines 2 comments Download
M src/lr-wpan/wscript View 4 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 2
Tommaso Pecorella
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-contikimac-test.cc File src/lr-wpan/examples/lr-wpan-contikimac-test.cc ...
3 years, 11 months ago (2015-07-22 15:46:38 UTC) #1
vrege
3 years, 11 months ago (2015-07-25 10:22:48 UTC) #2
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?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b