API: Problem.addMap(), addHardware(), addTrajectory()

Created by: joansola

These methods are not adding , but setting, the Map, Hardware and Trajectory. Should we rename them?

• setMap()
• setHardware()
• setTrajectory()

Alternatively, and to keep the addXxx() API, does it make sense to have:

• a list of Maps? <-- yes, it seems we could:
  • open maps and closed maps
  • maps of areas that are far from the current area
  • maps of incompatible landmarks, managed by different processors and managers
• a list of Trajectories? -- one for each sensor, or for each sensor bundle?
• a list of Hardware? <-- no, in my opinion

As for now, my opinion is that these 3 methods would be better off as setXxx().

Created by: joanvallve

I would face this issue globally. From node linked, "add" means adding an already created node to the tree. It can be confusing, though. Problem only have one of each of these three nodes, so it is not exactly the same as adding a feature to a capture...

Created by: joansola

I think now my logic is this:

• addXxx() add an already built item to a list of existing items
• createXxx() create and add to list
• setXxx() set a stand-alone item (no list)

Note: createXxx() is kind of ambiguous. What about:

• createXxx() only crete, return a pointer to the created object.
• incorporateXxx() create and add <-- yes, the name is not good...

Created by: joansola

Then,

• addMap() would add all the landmarks in the provided map to the existing map, similar to what we did in the processors with addFeaturesList()

Created by: joanvallve

If it is equivalent to addFeatureList(), it should be addLandmarkList() instead of addMap()

Created by: joanvallve

Taking in consideration all you've said... what if the problem has a Hardware, a Trajectory and a Map, and we can add to them whatever we need to add..?

Created by: joansola

You mean, not pointers to H T M, but real objects? Why not, but as I say above, maybe we want to consider having multiple T and multiple M (sure not multiple H, unless we want to solve a problem with multiple robots, which is out of my scope...)

Created by: joanvallve

I wasn't meaning not having pointers, just not allowing to the constructor to provide them, and removing the add() for those three things..

Apart from that... if we are open to have different maps and trajectories.. why not different robots?

Created by: joansola

My concern, however, is not as much these three HTM things, but rather to have a consistent API that gives a unique value to words such as add, set, append, create, remove, delete, register, install, etc.

I think this is very very important. Then what happens to HTM is just a consequence.

Created by: joansola

So, let me try:

• set: set a single item setX(X& x);
• create: create an object X* x = createX(params) and even, for factories, create("X", params_ptr) see #58 (closed) .
• append: append an existing object to a list, or a list of objects to another list appendX(X* x) and appendX(const list<X*>& Lx)
• add: this could be create and append addX("type",params){appendX(create("type",params_ptr));}
• move, or transfer: move things from one list to the other, so that after that, the source list is empty transferX(list<X*>& Lx).
• install: provide software to hardware (install processor to sensor, for example)
• install: provide hardware to hardware (install the sensors to the robot, for example)

Created by: joanvallve

I agree, the issue with HTM is that despite being nodelinkeds, there are no lists containing them, so adding them to the wolf tree is not coherent to be said add().

My point is that there is no need to add HTM..

Created by: joansola

Yes, that's the idea. We'll need to come with a coherent set of keywords soon otherwise we'll get crazy! :-D

Created by: joanvallve

I like it, but we should discuss about the scope of wolf. I don't see a reason why more than one robot is out of it... but otherwise we could have more than one map or trajectory... Maybe, H T and M are not going to be a single item... so "set" wouldn't be needed..

I like "move" btw, it is useful!

Created by: joansola

Nice discussion!

I agree that we do not need to add HTM. They can be 'added' at construction time in Problem!

Created by: joansola

The scope of Wolf, by now, is the one we are giving to it, with a single H, a single T and a single M!

I think now it is good enough like this... despite of my comments about future extensions.

So, really, we could put HTM as members of Problem, not pointers.

Created by: joansola

I think 'transfer' is more explicit than 'move'. What do you think? Although 'move' is quite good! :-D

Created by: joanvallve

I would leave them as pointers.. because I don't see a good reason for the change, I only see a reason for removing the add() and creating them in the constructor.. I think it is more coherent and less changes.

I like move, like std::move... don't it does the same?

Created by: joansola

OK then, yes it's true, the pointers are there and they can stey there as long as the 'new' is done in the constructor, the 'delete' in the destructor, and we remove the addHTM() API.

Created by: joansola

Let's keep move() !!!

Created by: joansola

Actually, maybe install could be the create and append: install("X", params_ptr) {appendX(create("X",params_ptr));}

because 'install' seems more complex than 'add' in real life, it makes more sense to use it like this.

Man, install("refrigerator", "siemens", "kitchen")

And Man knows what to do.

Created by: joanvallve

in this way, we could keep add as adding an existing item..?

Created by: joansola

I guess so, then save a lot of work of changing everything:

• set: set a single item setX(const X& x);
• create: create an object X* x = createX(params) and even, for factories, create("X", params_ptr) see #58 (closed) .
• add: append an existing object to a list, or a list of objects to another list addX(const X* x) and addX(const list<X*>& Lx)
• move: move things from one list to the other, so that after that, the source list is empty moveX(list<X*>& Lx).
• install: this could be create and add installX("type",params){addX(createX("type",params_ptr));} see also #58 (closed)

Created by: joanvallve

Then, I think we won't need set for wolf nodes anymore, only for parameters of derived classes.

I like the add() for adding both: elements and lists.

The move() could be implemented in nodeLinked level and for lists and elements.. nodeLinked::move(upperNodeList&) and `nodeLinked::move(uperNodePtr). What do you think?

Created by: joansola

1/ OK 2/ OK 3/ OK. You want to do it?

Created by: joansola

We still have to clarify:

• add() adds to the list, and links to uppernode.
• append() only add to the list ? Or we just do not need this, no?

Created by: joanvallve

Now, append and add are equivalents. The first adds a node and the second adds a list of nodes.. Out of nodeLinked, there is no API for partially link to the tree. add and append = pushback in the list + link upper node.

Created by: joansola

OK then only add() is enough. For both nodes and lists of nodes -- we just overload the function

• add(const X& x)
• add(const list<X*>& Lx)

Created by: joansola

And what happens in NodeLinked? Do we keep just the push_back() API of the list<> for elementary operations, and then add() is always doing the link operation? Or how we differentiate one thing to the other within NodeLinked?

Created by: joanvallve

I don't understand the question... :( Can we discuss it live?

Created by: joansola

Nevermind, I have checked NodeLinked and what I say makes no sense. All the addXxx() operations are doing the link to uppernode! so OK.

Created by: joansola

@joanvallve , I remember we coded some functions using addXxxList() that were actually moves, because the input list was emptied after the call. Do you remember? Can we rename these methods according to what we discussed and agreed in this thread?

Mentioned in issue #109 (closed)

This discussion will be deprecated after !256 (merged), we agreed on the new emplace and link functions for this functionalities.

closed