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

Add robot init function to create *ri* and robot instance #222

Merged
merged 4 commits into from
Apr 27, 2016

Conversation

snozawa
Copy link
Contributor

@snozawa snozawa commented Apr 26, 2016

Add robot init function to create *ri* and robot instance according to the following discussion:
#175

  • Behavior
    1. Without /robot/type and argument, or no interface file is found, robot instance and interface instance are not created and errors are invoked.
      (robot-init) ;; without ros param -> error.
    2. If argument are specified and robot interface file is found, robot instance and interface with given name are created.
      (robot-init "pr2") -> OK
    3. If no argument is specified, /robot/type ROSPARAM is set, and robot interface file is found, robot instance and interface with given name are created.
      (robot-init) ;; with /robot/type setting -> OK. Create *ri* and robot from /robot/type.
  • Typical usage:
     (robot-init) ;; With setting /robot/type ROSPARAM anywhere. 
     (robot-init (or (ros::get-param \"/robot/type\") \"pr2\")) ;; If /robot/type ROSPARAM is specified, use it. Otherwise, use \"pr2\" by default.
  • Configuring user-defined robot:
    This function searches robot interface file from rospack plugins.
    If user want to use their own robots from robot-init function,
    please write export tag in [user_defined_rospackage]/package.xml as pr2eus/package.xml.
  <export>
    <pr2eus robot-name="pr2" interface-file="${prefix}/pr2-interface.l" />
 </export>

I also add test code for it (robot-init-test.l).

@snozawa snozawa force-pushed the add_robot_init branch 2 times, most recently from bef9121 to 7d68792 Compare April 26, 2016 16:30
@snozawa
Copy link
Contributor Author

snozawa commented Apr 27, 2016

Travis passed thanks for #223.

@snozawa snozawa merged commit c1e6a27 into jsk-ros-pkg:master Apr 27, 2016
@snozawa snozawa deleted the add_robot_init branch April 27, 2016 00:07
@k-okada
Copy link
Member

k-okada commented Apr 27, 2016

I think you'd better to leave this PR open for a while, if you want promote this new way of initialize *ri* and *robot*, otherwise no one realize this changes...

@snozawa
Copy link
Contributor Author

snozawa commented Apr 27, 2016

I think you'd better to leave this PR open for a while, if you want promote this new way of initialize ri and robot, otherwise no one realize this changes...

This PR is discussed more than a half year,
#175
and I waited as your comments in almost one day.
But I'm very sorry for too short period to leave this PR open.

where piped-fork-returns-list defined ?

Defined in jskeus : https://github.com/euslisp/jskeus/blob/master/irteus/irtutil.l#L369.
This is the commit at sourforge age.
This performs

  • Close p-opened stream after using
  • Returns list
11.irteusgl$ ls -l /tmp/irtmodel-ik-14751/
合計 12
-rw-r--r-- 1 leus leus 1347 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-51-95-16-97-failure.l
-rw-r--r-- 1 leus leus 1351 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-51-99-51-47-failure.l
-rw-r--r-- 1 leus leus 1277 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-52-03-33-48-failure.l
0
13.irteusgl$ pprint (piped-fork-returns-list "ls -l /tmp/irtmodel-ik-14751/")
("合計 12"
 "-rw-r--r-- 1 leus leus 1347 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-51-95-16-97-failure.l"
 "-rw-r--r-- 1 leus leus 1351 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-51-99-51-47-failure.l"
 "-rw-r--r-- 1 leus leus 1277 Apr 27 09:33 hrp2jsknts-robot-2016-04-27-09-33-52-03-33-48-failure.l")
nil

is calling rospack from piped fork safe? (c.f. http://www.jsk.t.u-tokyo.ac.jp/redmine/public/issues/134)

Oh, is this resolved by ueda's commit??

For this PR, originally I did not use piped-fork and I used ros::rospack-plugins.
However, ros::rospack-plugins euslisp function is too new
jsk-ros-pkg/jsk_roseus@e538fdd.
So we could not use ros::rospack-plugins from ros-hydro-roseus because of ros hydro deb freeze.
Moreover, jsk_pr2eus's hydro travis test failed because of this reason.
Therefore, I reluctantly add piped-fork for robot-init.

do we have to write export tag for all robots? ened todo list?

Yes, if we want to initialize our own robot from robot-init,

  • Just call (robot-init "pepper")
  • Call (robot-init) with writing export tag for robot-defined rso package.

Currently, hrpsys_ros_bridge_tutorials and pr2eus have this export tag.

@k-okada
Copy link
Member

k-okada commented Apr 27, 2016

This PR is discussed more than a half year,

This PR is leave more than a half year, without any discussion. This means

  • no one feel this is required
  • no one noticed
  • every one agreed but too shy to say +1

@snozawa
Copy link
Contributor Author

snozawa commented Apr 27, 2016

  • no one feel this is required
  • no one noticed
  • every one agreed but too shy to say +1

Exactly, as your comment, all of these are correct.

The purpose of this PR is

In addition to your comments,

(robot-name-list
(if (fboundp 'ros::rospack-plugins)
(mapcar #'cdr (ros::rospack-plugins "pr2eus" "robot-name"))
(piped-fork-returns-list "rospack plugins --attrib=robot-name pr2eus | cut -d\\ -f2")))
Copy link
Member

Choose a reason for hiding this comment

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

roseus < 1.4.0 does not support ros::rospack-plugins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, so i used fboundp to check existence.

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