UnicodeDecodeError when parsing strings containing Japanese characters

[ros-visualization]

UnicodeDecodeError when parsing strings containing Japanese characters

[MartinPeris]

On a second thought, this seems to be a minor issue that only appears with deprecated versions of Python and no one had complained about this before.

Maybe instead of tainting the code with type checks or version checks that might introduce bigger headaches down the road, the best solution might be to do nothing.

New versions ROS don't have this issue and if anyone using legacy versions encounters this crash, they can find the solution in this PR.

Shall we just close this?

[arne48]

Hello @MartinPeris,
thank you very much for creating this issue.
From your error message I can infer that you use Kinetic Kame.
Unfortunately, Kinetic is not supported anymore since April 2021.
However, it was quite a popular release and I would like to make a fix for this Unicode issue available.
But all newer ROS distributions are using python3 and therefore Unicode by default.
Because of this I wouldn't like introduce the (then in many cases) unnecessary ".decode(str)" into the main branch.

I will check a few things on my test machines and then come back to you to talk how we could implement this maybe a little bit precise.

However, thank you very much for your engagement so that we can fix this problem.

Best,
Arne

[MartinPeris]

Hi @arne48, thanks a lot for your feedback!

You are right, I am still using Kinetic Kame. I checked and it also happens in Melodic. Trying to check with ROS2 Foxy

[arne48]

Hi @MartinPeris,
I tried it with my personal melodic installation so far, and that one didn't crash and showed もしもし correctly.
Noetic is the first release that was supposed to use python3 exclusively.
I didn't had time yet to do all the testing though.
But please keep me updated about your results with Foxy.

Nevertheless, once we figured out an appropriate solution, I would issue updates for all distributions >=Kinetic.

[MartinPeris]

Did you try double clicking the message? The issue is not when displaying the row, but when hovering the mouse or double clicking on it

[arne48]

Thanks, now it is also confirmed for Melodic and thankfully therefore now consistent with my expectation.

[MartinPeris]

I tried with ROS2 Foxy and the crash does not happen. The Japanese characters appear as "squares", but that is because I am running ROS2 Foxy + Ubuntu Focal on a Docker container and my locale is not properly set.

This patch does not apply to ROS distros with Python 3

What would you propose to do?

[arne48]

Yes so I wouldn't like to apply the patch to the ROS2 branch as those are Python 3 exclusive anyway.
About how I would like to implement the check I am still a bit undecided if checking for type or checking for the python version is the prettier solution.

But from the view of the logic behind the check, I would somehow check for the python version to determine if ".decode(str)" should be used or not.

[arne48]

I though about this a bit more and couldn't find a satisfying way to not clutter the source code for recent versions that are running Python 3.

Therefore, I decided to branch-off for these older versions with your fix and leave the main branch as is.
I created the "melodic-devel" branch and would like to ask you to re-issue your PR against that branch.
Because Python 3 could be theoretically used, please also add the python version check to decide if ".decode()" is needed.
And also please add a Comment explaining this change again within the source.

When that is done, I will change the release sources of melodic and kinetic (maybe lunar) to the new "melodic-devel" branch.
So with having these two main branches for ROS1 I think we made a sensible decision into reduced clutter, while still fixing it and providing a patch.

Until then it was really good that @MartinPeris brought this up here, so that others who have the same problem can find a workaround for themselves.

Thanks @MartinPeris

[MartinPeris]

My pleasure @arne48

I have updated the associated PR to satisfy your requests. Please let me know if there is anything else I could do.

[arne48]

Thank you @MartinPeris
Now I could merge it and I think we could successfully close this issue.