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

Replace RuntimeError with new custom exception RCLError #478

Merged
merged 23 commits into from
Dec 10, 2019

Conversation

suab321321
Copy link
Contributor

@suab321321 suab321321 commented Dec 6, 2019

Fixes #31

Replacing RuntimeError with new custom exception RCLError.

@jacobperron
Copy link
Member

@suab321321 Thanks for the patch! It looks like there was some git branching issues, so I've updated your branch customexception to just contain the one commit I think you wanted.

Also, for the future, it's good to populate the description and title with useful information (for example, a reference to the ticket this is fixing). I have updated the description for you this time.

@jacobperron jacobperron changed the title Customexception Replace RuntimeError with new custom exception RCLError Dec 6, 2019
@suab321321
Copy link
Contributor Author

@jacobperron thank you sir.. I will keep this in mind..will this be merged now sir?

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

There are some places where I don't think using RCLError is right, since the error is not caused by rcl.

rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
@jacobperron jacobperron self-assigned this Dec 6, 2019
@suab321321
Copy link
Contributor Author

@jacobperron sir I m just comitting the suggested changes from here.

rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Couple more things. And after addressing @allenh1's comments, I'll trigger CI.

rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
@suab321321
Copy link
Contributor Author

@jacobperron done sir.....

rclpy/src/rclpy/_rclpy.c Outdated Show resolved Hide resolved
suab321321 and others added 16 commits December 10, 2019 11:12
Signed-off-by: AbhinavSingh <singhabhinav9051571833@gmail.com>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
suab321321 and others added 7 commits December 10, 2019 11:12
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Hunter L. Allen <hunterlallen@protonmail.com>
Co-Authored-By: Hunter L. Allen <hunterlallen@protonmail.com>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Co-Authored-By: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I've rebased these changes on master to get the latest fixes there for CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron jacobperron merged commit 83dd5c3 into ros2:master Dec 10, 2019
@jacobperron
Copy link
Member

@suab321321 Thanks again for the improvement!

@suab321321
Copy link
Contributor Author

@allenh1 @jacobperron thank you for your guiding me sir.

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.

Create custom exceptions in _rclpy.c
3 participants