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.

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

Modification of sensor data publisher #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Hiroaki-Masuzawa
Copy link
Contributor

The following modifications have been changed.

  • Skip the publishing process when there is no connection.
  • Modified publishing data type according to format.
  • Add CameraInfo publisher.

@ssr-yuki
Copy link
Member

ssr-yuki commented Jun 6, 2023

Thank you. Your suggestions are important.

However, this PR adds/affects multiple features while it includes only one commit. It is difficult for us to discuss, organize, and merge it, with this PR structure.

Could you open one PR for each topic?

  • Skip the publishing process when there is no connection: looks nice for me.
  • Add CameraInfo publisher: also nice.
  • Modified publishing data type according to format: important, but more discussion needed.

Moreover, we are glad if you format your codes: Choreonoid recommends you to use the Camel case for all variables, use ++i instead of i++ for counters, avoid multiple operations on the same line, etc.

This was referenced Jun 7, 2023
@Hiroaki-Masuzawa
Copy link
Contributor Author

Thank you for your comment.

I will create 3 separate pull requests, but I created 2 (#12, #13) of them first because there are many conflicts between the pull requests.

I am not familiar enough with the coding standards, so I would appreciate your comments.

Hiroaki-Masuzawa pushed a commit to Hiroaki-Masuzawa/choreonoid_ros that referenced this pull request Oct 2, 2023
…table

PointCloudのROSメッセージのアイテム修正
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants