Simple-wireless-tdma

This issue is created to review the code that adds a simple centralized TDMA Mac protocol to ns-3. This model is built on top of a simple-wireless channel model. The TDMA slot assignment is done by a centralized tdmacontroller. Users could provide the slot assignments either through the simulation script or through an external file.

[Tom Henderson, 2011-08-16 05:39:49]

This looks close to merging; there are several small issues left inline in the
review.  It will be nice to add a TDMA model to ns-3.

In general, there is heavy reuse of WiFi code that inherits WiFi-specific
attributes such as framing so I'm wondering whether it is trying to model
something like 802.11p or whether it is purely due to code reuse.  Suggest to
discuss this in the .rst file somewhere.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/doc/tdm...
File src/simple-wireless-tdma/doc/tdma.rst (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/doc/tdm...
src/simple-wireless-tdma/doc/tdma.rst:59: TDMA maintains a drop-tail queue to
store packets received from the network layer until it gets its
does it require a drop-tail queue?  Could it use the RED queue that is
forthcoming for ns-3?

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/doc/tdm...
src/simple-wireless-tdma/doc/tdma.rst:60: transmission slot. The attributes that
can me modified for this class are **MacQueueLength** and
be modified

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/doc/tdm...
src/simple-wireless-tdma/doc/tdma.rst:69: sessions and authorizes the nodes to
transmit in the slots specified by it. The number of slots alloted for
allotted

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/example...
File src/simple-wireless-tdma/examples/tdma-example.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/example...
src/simple-wireless-tdma/examples/tdma-example.cc:48: 
please comment here on what this example is trying to exemplify

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/example...
src/simple-wireless-tdma/examples/tdma-example.cc:131: cmd.AddValue ("nWifis",
"Number of wifi nodes[Default:30]", nWifis);
are these always wifi nodes, or only if "usingWifi" is set?

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/example...
src/simple-wireless-tdma/examples/tdma-example.cc:145: cmd.AddValue
("gaurdTime", "Duration to wait between slots [Default(us):0]", gaurdTime);
guardTime

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/example...
src/simple-wireless-tdma/examples/tdma-example.cc:424: void
maybe a comment here would help on what this method is doing (and what are the
units of these values)

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/helper/...
File src/simple-wireless-tdma/helper/tdma-controller-helper.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/helper/...
src/simple-wireless-tdma/helper/tdma-controller-helper.h:41: * \brief Helper
class that adds AODV routing to nodes.
doxygen seems to have been copied from AODV

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/s...
File src/simple-wireless-tdma/model/simple-wireless-channel.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/s...
src/simple-wireless-tdma/model/simple-wireless-channel.cc:61: for
(TdmaMacLowList::const_iterator i = m_tdmaMacLowList.begin (); i !=
m_tdmaMacLowList.end (); ++i)
Should this be a TdmaWirelessChannel instead of a SimpleWirelessChannel?  You
are embedding Tdma-specific stuff into this class, so it makes
"SimpleWirelessChannel" unavailable in the future for other MACs.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/s...
File src/simple-wireless-tdma/model/simple-wireless-channel.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/s...
src/simple-wireless-tdma/model/simple-wireless-channel.h:38: * \ingroup channel
may want to add to group tdma instead

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/s...
src/simple-wireless-tdma/model/simple-wireless-channel.h:41: class
SimpleWirelessChannel : public Channel
again, perhaps something like SimpleTdmaWirelessChannel would be better name.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-central-mac.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-central-mac.h:52: * slot interval which
would take affect from the next epoch.
I could not read clearly this last sentence, suggest to rephrase.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-controller.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-controller.cc:56:
TdmaController::GetDefaultGaurdTime (void)
again, please audit this code for "Gaurd" misspelling.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-controller.cc:290: NS_ASSERT_MSG
(packet->GetSize () < 1500,"PacketSize must be less than 1500B, it is: " <<
packet->GetSize ());
Why is 1500 hard coded here?

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-controller.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-controller.h:147: //  EventId
m_accessTimeout;
suggest to delete these if they are going to stay commented out

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-controller.h:162: #endif /*
AERO_TDMA_CONTROLLER_H */
this is no longer AERO_TDMA_CONTROLLER

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac-low.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-low.cc:82: return m_bssid;
Bssid seems WiFi-specific; please explain.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-low.cc:129: WifiMacTrailer fcs;
not sure why you are using WiFi framing here and introducing WiFi dependency

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-low.cc:168: //HERE IT IS
SIMPLEWIRELESSCHANNEL SEND CALL
suggest to delete this comment

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac-low.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-low.h:66: void SetBssid (Mac48Address
ad);
needs doxygen

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac-net-device.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-net-device.cc:57:
MakeUintegerChecker<uint16_t> (1,MAX_MSDU_SIZE - LLC_SNAP_HEADER_LENGTH))
not clear where these constants (MAX_MSDU_SIZE, LLC_SNAP_HEADER_LENGTH) are
coming from or why they relate to TDMA.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-net-device.cc:108:
NS_LOG_FUNCTION_NOARGS ();
Use NS_LOG_FUNCTION (this) for non-static class member methods.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac-queue.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-queue.h:47: * Specifically, it refers to
802.11e/D9, section 9.9.1.6, paragraph 6.
Is this 802.11e statement true, or just copied from wifi-mac-queue.h?

