Bug 511 - Need ability to change mobility model
Need ability to change mobility model
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: mobility models
ns-3-dev
All All
: P3 normal
Assigned To: Mathieu Lacage
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-25 06:47 UTC by Gustavo J. A. M. Carneiro
Modified: 2009-04-17 10:35 UTC (History)
2 users (show)

See Also:


Attachments
my current patch (3.75 KB, patch)
2009-04-16 06:55 UTC, Gustavo J. A. M. Carneiro
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo J. A. M. Carneiro 2009-02-25 06:47:26 UTC
Writing complex mobility models is not easy.  In my simulation of vehicles, I have a state machine for buses:

1. stopped
2. accelerate
3. constant velocity
4. decelerate
5. goto 1

It is much easier to just keep switching among several simple mobility model as the state changes, rather than write one big complex mobility model from scratch.

However, Object::AggregateObject does not allow changing one mobility model for another, and for good reason.  NetDevices could want to cache the mobility model, to avoid calling GetObject on a per packet basis, for instance.

In my tree I worked around this issue by hacking HierarchicalMobilityModel and make it accept a NULL parent.  This way, I can modify the Child mobility model as many times as I want, and with a NULL parent I made the model just return the absolute child position.

I could post these changes I made to that model, or we can come up with a new "container" mobility model to do the same.  I am ok either way, but the first case means one less class to worry about.
Comment 1 Mathieu Lacage 2009-04-16 05:12:40 UTC
Would you mind attach your patch for review ?
Comment 2 Gustavo J. A. M. Carneiro 2009-04-16 06:55:20 UTC
Created attachment 417 [details]
my current patch

Sorry, I have been having problems with codereview behind the company proxy, so here's the patch I use now.
Comment 3 Mathieu Lacage 2009-04-16 07:01:36 UTC
thanks, I see.

I think that the challenge with this approach is that it somewhat abandons an ns-3 invariant which is that, once created, the whole object topology does not change. Specifically, this means that the ConfigStore won't be able to capture the parameters of the models you change on the fly. Don't you have a per-node object which manages the mobility model changes ? If so, can't you just make this per-node object be a MobilityModel subclass ?
Comment 4 Gustavo J. A. M. Carneiro 2009-04-16 07:18:00 UTC
(In reply to comment #3)
> thanks, I see.
> 
> I think that the challenge with this approach is that it somewhat abandons an
> ns-3 invariant which is that, once created, the whole object topology does not
> change. Specifically, this means that the ConfigStore won't be able to capture
> the parameters of the models you change on the fly. Don't you have a per-node
> object which manages the mobility model changes ? If so, can't you just make
> this per-node object be a MobilityModel subclass ?
> 

I have such an object, but it is in Python, and for performance reasons it cannot directly be a mobility model.

This is not really topology information, IMHO.  The main point of HierarchicalMobilityModel is that you can have a node traveling inside a vehicular node, and I am putting this model to good use.  However, this HierarchicalMobilityModel is not very useful if a node is always attached to the vehicle and cannot leave the vehicle or switch to another vehicle.  I know you agree with me because the attributes Parent and Child had already existed before this patch :-)

My patch is only adding a special value to the Parent attribute.  Parent == NULL is currently invalid; with my patch, Parent == NULL means "take the Child mobility model as absolute coordinates, relative to the world (0,0,0)".

In other words, with the patch, Parent == NULL has the same effect as Parent == StaticMobilityModel(0,0,0), but is simpler and more efficient.
Comment 5 Mathieu Lacage 2009-04-16 07:27:28 UTC
(In reply to comment #4)

> > I think that the challenge with this approach is that it somewhat abandons an
> > ns-3 invariant which is that, once created, the whole object topology does not
> > change. Specifically, this means that the ConfigStore won't be able to capture
> > the parameters of the models you change on the fly. Don't you have a per-node
> > object which manages the mobility model changes ? If so, can't you just make
> > this per-node object be a MobilityModel subclass ?
> > 
> 
> I have such an object, but it is in Python, and for performance reasons it
> cannot directly be a mobility model.
> 
> This is not really topology information, IMHO.  The main point of

I said "object topology", not "network topology" and, by that, I meant: "the graph of pointer interconnections of C++ objects deriving from ns3::Object".

> HierarchicalMobilityModel is that you can have a node traveling inside a
> vehicular node, and I am putting this model to good use.  However, this
> HierarchicalMobilityModel is not very useful if a node is always attached to
> the vehicle and cannot leave the vehicle or switch to another vehicle.  I know
> you agree with me because the attributes Parent and Child had already existed
> before this patch :-)

Nice try, but, the reason why Parent and Child attributes where there was only to make is possible to control from ConfigStore and GtkConfigStore the attributes of the child and parent models. i.e., to read them. I made them settable because I am stupid.

> My patch is only adding a special value to the Parent attribute.  Parent ==
> NULL is currently invalid; with my patch, Parent == NULL means "take the Child
> mobility model as absolute coordinates, relative to the world (0,0,0)".
> 
> In other words, with the patch, Parent == NULL has the same effect as Parent ==
> StaticMobilityModel(0,0,0), but is simpler and more efficient.

I could be convinced to pull this in if you beef up the documentation :)

To summarize, that specific patch is ok if there is enough documentation but, the way you use this code in your tree is just plain evil and I hope no one ever gets the same idea :)

Comment 6 Gustavo J. A. M. Carneiro 2009-04-16 07:34:56 UTC
Well, if you think settable Parent and Child attributes are evil, then there is no point in the patch, documentation or not, is there?

Personally I don't think it's evil because I do not use ConfigStore.  But feel free to mark this WONTFIX if you think this is evil.
Comment 7 Mathieu Lacage 2009-04-16 07:43:43 UTC
ok, let's make a deal: if I take this patch, you become the maintainer of this class
Comment 8 Gustavo J. A. M. Carneiro 2009-04-17 06:20:20 UTC
I don't mind being maintainer of this class.  It's pretty simple code, actually.
Comment 9 Mathieu Lacage 2009-04-17 06:32:48 UTC
feel free to push your patch then