UAN module addition

[socket, 2010-06-05 09:59:48]

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 of the nodes are not being updated

http://codereview.appspot.com/1350041/diff/1/13
File src/devices/uan/uan-header-rc.cc (right):

http://codereview.appspot.com/1350041/diff/1/13#newcode33
src/devices/uan/uan-header-rc.cc:33: // TODO Auto-generated constructor stub
Delete this autogenerated comment

http://codereview.appspot.com/1350041/diff/1/13#newcode47
src/devices/uan/uan-header-rc.cc:47: // TODO Auto-generated destructor stub
Delete this autogenerated comment

http://codereview.appspot.com/1350041/diff/1/15
File src/devices/uan/uan-mac-aloha.cc (right):

http://codereview.appspot.com/1350041/diff/1/15#newcode85
src/devices/uan/uan-mac-aloha.cc:85: header.SetType (0);
the SetType parameter should be the protocolNumber instead of 0

http://codereview.appspot.com/1350041/diff/1/15#newcode98
src/devices/uan/uan-mac-aloha.cc:98: this->m_forUpCb = cb;
"this->" should be removed and should be left only "m_forUpCb"

http://codereview.appspot.com/1350041/diff/1/15#newcode105
src/devices/uan/uan-mac-aloha.cc:105: phy->SetReceiveErrorCallback (MakeCallback
(&UanMacAloha::RxPacketError, this));
Change "phy->" to "m_phy->"

http://codereview.appspot.com/1350041/diff/1/17
File src/devices/uan/uan-mac-cw.cc (right):

http://codereview.appspot.com/1350041/diff/1/17#newcode129
src/devices/uan/uan-mac-cw.cc:129: header.SetType (0);
The SetType parameter should be the protocolNumber instead of 0

[Josh Pelkey, 2010-06-08 19:24:26]

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.  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 :)

[lentracy_u.washington.edu, 2010-06-08 20:36:46]

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
>