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

Issue 1350041: UAN module addition

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by lentracy
Modified:
10 years, 10 months ago
Reviewers:
Josh Pelkey, socket
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Patch Set 1 #

Total comments: 14
Unified diffs Side-by-side diffs Delta from patch set Stats (+10760 lines, -0 lines) Patch
A examples/uan/uan-cw-example.h View 1 chunk +68 lines, -0 lines 0 comments Download
A examples/uan/uan-cw-example.cc View 1 chunk +299 lines, -0 lines 4 comments Download
A examples/uan/uan-rc-example.h View 1 chunk +66 lines, -0 lines 0 comments Download
A examples/uan/uan-rc-example.cc View 1 chunk +304 lines, -0 lines 0 comments Download
A examples/uan/wscript View 1 chunk +11 lines, -0 lines 0 comments Download
A src/devices/uan/uan.h View 1 chunk +182 lines, -0 lines 1 comment Download
A src/devices/uan/uan-address.h View 1 chunk +98 lines, -0 lines 0 comments Download
A src/devices/uan/uan-address.cc View 1 chunk +139 lines, -0 lines 0 comments Download
A src/devices/uan/uan-channel.h View 1 chunk +103 lines, -0 lines 0 comments Download
A src/devices/uan/uan-channel.cc View 1 chunk +187 lines, -0 lines 0 comments Download
A src/devices/uan/uan-header-common.h View 1 chunk +90 lines, -0 lines 0 comments Download
A src/devices/uan/uan-header-common.cc View 1 chunk +129 lines, -0 lines 0 comments Download
A src/devices/uan/uan-header-rc.h View 1 chunk +361 lines, -0 lines 0 comments Download
A src/devices/uan/uan-header-rc.cc View 1 chunk +627 lines, -0 lines 2 comments Download
A src/devices/uan/uan-mac.h View 1 chunk +91 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-aloha.h View 1 chunk +83 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-aloha.cc View 1 chunk +135 lines, -0 lines 3 comments Download
A src/devices/uan/uan-mac-cw.h View 1 chunk +120 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-cw.cc View 1 chunk +367 lines, -0 lines 1 comment Download
A src/devices/uan/uan-mac-rc.h View 1 chunk +171 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-rc.cc View 1 chunk +751 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-rc-gw.h View 1 chunk +162 lines, -0 lines 0 comments Download
A src/devices/uan/uan-mac-rc-gw.cc View 1 chunk +741 lines, -0 lines 1 comment Download
A src/devices/uan/uan-net-device.h View 1 chunk +139 lines, -0 lines 0 comments Download
A src/devices/uan/uan-net-device.cc View 1 chunk +376 lines, -0 lines 0 comments Download
A src/devices/uan/uan-noise-model.h View 1 chunk +41 lines, -0 lines 0 comments Download
A src/devices/uan/uan-noise-model-default.h View 1 chunk +66 lines, -0 lines 0 comments Download
A src/devices/uan/uan-noise-model-default.cc View 1 chunk +80 lines, -0 lines 0 comments Download
A src/devices/uan/uan-phy.h View 1 chunk +307 lines, -0 lines 0 comments Download
A src/devices/uan/uan-phy-dual.h View 1 chunk +241 lines, -0 lines 0 comments Download
A src/devices/uan/uan-phy-dual.cc View 1 chunk +636 lines, -0 lines 0 comments Download
A src/devices/uan/uan-phy-gen.h View 1 chunk +248 lines, -0 lines 0 comments Download
A src/devices/uan/uan-phy-gen.cc View 1 chunk +863 lines, -0 lines 0 comments Download
A src/devices/uan/uan-prop-model.h View 1 chunk +146 lines, -0 lines 1 comment Download
A src/devices/uan/uan-prop-model.cc View 1 chunk +339 lines, -0 lines 0 comments Download
A src/devices/uan/uan-prop-model-ideal.h View 1 chunk +50 lines, -0 lines 0 comments Download
A src/devices/uan/uan-prop-model-ideal.cc View 1 chunk +67 lines, -0 lines 0 comments Download
A src/devices/uan/uan-prop-model-thorp.h View 1 chunk +54 lines, -0 lines 0 comments Download
A src/devices/uan/uan-prop-model-thorp.cc View 1 chunk +95 lines, -0 lines 0 comments Download
A src/devices/uan/uan-test.cc View 1 chunk +269 lines, -0 lines 0 comments Download
A src/devices/uan/uan-transducer.h View 1 chunk +173 lines, -0 lines 1 comment Download
A src/devices/uan/uan-transducer-hd.h View 1 chunk +70 lines, -0 lines 0 comments Download
A src/devices/uan/uan-transducer-hd.cc View 1 chunk +214 lines, -0 lines 0 comments Download
A src/devices/uan/uan-tx-mode.h View 1 chunk +195 lines, -0 lines 0 comments Download
A src/devices/uan/uan-tx-mode.cc View 1 chunk +305 lines, -0 lines 0 comments Download
A src/devices/uan/waf View 1 chunk +1 line, -0 lines 0 comments Download
A src/devices/uan/wscript View 1 chunk +51 lines, -0 lines 0 comments Download
A src/helper/uan-helper.h View 1 chunk +218 lines, -0 lines 0 comments Download
A src/helper/uan-helper.cc View 1 chunk +228 lines, -0 lines 0 comments Download
M src/helper/wscript View 2 chunks +2 lines, -0 lines 0 comments Download
M src/wscript View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3
socket
http://codereview.appspot.com/1350041/diff/1/2 File examples/uan/uan-cw-example.cc (right): http://codereview.appspot.com/1350041/diff/1/2#newcode111 examples/uan/uan-cw-example.cc:111: Vector a (uv.GetValue (), uv.GetValue (), 70.0); The positions ...
10 years, 10 months ago (2010-06-05 09:59:48 UTC) #1
Josh Pelkey
Overall code seems good and organized to me. Style and documentation are good. I did ...
10 years, 10 months ago (2010-06-08 19:24:26 UTC) #2
lentracy_u.washington.edu
10 years, 10 months ago (2010-06-08 20:36:46 UTC) #3
Haven't got internet at my house yet, so I will take care of these and
Andrea's comments very soon I hope (was supposed to be done Sunday, then
today, now maybe later today... probably tomorrow).

