Feature/add lidar depth #9
Conversation
|
New documentation is in feature/better-submap-align. |
| img_header = self.img_data.img_header(t) | ||
|
|
||
| # get exact times of point cloud and image messages | ||
| pointcloud_time = pcl.header.stamp.sec + 1e-9 * pcl.header.stamp.nanosec | ||
| img_time = img_header.stamp.sec + 1e-9 * img_header.stamp.nanosec |
There was a problem hiding this comment.
I think using the header attribute constrains us to only using ros data. The robot data should all be essentially a list of times and datapoints at those times, so you should be able to get the time with:
pointcloud_time = self.pointcloud_data.nearest_time(t)
img_time = self.img_data.nearest_time(t)
And that way if we want to use pointclouds from other source we aren't accessing a non-existant header
| @@ -111,13 +114,14 @@ def __init__(self, | |||
| or self.rotate_img == '180', "Invalid rotate_img option." | |||
|
|
|||
| @classmethod | |||
| def from_params(cls, params: FastSAMParams, depth_cam_params: CameraParams): | |||
| def from_params(cls, params: FastSAMParams, depth_cam_params: CameraParams, use_pointcloud: bool): | |||
There was a problem hiding this comment.
Can we make use_pointcloud part of the FastSAM params? The params in FastSAM are a mess and we should probably clean them up, but one of the things I wanted to do with the from_params function is try to contain things a little bit so that when using the FastsamWrapper you only have to think about things in one central place.
| eroded_mask = cv.erode(mask, self.erosion_element) | ||
| depth_obj[eroded_mask==0] = 0 | ||
| if depth_data is not None: | ||
| if self.use_pointcloud: |
There was a problem hiding this comment.
Could we clean up these comments a bit for the PR?
| self.fastsam = FastSAMWrapper.from_params(params=self.fastsam_params, | ||
| depth_cam_params=self.img_data.camera_params if self.data_params.use_pointcloud else | ||
| self.depth_data.camera_params, | ||
| use_pointcloud=self.data_params.use_pointcloud) # TODO: clean up? |
There was a problem hiding this comment.
See earlier, but I think we should just add a param to the fastsam params for pointcloud as well. It's a bit redundant but also makes it more general to running online (in ros) and not using this offline runner.
There was a problem hiding this comment.
@AndyLi23 this looks good to me! If you can make the few changes above this should be good to get merged in!
I did change how the params files are handled to reduce repetition, so changes might be breaking:
run_data = {**data, **data[run]}in data_params.py makes it possible to have params shared between runs AND params specific to runs (e.g. same robot topics, different timestamps for different runs).The time params format was redundant, I changed it so all that is needed to set the start & end time is:
use_pointcloud: Trueordepth_source: 'pointcloud'Example showing these changes:
Still needs official documentation & probably some cleanup, but wanted to start the merging process.