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.

Sign up for GitHub

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

Jump to bottom

add support for point clouds over flatbuffers #193

Merged
merged 54 commits into from
Oct 24, 2022
Merged

add support for point clouds over flatbuffers #193

merged 54 commits into from
Oct 24, 2022

Conversation

jarkenau
Copy link
Member

This PR adds support for point clouds over Flatbuffers (Closes #88).

Currently, supported data fields are:

  • x,y,z
  • r,g,b
  • r,gb,a

Left to implement (via separate PRs):

@jarkenau jarkenau added pointclouds flatbuffer hdf5 Something related to the hdf5 files labels Oct 11, 2022
@jarkenau jarkenau self-assigned this Oct 11, 2022
@jarkenau
Copy link
Member Author

I will make it "real" PR after I'm finished with further testing

@jarkenau jarkenau marked this pull request as ready for review October 11, 2022 13:58
@jarkenau
Copy link
Member Author

jarkenau commented Oct 11, 2022 •
edited
Loading

@mhoellmann I think the easiest way to review is, if you check if the queries from the python script work as expected. Also, please have a look at hdf5-fb-pointcloud for improvements. The rest of the code should be pretty straight forward.

Mark-Niemeyer
Mark-Niemeyer reviewed Oct 11, 2022
View reviewed changes
Copy link
Member

@Mark-Niemeyer Mark-Niemeyer left a comment

Choose a reason for hiding this comment

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

This is a really huge PR. For the next feature we should aim for smaller PRs.

I had a quick look at the code without running it. The remarks are just my first thought based purely on the code.

seerep-srv/seerep-core-fb/src/core-fb-conversion.cpp Outdated Show resolved Hide resolved
seerep-srv/seerep-core-fb/src/core-fb-conversion.cpp Outdated Show resolved Hide resolved
seerep-srv/seerep-core-fb/include/seerep-core-fb/core-fb-conversion.h Outdated Show resolved Hide resolved
seerep-hdf5/seerep-hdf5-fb/src/hdf5-fb-general.cpp Outdated Show resolved Hide resolved
seerep-hdf5/seerep-hdf5-fb/src/hdf5-fb-general.cpp Outdated Show resolved Hide resolved
seerep-hdf5/seerep-hdf5-fb/src/hdf5-fb-general.cpp Outdated Show resolved Hide resolved
seerep-hdf5/seerep-hdf5-fb/src/hdf5-fb-general.cpp Outdated Show resolved Hide resolved
@jarkenau jarkenau force-pushed the feat/fb-point-cloud-cleaned-up branch 2 times, most recently from ee17ea5 to 254529f Compare October 13, 2022 07:06
@jarkenau jarkenau mentioned this pull request Oct 17, 2022
Open
Julian Arkenau and others added 16 commits October 20, 2022 12:46
add grpc interface for point clouds
36dab07
@Mark-Niemeyer
fix build error grpc fb service pointcloud
55fd9c0
@Mark-Niemeyer
apply changed pointcloud fb-msg to other pkgs
8f9163a
update point_field fb message, due to name clashing
7ad2fcf
WIP: point cloud iterator for fb
1563b4c
add label with instances to fb point cloud
3261cbe
WIP: fb point cloud
84d4526
refactor to only use reference where possible in hdf5 fb
fe72315
hdf5 fb write for point clouds compiling
e037ca5
add fb to core conversions for point clouds
75aa12c
add service to write fb point clouds
a20c299
add flatbuffer point cloud service implementation
2dd979a
WIP first working fb point cloud example script
42ced15
write fb point clouds with general labels
0fbeb5f
add boudingBoxLabeled into fb point cloud script
e8b9b58
refactor: fb point cloud example to use fb util
fb45b1d
Julian Arkenau added 24 commits October 20, 2022 12:46
rename fb point cloud iterator
817500f
add docs in seerep-hdf5-fb for point clouds
75d4832
remove unused methods from hdfb-fb-pointcloud
ab5bd5b
update flatbuffers pointcloud2iterator
c46a765
add docs to core-fb-pointcloud
3ade865
remove temporary fix from core-fb-pointcloud
87dd1ea
remove quaternion attributes from hdf5-core-point-cloud
0c2b42e
add docs to flatbuffers point cloud service
c997cc8
fix: funcions call in hdf5-fb-point and hdf5-fb-image after rebase
7c30d32
fix: general labels not in indicies for flatbuffer point clouds
9b872b5
update header and query in fb_util to use optional parameters
cc8f42d
fix: bounding box indecies for flatbuffer point clouds
5d1c8e7 
This is currently the only way to get the bounding box into the indecies without looping over the x,y,z data twice
remove TODO in seerep-core-fb-conversions
e4bb7bd
add checks for empty flatbuffers message fields inside write function
fdce344 
stop using references here so that the arguments can be null and checked inside the function
add null check in write of point field attributes
5ce375b
rename conversion functions in core-fb-conversion
02bc3aa
separate calculation of rgba offset in hdf5-fb-pointcloud
059ea30
add method for writing general attributs to the hdf5 data group in hd…
328d98b 
…f5-fb-pointcloud"
add hdf5 data set and data group accessors in hdf5-core-general
03f6215
add method to compute bounding box in hdf5-fb-pointcloud
8cba06a
add methods to calculate multiple offsets for point clouds
0764d5f
add precision for numpy output in point cloud examples
1b155b8
refactor data group accessor method in hdf5-core-general
f077a9f
fix write of rgba field in flatbuffers point cloud
c44fd92
@jarkenau jarkenau force-pushed the feat/fb-point-cloud-cleaned-up branch from c752884 to c44fd92 Compare October 20, 2022 12:53
jarkenau
jarkenau commented Oct 20, 2022
View reviewed changes
Copy link
Member Author

@jarkenau jarkenau left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One way that I could improve the API of the Hdf5FbPointCloud Class would be to extend the CloudInfo struct with more information like height, width etc. and then use it in function parameters. But it's just another abstraction. @mhoellmann do you think it should be included in this PR?

seerep-hdf5/seerep-hdf5-fb/include/seerep-hdf5-fb/hdf5-fb-pointcloud.h Show resolved Hide resolved
seerep-hdf5/seerep-hdf5-fb/include/seerep-hdf5-fb/hdf5-fb-pointcloud.h Show resolved Hide resolved
@Mark-Niemeyer Mark-Niemeyer merged commit e9c0642 into main Oct 24, 2022
@Mark-Niemeyer Mark-Niemeyer deleted the feat/fb-point-cloud-cleaned-up branch October 24, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Niemeyer Mark-Niemeyer Mark-Niemeyer approved these changes

Assignees

@jarkenau jarkenau

Labels
flatbuffer hdf5 Something related to the hdf5 files pointclouds
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pointcloud fb interface
2 participants
@jarkenau @Mark-Niemeyer