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

back to unicode in decode_config #45

Closed
wants to merge 1 commit into from

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Nov 13, 2014

I fixed the code to work like

client.update_configuration({"mstr_": u"いろは"} )
config = client.get_configuration(timeout=5)
self.assertEqual(u"いろは", config['mstr_'])

if you set reconfigure param in unicode, then you'll get them using unicode
see
https://github.com/ros/dynamic_reconfigure/pull/45/files#diff-326aeceb164c22eb7839ff7094d2f850L83
for test code

@esteve
Copy link
Member

esteve commented Nov 13, 2014

@k-okada Thanks for your pull request. I'm assuming this is to add support for Python 3 to your previous PR, would you mind adding a description to the pull request that explains what it does?

Thanks.

@esteve esteve mentioned this pull request Nov 13, 2014
@k-okada
Copy link
Contributor Author

k-okada commented Nov 13, 2014

No, this is not related Python3 support, this is for Python 2.7,
My previous PR behave as

client.update_configuration({"mstr_": u"いろは"} )
config = client.get_configuration(timeout=5)
self.assertEqual("いろは", config['mstr_'])

, which we can say Unicode In, Str Out.
and newer one behave as

client.update_configuration({"mstr_": u"いろは"} )
config = client.get_configuration(timeout=5)
self.assertEqual(u"いろは", config['mstr_'])

is Unicode In and Unicode Out

As far as I understand, current rospy convert any Unicode data to Str during serialization (see ros/genpy#31), so if you use echo like service call, you send unicode, and you'll get str like

a = s.call(u'ほげ')
a.str == 'ほげ' # Not u'ほげ'

however, parameter server preserves Unicode since it directly uses xmlrpc lib of Python. So I think behavior of dynamic_reconfigure should follow Unicode In Unicode Out style.

@esteve
Copy link
Member

esteve commented Nov 13, 2014

@k-okada I submitted a pull request #46 based on your code that addresses a few issues:

  • it uses sys.version_info to extract the version of the Python interpreter
  • bypasses decoding if the input string is already in unicode
  • adds more tests to make sure strings are returned as unicode

could you have a look at it and make sure it works for you? Thanks.

@k-okada
Copy link
Contributor Author

k-okada commented Nov 14, 2014

it seems ok for me, as for Python3 it working well, see https://travis-ci.org/k-okada/dynamic_reconfigure/builds/40958513

@esteve
Copy link
Member

esteve commented Nov 14, 2014

@k-okada I'm closing this PR and merge #46, thanks for testing it.

@esteve esteve closed this Nov 14, 2014
@k-okada k-okada deleted the fix_for_unicode_decode branch March 22, 2017 01:43
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