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

Fixed draco subscriber parameter names #43

Merged
merged 4 commits into from
Dec 12, 2023
Merged

Fixed draco subscriber parameter names #43

merged 4 commits into from
Dec 12, 2023

Conversation

ahcorde
Copy link
Collaborator

@ahcorde ahcorde commented Dec 11, 2023 •
edited
Loading

Superseed this PR ros-perception/point_cloud_transport_plugins#37

The subscriber param names:

pct.point_cloud1.draco.skipDequantizationColor
pct.point_cloud1.draco.skipDequantization...
pct.point_cloud1.draco.skipDequantization...
pct.point_cloud1.draco.skipDequantization...
...
pct.point_cloud2.draco.skipDequantizationColor
pct.point_cloud2.draco.skipDequantization...
pct.point_cloud2.draco.skipDequantization...
pct.point_cloud2.draco.skipDequantization...

@ahcorde
Fixed draco subscriber parameter names
21a814a 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from john-maidbot December 11, 2023 17:19
@ahcorde ahcorde self-assigned this Dec 11, 2023
@ahcorde ahcorde mentioned this pull request Dec 11, 2023
Closed
@ahcorde
Removed trace
64149f5 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde
Copy link
Collaborator Author

ahcorde commented Dec 11, 2023

@FranzAlbers

Do you mind to try the combinantion of these two PRs

@FranzAlbers
Copy link

Thanks for helping out so fast, @ahcorde!

I've tested the PR cherrypicked on my ROS2 Humble installation, but I think this shouldn't make a difference.
Two different subscribers can now be instantiated without crash.

The crash still occurs when parameters of the same subscriber are redeclared (e.g., add different two displays for /points/draco or add only one display for /points/draco, rename the topic to something different and then again to /points/draco)

@ahcorde
Fix crash
3252ccd 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde
Copy link
Collaborator Author

ahcorde commented Dec 12, 2023

I made a new change, do you mind to try it @FranzAlbers ?

@ahcorde
make linters happy
d807ed4 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@FranzAlbers
Copy link

Thanks @ahcorde, this fixes the crashes! 👍

john-maidbot
john-maidbot approved these changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@john-maidbot john-maidbot left a comment

Choose a reason for hiding this comment

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

Great find y'all!

@ahcorde ahcorde merged commit 48cd0ce into rolling Dec 12, 2023
3 checks passed
@ahcorde ahcorde deleted the ahcorde/rolling/fixed_subscriber_param_names branch December 12, 2023 14:31
@ahcorde
Copy link
Collaborator Author

ahcorde commented Dec 12, 2023

https://github.com/Mergifyio backport humble iron

@mergify Mergify
Copy link

mergify bot commented Dec 12, 2023 •
edited
Loading

backport humble iron

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Dec 12, 2023
@ahcorde @mergify
Fixed draco subscriber parameter names (#43)
dffef51 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 48cd0ce)
@mergify mergify bot mentioned this pull request Dec 12, 2023
Merged
mergify bot pushed a commit that referenced this pull request Dec 12, 2023
@ahcorde @mergify
Fixed draco subscriber parameter names (#43)
bc8c0b6 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 48cd0ce)
@mergify mergify bot mentioned this pull request Dec 12, 2023
Merged
ahcorde added a commit that referenced this pull request Dec 12, 2023
@mergify @ahcorde
Fixed draco subscriber parameter names (#43) (#45)
88575d9 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 48cd0ce)

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
ahcorde added a commit that referenced this pull request Dec 12, 2023
@mergify @ahcorde
Fixed draco subscriber parameter names (#43) (#44)
5ade7bc 
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
(cherry picked from commit 48cd0ce)

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-maidbot john-maidbot john-maidbot approved these changes

Assignees

@ahcorde ahcorde

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahcorde @FranzAlbers @john-maidbot