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

Add Parameters to VoxelMap.msg #151

Merged
merged 3 commits into from
May 5, 2022

Conversation

ankitVP77
Copy link
Contributor

@ankitVP77 ankitVP77 commented May 2, 2022

Added the variables, val_occ, val_even, val_unknown, val_free, val_add and val_default to VoxelMap.msg file with their default value. Subsequently replaced hard-codings of all these variables in map_utils.h files of the mpl and jps3d packages, voxel_mapper.h file from mapper package and global_plan_server.cpp and local_plan_server.cpp files from action_planner package

@fcladera
Copy link
Collaborator

fcladera commented May 2, 2022

Checks seem to be failing. This is a problem on our end.

@versatran01
Copy link
Collaborator

versatran01 commented May 3, 2022

I very much hope that JPS does not depend on planning_ros_msgs.
Please do not merge this atm. I will provide some feedback later.

@versatran01
Copy link
Collaborator

Here are a few high-level points:

  1. JPS should not depend on planning_ros_msgs.
  2. voxel values should not be hardcoded in msg as well, they should be fields, which can be assigned during msg creation. These values should be set in a parameter yaml file read by the mapper.
  3. Main APIs in JPS/MPL should then be augmented to take these values as parameters.

This task is highly non-trivial, you need to look into many files in JPS/MPL to understand how things work.
If you are not familiar with C++, then it is unlikely to fix this in one go.
Therefore, I suggest a preliminary task, which is to add back some of the tests I removed from JPS/MPL. https://github.com/KumarRobotics/jps3d/tree/master/test

Once the tests are in place, you can then proceed to make these voxel values (val_occ,...) input parameters to functions like setMap()

void setMap(const Vecf<Dim> &ori, const Veci<Dim> &dim, const Tmap &map,

@fcladera
Copy link
Collaborator

fcladera commented May 3, 2022

@ankit16-py please fetch, rebase on origin/master and force push to this branch.

@XuRobotics
Copy link
Collaborator

@ankit16-py please fetch, rebase on origin/master and force push to this branch.

Good point, I will do this after I merge the other PR.

Copy link
Collaborator

@XuRobotics XuRobotics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I am going to test it before approving.

@XuRobotics
Copy link
Collaborator

Approving the changes now, but Chao's suggestions need to be considered, probably we will create a new PR for this.

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

Successfully merging this pull request may close these issues.

None yet

4 participants