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

[jsk_data] add YAML topic savetype #1476

Merged
merged 2 commits into from
Nov 7, 2016
Merged

Conversation

knorth55
Copy link
Member

@knorth55 knorth55 commented Nov 4, 2016

  • support no-header message

  • add YAML topic savetype

@@ -119,6 +127,9 @@ def service_cb(self, req):
bridge = cv_bridge.CvBridge()
label = bridge.imgmsg_to_cv2(msg)
cv2.imwrite(osp.join(save_dir, topic['fname']), label)
elif topic['savetype'] == 'Text':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use YAML save type?

In [1]: from std_msgs.msg import String

In [2]: msg = String(data='spam')

In [3]: import genpy

In [4]: genpy.message.strify_message(msg)
Out[4]: 'data: spam'

In [5]: from sensor_msgs.msg import FluidPressure

In [6]: msg = FluidPressure(fluid_pressure=1080, variance=0)

In [7]: msg.header.frame_id = 'right_vacuum_gripper'

In [8]: genpy.message.strify_message(msg)
Out[8]: 'header: \n  seq: 0\n  stamp: \n    secs: 0\n    nsecs:         0\n  frame_id: right_vacuum_gripper\nfluid_pressure: 1080\nvariance: 0'

In [9]: print(genpy.message.strify_message(msg))
header:
  seq: 0
  stamp:
    secs: 0
    nsecs:         0
  frame_id: right_vacuum_gripper
fluid_pressure: 1080
variance: 0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that looks better

self.msg[topic_name] = msg
else:
self.msg[topic_name] = msg
self.msg_received_time[topic_name] = rospy.Time.now()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use below?

self.msg[topic_name] = {
  'stamp': msg.header.stamp if msg._has_header else rospy.Time.now(),
  'msg': msg,
}

@knorth55 knorth55 changed the title [jsk_data] add std_msgs support in data_collection_server [jsk_data] add YAML topic savetype Nov 4, 2016
@knorth55
Copy link
Member Author

knorth55 commented Nov 4, 2016

thx, fixed

@knorth55
Copy link
Member Author

knorth55 commented Nov 4, 2016

PR is tottaly modified to save msg in YAML file, and merged #1475 PR

@k-okada k-okada merged commit 1b82ecb into jsk-ros-pkg:master Nov 7, 2016
@knorth55 knorth55 deleted the add-std-msg branch November 7, 2016 12:42
@@ -86,7 +87,10 @@ def __del__(self):
sub.unregister()

def sub_cb(self, msg, topic_name):
self.msg[topic_name] = msg
self.msg[topic_name] = {
'stamp': msg.header.stamp if msg._has_header else rospy.Time.now(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many indentations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no lint error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no lint error

Copy link
Member

@wkentaro wkentaro Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any reasons you use 8 spaces? why not 4 spaces?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no reason for 8 spaces, but no reason for 4 spaces because there is no lint error.
i read python style code, and it says continuous line indent is optional rule, which i didn't know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think linter is perfect, and I think it needs human help.
Do you think it is ok if you add 100 spaces over there?
There is no linter error, but it should be fixed, isn't it?

Copy link
Member Author

@knorth55 knorth55 Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i fixed indent in #1480

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.

None yet

3 participants