I'll run the check-style.py again.. I thought I had done this for all
files.  Maybe I missed the examples.

Thanks,
Leonard


On Tue, Jun 8, 2010 at 12:24 PM, <joshpelkey@gmail.com> wrote:

> Overall code seems good and organized to me.  Style and documentation
> are good.  I did notice a few spots where style was violated (especially
> in examples), for example, missing spaces after parentheses or lines
> possibly over 128 columns.  I would recommend running check-style.py on
> the new files (http://www.nsnam.org/doxygen/group___check_style.html).
> Also, as a lucky contributor to a release >= ns-3.9, doxygen should not
> generate any more warnings.  While the majority of the methods are
> documented, there are a few here and there missing documentation.  I've
> compiled the list of new warnings here:
>
http://www.nsnam.org/~jpelkey3/uan-doxygen-warnings.txt<http://www.nsnam.org/....
>  It's actually
> not that many, considering the amount of code.
>
> Tom has suggested to experiment with this module regarding initial steps
> toward a more modular ns-3 build.  No immediate action necessary from
> Leonard, but I agree that this is a good candidate to try this on.  I
> could see a slight reorganization of the code if placed, for example, in
> src/contrib (ie, some new directories like phy, channel, etc).
>
> Additional, very minor, inline comments below.
>
>
>
> http://codereview.appspot.com/1350041/diff/1/2
> File examples/uan/uan-cw-example.cc (right):
>
> http://codereview.appspot.com/1350041/diff/1/2#newcode29
> examples/uan/uan-cw-example.cc:29: * to the IEEE 802.11 DCF with a
> constant backoff window.  It requires two parameters to bet set,
> typo: be instead of bet
>
> http://codereview.appspot.com/1350041/diff/1/2#newcode35
> examples/uan/uan-cw-example.cc:35: * the X and Y coordinates of the
> nodes distributed uniformly.  The CW parameter is varried throughout
> typo: varied instead of varried
>
> http://codereview.appspot.com/1350041/diff/1/2#newcode36
> examples/uan/uan-cw-example.cc:36: * the simulation in order to show the
> variation in throughout with respect to changes in CW.
> typo: throughput instead of throughout
>
> http://codereview.appspot.com/1350041/diff/1/19
> File src/devices/uan/uan-mac-rc-gw.cc (right):
>
> http://codereview.appspot.com/1350041/diff/1/19#newcode228
> src/devices/uan/uan-mac-rc-gw.cc:228: //
> NS_LOG_DEBUG(Simulator::Now().GetSeconds() << " GW Received in error:
> Packet of type " << type << " from " << ch.GetSrc() << " with SINR " <<
> sinr << " and mode ?");
> is this needed?
>
> http://codereview.appspot.com/1350041/diff/1/39
> File src/devices/uan/uan-prop-model.h (right):
>
> http://codereview.appspot.com/1350041/diff/1/39#newcode123
> src/devices/uan/uan-prop-model.h:123: class UanPropagationModel : public
> Object
> would suggest UanPropModel to align with all the others (and the
> filename)
>
> http://codereview.appspot.com/1350041/diff/1/43
> File src/devices/uan/uan-transducer.h (right):
>
> http://codereview.appspot.com/1350041/diff/1/43#newcode28
> src/devices/uan/uan-transducer.h:28: // #include "uan-phy.h"
> remove
>
> http://codereview.appspot.com/1350041/diff/1/46
> File src/devices/uan/uan.h (right):
>
> http://codereview.appspot.com/1350041/diff/1/46#newcode179
> src/devices/uan/uan.h:179: * c) Simple ALOHA (ns3::UanMacAloha)  Nodes
> transmit at will.
> very nice documentation :)
>
>
> http://codereview.appspot.com/1350041/show
>
Sign in to reply to this message.

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