[ROS2] Lidar add instance and semantic segmentation #713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fireronin
force-pushed
the
add-instance-segmentation-dev
branch
from
c46a1f0
to
3454938
Compare
byrcolin
added
the
sig/simulation
adamdbrw
requested changes
Jun 27, 2024
There was a problem hiding this comment.
This is a nice feature, though there are some issues in the implementation that I wish to be addressed:
- The main publish procedure is now too big, difficult to understand and error-prone. We need subroutines.
- The new feature is not separated or abstracted enough from existing lidar classes. More abstraction of "lidar fields" would be better.
Fireronin
force-pushed
the
add-instance-segmentation-dev
branch
from
758f55c
to
4cbb9a8
Compare
adamdbrw
reviewed
Jul 8, 2024
4 tasks
Fireronin
force-pushed
the
add-instance-segmentation-dev
branch
from
f3a1750
to
fce7316
Compare
This was referenced Jul 25, 2024
Fireronin
force-pushed
the
add-instance-segmentation-dev
branch
from
fce7316
to
df92e60
Compare
Signed-off-by: Paweł Liberadzki <[email protected]> Entity id, and class id published to rviz2, preparation for building UI Signed-off-by: Krzysztof Rymski <[email protected]> Optimized field sizes Changed constructors, system right now is functional but not yet finished Signed-off-by: Krzysztof Rymski <[email protected]> Cleanup Changed constructors, system right now is functional but not yet finished Signed-off-by: Krzysztof Rymski <[email protected]> Cleanup part2 Changed constructors, system right now is functional but not yet finished Signed-off-by: Krzysztof Rymski <[email protected]> Added GUI config for Lidar segmentation Config supports tag names, ids and colors all updated OnLidarRaycaser Ready or something like this Signed-off-by: Krzysztof Rymski <[email protected]> formating Support for publishing segmentation classes data Signed-off-by: Krzysztof Rymski <[email protected]> formating Adressing formating issues and similar Formating Constant bug fix init pretty gui change to unoredered set now segmentation is optional Signed-off-by: Krzysztof Rymski <[email protected]> Refactoring Signed-off-by: Krzysztof Rymski <[email protected]>
Fireronin
force-pushed
the
add-instance-segmentation-dev
branch
from
df92e60
to
b2590aa
Compare
|
Please rebase and ask me to review again |
After internal conversations with @PawelLiberadzki we've decided that this PR will be rebased by me after the Raycast Result refactor (on which I'm currently working) is merged. This refactor will soon be published as a separate PR. For more context please checkout #736. |
|
@jhanca-robotecai Please close this pull request in favour of #754. I do not have such permissions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This pr adds support for new lidar feature, that is instance and semantic segmentation.
It extends relevant fields and classes to allow implementations of lidar to support this.
It's is made in such a way that lidar implementation that do not support this feature will not be impacted.
How was this PR tested?
It was tested internally, and on simple projects