Bug 1249

Summary:	doxygen comments on device-level SetMobility ()
Product:	ns-3	Reporter:	Tom Henderson <tomh>
Component:	wifi	Assignee:	sebastien.deronne
Status:	RESOLVED FIXED
Severity:	normal	CC:	mathieu.lacage, ns-bugs, ruben
Priority:	P5
Version:	pre-release
Hardware:	All
OS:	All
Attachments:	suggested patch to fix

Description Tom Henderson 2011-08-18 18:52:17 UTC

I'd like to add this type of Doxygen to those classes (e.g. WiFi) that have device-level or Phy-level SetMobility () calls, since the usage of this is kind of subtle.  Do people agree with these clarifications and use patterns?

diff -r f24a2b525ba7 src/wifi/model/yans-wifi-phy.h
--- a/src/wifi/model/yans-wifi-phy.h	Wed Aug 17 13:25:27 2011 -0700
+++ b/src/wifi/model/yans-wifi-phy.h	Mon Aug 15 03:09:03 2011 -0700
@@ -106,6 +106,24 @@
   void SetCcaMode1Threshold (double threshold);
   void SetErrorRateModel (Ptr<ErrorRateModel> rate);
   void SetDevice (Ptr<Object> device);
+  /*
+   * \brief assign either a mobility model or an ns3::Object to which 
+   *  mobility model has been aggregated.
+   *
+   * This method may be used in one of two ways.  Either a mobility model
+   * pointer itself may be passed in, or a pointer to another Object,
+   * typically an ns3::Node, may be passed.  This is a subtle point but
+   * it allows one to either use traditional node-based mobility model
+   * assignment, with use of MobilityHelper class (which assigns 
+   * MobilityModels to Nodes) or to assign mobility models on a 
+   * per-device basis (if that level of position granularity is desired).
+   * The usage pattern of GetMobility() allows either technique to work,
+   * since calls to GetObject<MobilityModel> () on the mobility model
+   * itself will return a pointer to the same mobility model.
+   *
+   * \param mobility a pointer to either an ns3::Object to which a 
+   *  MobilityModel has been aggregated, or to a MobilityModel itself
+   */
   void SetMobility (Ptr<Object> mobility);
   double GetRxNoiseFigure (void) const;
   double GetTxGain (void) const;
@@ -114,6 +132,17 @@
   double GetCcaMode1Threshold (void) const;
   Ptr<ErrorRateModel> GetErrorRateModel (void) const;
   Ptr<Object> GetDevice (void) const;
+  /**
+   * \brief get Object to which MobilityModel has been aggregated
+   * 
+   * This method returns an object (typically, an ns3 Node) to which a
+   * MobilityModel has been aggregated.  Therefore, the typical usage
+   * pattern is `Ptr<MobilityModel> receiverMobility = (*i)->GetMobility ()->GetObject<MobilityModel> ();`; i.e. users are expected to call
+   * GetObject<MobilityModel> () on the Object pointer returned from this
+   * method. 
+   * 
+   * \return ns3::Object that can be queried for an aggregated MobilityModel
+   */
   Ptr<Object> GetMobility (void);

Comment 1 Tom Henderson 2012-01-04 12:40:35 UTC

bump

Comment 2 Nicola Baldo 2012-01-04 13:13:40 UTC

What if we just enforce the use of Ptr<MobilityModel>? 
I think it would be inline with other discussions and modifications that we had recently.

Comment 3 Tom Henderson 2012-01-04 13:27:54 UTC

(In reply to comment #2)
> What if we just enforce the use of Ptr<MobilityModel>? 
> I think it would be inline with other discussions and modifications that we had
> recently.

I would be fine with that solution from an API perspective.  Presumably also m_device could be changed similarly as well.

One aspect of this that could use clarification is the need to call SetMobility().  Presently, one must associate a MobilityModel with the device explicitly; that is, it is not sufficient to aggregate a MobilityModel with the Node (e.g. a multi-device node) and have the channel successfully call GetMobility().  Something like this would make the assignment of SetMobility() optional.

--- a/src/wifi/model/yans-wifi-phy.cc	Sun Nov 13 22:47:56 2011 -0800
+++ b/src/wifi/model/yans-wifi-phy.cc	Wed Jan 04 10:27:05 2012 -0800
@@ -303,7 +303,14 @@
 Ptr<Object>
 YansWifiPhy::GetMobility (void)
 {
-  return m_mobility;
+  if (m_mobility != 0)
+    {
+      return m_mobility;
+    }
+  else
+    {
+      return GetDevice()->GetNode()->GetObject<MobilityModel> ();
+    }
 }

Comment 4 Mathieu Lacage 2012-03-24 09:06:43 UTC

I think that the original reason for the Set/GetMobility method was to allow unit testing of the PHY layer outside of the node.

Comment 5 sebastien.deronne 2015-05-27 10:43:44 UTC

Could we end up to a conclusion?

It's already an old discussion and I do not think it will be once cleaned from the tracker if we do not decide to a final point :)

Comment 6 Tom Henderson 2015-06-10 14:47:53 UTC

Created attachment 2063 [details]
suggested patch to fix

This makes the SetMobility() of the Phy optional; the model defaults to using the mobility model that has been aggregated to the Node, but allows one to override this and set a device-specific mobility object.

This also changes the pointer type of SetNetDevice and SetMobility to be of the respective type, rather than ns3::Object.  It turns out that it is not needed this way for the tests.  The Ptr<Object> was being exploited by the YansWifiPhyHelper to set a mobility pointer to the node since at the time of Phy creation, the mobility pointer was not yet available.  However, the current patch addresses this in a different way.

Comment 7 sebastien.deronne 2015-06-10 15:18:55 UTC

Tom, thanks for the fix.
I find this solution indeed better and cleaner than the existing one, I do not have further comments on your patch.

Comment 8 sebastien.deronne 2015-06-22 12:45:31 UTC

Fix committed in changeset 11451:36f951da53ac