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

Use rclcpp::SystemDefaultsQoS() instead of magic numbers (or document magic numbers) #2225

Closed
sjahr opened this issue Jun 5, 2023 · 4 comments · Fixed by #2271
Closed

Use rclcpp::SystemDefaultsQoS() instead of magic numbers (or document magic numbers) #2225

sjahr opened this issue Jun 5, 2023 · 4 comments · Fixed by #2271
Assignees
@Shobuj-Paul
Labels
enhancement New feature or request good first issue Good for newcomers persistent Allows issues to remain open without automatic stalling and closing.

Comments

@sjahr
Copy link
Contributor

sjahr commented Jun 5, 2023

In many places throughout the codebase, we're still using magic numbers to configure QoS for publishers and subscribers for example:
here, here or here.

I think we should replace this in all cases with rclcpp::SystemDefaultsQoS() for consistency and maybe more stability. @henningkayser + @tylerjw What do you think?
@sjahr sjahr added the enhancement New feature or request label Jun 5, 2023
@henningkayser
Copy link
Member

Absolutely, related issue: #1190

@sjahr sjahr added the good first issue Good for newcomers label Jun 6, 2023
@SANTHOSH-MAMIDISETTI
Copy link

@henningkayser @sjahr I would love to work on it , Can I be assigned it ?

@sjahr
Copy link
Contributor Author

sjahr commented Jun 30, 2023 •
edited
Loading

Thanks for your initiative. You can start working on it 👍 Once you've opened a PR we can link it to this issue

@sjahr sjahr added the persistent Allows issues to remain open without automatic stalling and closing. label Jul 12, 2023
@Shobuj-Paul Shobuj-Paul mentioned this issue Jul 23, 2023
Merged
6 tasks
@Shobuj-Paul
Copy link
Contributor

Hi @sjahr
Could you check the draft PR if this was what you were looking for? If this is correct I shall look for more instances and make the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Shobuj-Paul Shobuj-Paul

Labels
enhancement New feature or request good first issue Good for newcomers persistent Allows issues to remain open without automatic stalling and closing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replaced numbers with SystemDefaultsQos()
4 participants
@henningkayser @sjahr @Shobuj-Paul @SANTHOSH-MAMIDISETTI