search

jsk-ros-pkg / geneus

3 stars 16 forks source link

[geneus] treat uint8[] as string like rospy #15

Closed furushchev closed 9 years ago

furushchev commented 9 years ago

This is claimed at https://github.com/jsk-ros-pkg/geneus/issues/14 Until merging this change, existing codes that treats messages containing uint8[] will fail.(e.g.: converting sensor_msgs/PointCloud2 to eus-pointcloud)

garaemon commented 9 years ago

Does it mean we need to modify existing code?

garaemon commented 9 years ago

https://github.com/jsk-ros-pkg/jsk_demos/blob/master/jsk_2015_06_hrp_drc/drc_com_common/msg/FC2OCSBasicInfoSmall.msg

k-okada commented 9 years ago

it depends on what kind of code you write, where is the code that uses FC2OCSBasicInfoSmall.msg? Before i rewrite geneus, uint8[] is convert to string, and at here https://github.com/jsk-ros-pkg/geneus/commit/b157cd5fb0abfd4a0c422fa9b78e5d379cc0ef2f, I have change them tointeger-vector but that may not correct. at least it fails at https://github.com/jsk-ros-pkg/jsk_roseus/blob/master/roseus/roseus_c_util.c#L50

garaemon commented 9 years ago

Hmm, I'm treating elements of uint8[] as integer.

This is another example:

garaemon commented 9 years ago

(elt <string> <index>) returns character constant and it may be no problem.

k-okada commented 9 years ago

humm, if we convert uint to integer-vector

garaemon commented 9 years ago

Could you test roslaunch jsk_network_tools joint_state_compress_sample.launch Y_DIFF=1.0 before merging this?

If it work, I'm willing to accept this change.

garaemon commented 9 years ago

https://github.com/jsk-ros-pkg/jsk_common/pull/852

This pull request may be required to run that.

k-okada commented 9 years ago

ok, please check this @furushchev

furushchev commented 9 years ago

I tried to test joint_state_compress_sample.launch, and looked rviz, but the poses of robots("no TF prefix" and with "decompressed") are quite different each other.(I use PR2 on local) Is this correct? If correct, how can I test?

furushchev commented 9 years ago

Poses are different both on normal geneus environment and "this pullreq" geneus environment.

garaemon commented 9 years ago

It should be almost same

2015年4月9日木曜日、Furushchevnotifications@github.comさんは書きました:

Poses are different both on normal geneus environment and "this pullreq" geneus environment.

— Reply to this email directly or view it on GitHub https://github.com/jsk-ros-pkg/geneus/pull/15#issuecomment-91231111.

✉︎ from iPhone

furushchev commented 9 years ago

@garaemon I added test for angle vector compress/decompress https://github.com/jsk-ros-pkg/jsk_common/pull/856. I'll now check if occurs any problem using this test. ;; I could not check the diff of this pullreq using RViz, because poses are already different both on normal/"new" geneus environment.

furushchev commented 9 years ago

I checked error in both env:

in existing geneus env:

head_tilt_joint: 0.000615
head_pan_joint: -0.003425
r_wrist_roll_joint: 0.002261
r_wrist_flex_joint: 0.005686
r_forearm_roll_joint: -0.003865
r_elbow_flex_joint: 6.28319
r_upper_arm_roll_joint: 0.003725
r_shoulder_lift_joint: 6.28319
r_shoulder_pan_joint: 0.003425
l_wrist_roll_joint: 0.002261
l_wrist_flex_joint: -0.005686
l_forearm_roll_joint: -0.003865
l_elbow_flex_joint: 6.28319
l_upper_arm_roll_joint: 0.003725
l_shoulder_lift_joint: 6.28319
l_shoulder_pan_joint: 0.011204
torso_lift_joint: 0.001526
(norm diff): 12.5664
head_tilt_joint: -0.13585
head_pan_joint: -0.564602
r_wrist_roll_joint: -0.3536
r_wrist_flex_joint: -0.65
r_forearm_roll_joint: 0.003865
r_elbow_flex_joint: 1.30592
r_upper_arm_roll_joint: -0.540196
r_shoulder_lift_joint: 0.936318
r_shoulder_pan_joint: -0.790692
l_wrist_roll_joint: 1.2963
l_wrist_flex_joint: -1.54137
l_forearm_roll_joint: 0.98565
l_elbow_flex_joint: 4.97727
l_upper_arm_roll_joint: -0.540196
l_shoulder_lift_joint: 5.34687
l_shoulder_pan_joint: -0.011204
torso_lift_joint: 1.29626
(norm diff): 8.04649

the difference of joint_states are bigger in existing environment than new environment.

furushchev commented 9 years ago

I got it, lift_joint are rotational joint? (2 * pi different?) Anyway there is the difference among enviromnents... What should I do...

garaemon commented 9 years ago

lift_joint problem should be resolved already. https://github.com/jsk-ros-pkg/jsk_common/blob/master/jsk_network_tools/euslisp/angle-vector-compress.l#L9

garaemon commented 9 years ago

I will confirm this patch and please wait to merge this

2015年4月11日土曜日、Furushchevnotifications@github.comさんは書きました:

I got it, lift_joint are rotational joint? (2 * pi different?) Anyway there is the difference among enviromnents... What should I do...

— Reply to this email directly or view it on GitHub https://github.com/jsk-ros-pkg/geneus/pull/15#issuecomment-91831672.

✉︎ from iPhone

furushchev commented 9 years ago

The problem is two:

I fixed and updated new geneus pullreq and now I think it's ok (all geneus test passed and joint_state_compressor returns the same values as on existing geneus environment.)

furushchev commented 9 years ago

NOTE: Still please do not merge before the test ( https://github.com/jsk-ros-pkg/jsk_roseus/pull/269 ) passes.

furushchev commented 9 years ago

Sorry I mistook comment. At first please merge this for testing message.l generated by new geneus:

After merging this PR, please re-run tests above to check