Update PrimitiveFieldType$STRING.java for UTF-8 #19

Theosakamg · 2014-02-17T00:22:19Z

Add support of UTF-8!
For ~i18n (eg. TTS).

Docs

"A string must always contain UTF-8 encoded or 7-bit ASCII text."
"unicode strings are currently not supported as a ROS data type. utf-8 should be used to be compatible with ROS string serialization. "

Quote

It works.

testing with python node.
for bug test, send string msg with "éè", on receive no Multibyte-char managed.
check for best implement, just a fix!

Add support of UTF-8! For i18n (eg. TTS). "A string must always contain UTF-8 encoded or 7-bit ASCII text." https://developers.google.com/protocol-buffers/docs/proto#scalar "unicode strings are currently not supported as a ROS data type. utf-8 should be used to be compatible with ROS string serialization. " http://wiki.ros.org/msg but it works. (testing with python)

stonier · 2014-02-18T07:05:32Z

@damonkohler will this have any unforeseen side effects?

damonkohler · 2014-02-18T07:09:12Z

LGTM but please also fix the serialize method.

damonkohler · 2014-02-18T07:12:45Z

message_generation/src/main/java/org/ros/internal/message/field/PrimitiveFieldType.java

@@ -581,7 +581,7 @@ public int getSerializedSize() {
    public String deserialize(ChannelBuffer buffer) {
      int length = buffer.readInt();
      ByteBuffer stringBuffer = buffer.readSlice(length).toByteBuffer();
-      return Charset.forName("US-ASCII").decode(stringBuffer).toString();
+      return Charset.forName("UTF-8").decode(stringBuffer).toString();


You also need to specify "UTF8" to getBytes() on line 574. See http://docs.oracle.com/javase/tutorial/i18n/text/string.html

my apologies...
I have not tested the serializer in every context.
I use ROS on Android, which by default is UTF-8

add symmetric encoding transformation (serializer/deserializer)

Code Refactoring

Remove dead code

damonkohler · 2014-02-19T11:10:05Z

message_generation/src/main/java/org/ros/internal/message/field/PrimitiveFieldType.java

@@ -679,6 +680,7 @@ public String getJavaTypeName() {
  };

  private static final ImmutableSet<String> TYPE_NAMES;
+  private static final Charset DEFAULT_CHARSET = Charset.forName("utf-8");


Please use "UTF-8"

damonkohler · 2014-02-19T11:10:23Z

Last nit and then LG.

Change parameter of constant

Theosakamg · 2014-02-19T13:43:39Z

@stonier : I testing the change with common case. Do you have other ideas for test cases ?

in org.ros.internal.message.MessageSerializationTest

  @Test
  public void testStringUTF8() {
    std_msgs.String message = defaultMessageFactory.newFromType(std_msgs.String._TYPE);
    message.setData("éêè €àáßëœ 文字化け");
    checkSerializeAndDeserialize(message);

    // i18n test case
    // base on http://www.inter-locale.com/whitepaper/learn/learn-to-test.html

    // Combining Marks and Accents test
    message.setData("àéîōũ");
    checkSerializeAndDeserialize(message);

    // DOS 860 test
    message.setData("você nós mãe avô irmã criança");
    checkSerializeAndDeserialize(message);

    // Windows-1252 test
    message.setData("€ŒœŠš™©‰ƒ");
    checkSerializeAndDeserialize(message);

    // Turkish test
    message.setData("ışık bir İyi Günler");
    checkSerializeAndDeserialize(message);

    // Dakuten and handakuten marks test
    message.setData("がざばだぱか゛さ゛た゛は");
    checkSerializeAndDeserialize(message);

    // Combining Grapheme Joiner character
    message.setData("אִ͏ַ");
    checkSerializeAndDeserialize(message);

    // Bidi with Latin test
    message.setData("abcאבגדabc ");
    checkSerializeAndDeserialize(message);

    message.setData("אבגדabcאבגד");
    checkSerializeAndDeserialize(message);

    message.setData("אבגד012אבגד");
    checkSerializeAndDeserialize(message);

    message.setData("אבגד 012 012");
    checkSerializeAndDeserialize(message);

    // Complex Scripts test
    message.setData("สวัสดี");
    checkSerializeAndDeserialize(message);

    message.setData("டாஹ்கோ");
    checkSerializeAndDeserialize(message);

    message.setData("بِسْمِ اللّهِ الرَّحْمـَنِ الرَّحِيمِ");
    checkSerializeAndDeserialize(message);

    // Numeric Shaping test
    message.setData("عدد مارس ١٩٩٨");
    checkSerializeAndDeserialize(message);

    // Common Scripts and Encodings test
    message.setData("Слава Жанна Ювеналий Ярополк");
    checkSerializeAndDeserialize(message);
  }

damonkohler · 2014-02-19T14:21:41Z

Can you please add those tests to this change?

for [rosjava_bootstrap](rosjava/rosjava_bootstrap#19)

Theosakamg · 2014-02-19T18:32:35Z

I am not sure of the location. (I use the groovy branch.)

damonkohler · 2014-02-26T08:04:23Z

Sorry, I don't understand the problem. Can you please add the message serialization tests you pasted above to this patch?

Add the message serialization tests for UTF-8.

Theosakamg · 2014-02-27T18:41:45Z

The problem was : I use groovy stack and the test is available in https://github.com/rosjava/rosjava_core/blob/master/rosjava/src/test/java/org/ros/internal/message/MessageSerializationTest.java (always available in master)
But with the news stack is in https://github.com/rosjava/rosjava_bootstrap/blob/master/message_generation/src/test/java/org/ros/internal/message/RawMessageSerializationTest.java
(If I understand)

It's done in rosjava_core and rosjava_bootstrap repository. It's ok for you?

Raw format style.

Theosakamg · 2014-02-27T18:55:56Z

Apologies I just see the difference (Raw vs Std_msg)

damonkohler · 2014-02-28T08:52:27Z

message_generation/src/main/java/org/ros/internal/message/field/PrimitiveFieldType.java

@@ -679,6 +680,7 @@ public String getJavaTypeName() {
  };

  private static final ImmutableSet<String> TYPE_NAMES;
+  private static final Charset DEFAULT_CHARSET = Charset.forName("UTF-8");


Please move this to the top of the class (line 34).

damonkohler · 2014-02-28T08:53:05Z

Looks good, thanks! Just one last nit. The constant DEFAULT_CHARSET belongs at the top of the class.

Clean code

Theosakamg · 2014-02-28T13:19:58Z

Done.

Update PrimitiveFieldType$STRING.java for UTF-8

stonier · 2014-03-16T12:53:49Z

@mh0rst is this the error you were also seeing?

http://pastebin.com/LV31s8g0

This was coming from a ros answers post: http://answers.ros.org/question/140371/compilation-problem-in-rosjava-master-branch/

mh0rst · 2014-03-16T21:58:06Z

@stonier yes, exactly. The fix is trivial, I created a pull request if that helps.

stonier · 2014-03-19T06:53:34Z

Definitely. @damonkohler will come undoubtedly across it when he's free.

damonkohler reviewed Feb 18, 2014
View reviewed changes

Theosakamg added 3 commits February 19, 2014 00:42

Update PrimitiveFieldType.java

20d010f

add symmetric encoding transformation (serializer/deserializer)

Update PrimitiveFieldType.java

ef56ba6

Code Refactoring

Update PrimitiveFieldType.java

161226d

Remove dead code

damonkohler reviewed Feb 19, 2014
View reviewed changes

Update PrimitiveFieldType.java

ae94c48

Change parameter of constant

Theosakamg added a commit to Theosakamg-Archive/rosjava_core that referenced this pull request Feb 19, 2014

Update Test for PrimitiveFieldType$STRING for UTF-8

cde1e61

for [rosjava_bootstrap](rosjava/rosjava_bootstrap#19)

Theosakamg mentioned this pull request Feb 19, 2014

Update Test for PrimitiveFieldType$STRING for UTF-8 rosjava/rosjava_core#194

Closed

Update RawMessageSerializationTest.java

ccc8acc

Add the message serialization tests for UTF-8.

Update RawMessageSerializationTest.java

45bfaac

Raw format style.

damonkohler reviewed Feb 28, 2014
View reviewed changes

Update PrimitiveFieldType.java

f39e6fd

Clean code

damonkohler added a commit that referenced this pull request Feb 28, 2014

Merge pull request #19 from Theosakamg/master

61079f6

Update PrimitiveFieldType$STRING.java for UTF-8

damonkohler merged commit 61079f6 into rosjava:master Feb 28, 2014

mh0rst mentioned this pull request Mar 16, 2014

Fix compile error in master #23

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update PrimitiveFieldType$STRING.java for UTF-8 #19

Update PrimitiveFieldType$STRING.java for UTF-8 #19

Theosakamg commented Feb 17, 2014

stonier commented Feb 18, 2014

damonkohler commented Feb 18, 2014

damonkohler Feb 18, 2014

Theosakamg Feb 19, 2014

damonkohler Feb 19, 2014

damonkohler commented Feb 19, 2014

Theosakamg commented Feb 19, 2014

damonkohler commented Feb 19, 2014

Theosakamg commented Feb 19, 2014

damonkohler commented Feb 26, 2014

Theosakamg commented Feb 27, 2014

Theosakamg commented Feb 27, 2014

damonkohler Feb 28, 2014

damonkohler commented Feb 28, 2014

Theosakamg commented Feb 28, 2014

stonier commented Mar 16, 2014

mh0rst commented Mar 16, 2014

stonier commented Mar 19, 2014

Update PrimitiveFieldType$STRING.java for UTF-8 #19

Update PrimitiveFieldType$STRING.java for UTF-8 #19

Conversation

Theosakamg commented Feb 17, 2014

Docs

Quote

stonier commented Feb 18, 2014

damonkohler commented Feb 18, 2014

damonkohler Feb 18, 2014

Choose a reason for hiding this comment

Theosakamg Feb 19, 2014

Choose a reason for hiding this comment

damonkohler Feb 19, 2014

Choose a reason for hiding this comment

damonkohler commented Feb 19, 2014

Theosakamg commented Feb 19, 2014

damonkohler commented Feb 19, 2014

Theosakamg commented Feb 19, 2014

damonkohler commented Feb 26, 2014

Theosakamg commented Feb 27, 2014

Theosakamg commented Feb 27, 2014

damonkohler Feb 28, 2014

Choose a reason for hiding this comment

damonkohler commented Feb 28, 2014

Theosakamg commented Feb 28, 2014

stonier commented Mar 16, 2014

mh0rst commented Mar 16, 2014

stonier commented Mar 19, 2014