Also, please be careful to retain original file copyrights if you copy over a
file and substantially reuse it for a similar class.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac-queue.h:56: */
Isn't this a copy of wifi-mac-queue.h?  Do we need to maintain almost the same
copy of WifiMacQueue as in WiFi?

Also, please be careful if you copy over files and only slightly modify them,
not to remove the original copyright statement.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac.cc:47: return Seconds (1000.0 /
300000000.0);
Is 1000m a reasonable default max for TDMA?  This was copied from WiFiMac

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
File src/simple-wireless-tdma/model/tdma-mac.h (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac.h:46: * (association/disassociation
state machines).
This doxygen needs updating.

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/model/t...
src/simple-wireless-tdma/model/tdma-mac.h:93: * dequeued as soon as the DCF
function determines that
again, DCF is a WiFi MAC construct

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/test/td...
File src/simple-wireless-tdma/test/tdma-test.cc (right):

http://codereview.appspot.com/4747046/diff/1/src/simple-wireless-tdma/test/td...
src/simple-wireless-tdma/test/tdma-test.cc:193: NS_TEST_ASSERT_MSG_EQ
(now.GetMicroSeconds (),1033,"XXX");
suggest not using XXX; either "" or a real string.

http://codereview.appspot.com/4747046/diff/1/src/wifi/wscript
File src/wifi/wscript (right):

http://codereview.appspot.com/4747046/diff/1/src/wifi/wscript#newcode118
src/wifi/wscript:118: 'model/wifi-mac-trailer.h',
this is probably a separate bug that should be dealt with outside of this patch

[nsiceiu_gmail.com, 2015-05-26 06:45:43]

On 2011/08/16 05:39:49, Tom Henderson wrote:
> This looks close to merging; there are several small issues left inline in the
> review.  It will be nice to add a TDMA model to ns-3.
> 
> In general, there is heavy reuse of WiFi code that inherits WiFi-specific
> attributes such as framing so I'm wondering whether it is trying to model
> something like 802.11p or whether it is purely due to code reuse.  Suggest to
> discuss this in the .rst file somewhere.
> 

I compiled successfully the above TDMA MAC protocol.
In this model, Tdma schedules every node in round robin manner, but I want there
should not be any priority among nodes.
That means, each node must use its own slot even it generates before getting its
own slot.
Example, node 1 should use 1-th slot
               node 3 should use 1-th slot
                :
                :
              node 50 should use 50-th slot
Here, if all nodes generates data traffic at the same time, then we get the
proper slot sequence as our need.
But, when each node generates data at random time, this model apply round robin
manner.
Could you please help me to implement TDMA MAC protocol without round robin
manner.

Thanks,

Nurullah Shahin

[nsiceiu_gmail.com, 2015-05-26 06:46:49]

I compiled successfully the TDMA MAC protocol from
http://code.nsnam.org/tomh/ns-3-simple-wireless/
In this model, Tdma schedules every node in round robin manner, but I want there
should not be any priority among nodes.
That means, each node must use its own slot even it generates before getting its
own slot.
Example, node 1 should use 1-th slot
               node 3 should use 1-th slot
                :
                :
              node 50 should use 50-th slot
Here, if all nodes generates data traffic at the same time, then we get the
proper slot sequence as our need.
But, when each node generates data at random time, this model apply round robin
manner.
Could you please help me to implement TDMA MAC protocol without round robin
manner.

Thanks,

Nurullah Shahin