Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What are the requirements for MapCollection / MapNode? #396

Open
adeilsonsilva opened this issue May 10, 2023 · 3 comments
Open

What are the requirements for MapCollection / MapNode? #396

adeilsonsilva opened this issue May 10, 2023 · 3 comments

Comments

@adeilsonsilva
Copy link

adeilsonsilva commented May 10, 2023

Recently I tried to use MapCollection class, but it seemed to me that this class and its derivatives are still in a early stage, with many TODOs and methods to be improved. I think I was able to understand some of the internal structure but it wasn't clear how to contribute for these classes specifically.

One minor example is that the write method in MapCollection iterate its nodes to create separate files for them. The problem is it tries to append the filename (node id) to the path in a wrong order, creating an invalid filename and thus making writeMap crash.

    std::ofstream outfile(filename.c_str());
    outfile << "#This file was generated by the write-method of MapCollection\n";

    for(typename std::vector<MAPNODE* >::iterator it = nodes.begin(); it != nodes.end(); ++it){
      std::string id = (*it)->getId();
      pose6d origin = (*it)->getOrigin();
      std::string nodemapFilename = "nodemap_";
      nodemapFilename.append(id);
      nodemapFilename.append(".bt");

      outfile << "MAPNODEID " << id << "\n";
      outfile << "MAPNODEFILENAME "<< nodemapFilename << "\n";
      outfile << "MAPNODEPOSE " << origin.x() << " " << origin.y() << " " << origin.z() << " "
              << origin.roll() << " " << origin.pitch() << " " << origin.yaw() << std::endl;
      ok = ok && (*it)->writeMap(nodemapFilename);
    }

Another thing that comes to mind is that MapCollection stores the node as a vector of pointers, leaving memory management to the user:

  protected:

    std::vector<MAPNODE*> nodes;

which may lead to some memory corruption problems (if the user intantiates MAPNODE with new but frees it incorrectly, for example). Maybe it could manage that memory internally.

@ahornung
Copy link
Member

As you have observed, the code was added in a quite early stage and can be considered orphaned and unmaintained at the moment.

Pull requests for improvements are welcome!

@adeilsonsilva
Copy link
Author

As you have observed, the code was added in a quite early stage and can be considered orphaned and unmaintained at the moment.

Pull requests for improvements are welcome!

I will contribute, I'm just not sure if I got a clear enough picture of what theses classes are supposed to be. Is there any information you can share about it, or should I propose something first and we discuss upon that?

@jehontan
Copy link

From what I can tell, it seems like the MapCollection is a collection of submaps represented by MapNodes, and serves as the basis for some map-sharing use-case. Seems like something that would be very useful to properly implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants