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

[ros_introspection] Use str instead of unicode in replace_contents #91

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

IamPhytan
Copy link

ROS Distro : melodic, noetic

I had the following error when running magical_ros2_conversion_tool :

'ascii' codec can't decode byte 0xe4

Part of it was maybe caused by the fact that the package I am trying to convert has comments with characters that are not part of ASCII. This error is combined with a message from pylint:

NameError: name 'unicode' is not defined

My solution is to replace unicode with str. Once replaced, the package ws then able to run without any problems.

@DLu
Copy link
Owner

DLu commented Nov 14, 2022

Would you mind sending the source file (or a placeholder source file with the nonASCII characters)?

@IamPhytan
Copy link
Author

Here is a repo on which I tried to use magical_ros2_conversion_tool: https://github.com/norlab-ulaval/ros_rslidar/

@DLu
Copy link
Owner

DLu commented Jan 23, 2023

Did some diggiging, and this looks like it was fixed in #83 here: https://github.com/DLu/roscompile/pull/83/files#diff-c3801915f37d209e220776e077539bf711bb6d734402af8ca2a4f57bf96ee4f1

Were you running from the binary?

@IamPhytan
Copy link
Author

IamPhytan commented Jan 23, 2023

No, I didn't run the binary, if there was any. I ran the tool from source, in a catkin workspace

As for the fix, it looks like the following line is not catching UnicodeDecodeErrors. I will fix it in a new commit :

https://github.com/DLu/roscompile/blob/main/ros_introspection/src/ros_introspection/source_code_file.py#L46

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