start-jsk / rtmros_common

OpenRTM - ROS interoperability packages
http://wiki.ros.org/rtmros_common
12 stars 52 forks source link

Extract common code from hironx and nextage to HrpsysConfigurator #289

Closed k-okada closed 10 years ago

k-okada commented 10 years ago

From gm130s on December 28, 2013 10:04:42

Discussed in https://github.com/tork-a/rtmros_nextage/issues/7

Original issue: http://code.google.com/p/rtm-ros-robotics/issues/detail?id=291

k-okada commented 10 years ago

From gm130s on December 27, 2013 17:06:03

I have work-in-progress copy where common client functionalities gathered in hrpsys_ros_bridge. Once I'll send patches that traverse multiple packages and repositories.

k-okada commented 10 years ago

From gm130s on January 20, 2014 21:20:10

Stuck at the same issue discussed in issue 145 . http://code.google.com/p/hrpsys-base/issues/detail?id=145#c10

Status: Started

k-okada commented 10 years ago

From gm130s on January 28, 2014 23:52:03

Continued from http://code.google.com/p/rtm-ros-robotics/issues/detail?id=305#c1 where I posted my initial patch candidate.

By kei.okada

2) https://github.com/130s/rtmros_common/blob/4903e2848353b594c89b439ccf99cfa1c4589ad0/hrpsys_ros_bridge/src/hrpsys_ros_bridge/rtmros_robot_client.py is not good idea since this includes hiroinx specific codes as Groups, HandGroups, OffPose, InitialPose variabeles, these codes should be located within hironx_ros_bridge package.

I got it.

Do you mean only variables are robot specific? Are there any methods that should also stay in hironx? Below I've taken out methods, and variables that you didn't mention. I appreciate if you could cross out what are not appropriate for hrpsys_ros_bridge.

    sc = None
    sc_svc = None

    def getRTCList(self):
    def setSelfGroups(self):
    def getActualState(self):
    def isCalibDone(self):
    def isServoOn(self, jname='any'):
    def liftRobotUp(self):
    def stOff(self):
    def flat2Groups(self, flatList):
    def servoOn(self, jname='all', destroy=1, tm=3):
    def servoOff(self, jname='all', wait=True):
    def checkEncoders(self, jname='all', option=''):
k-okada commented 10 years ago

From kei.ok...@gmail.com on January 29, 2014 00:00:32

Oh, these methods should go to hrpsys_config.py, except sc, sc_svc. That's good point.

k-okada commented 10 years ago

From gm130s on January 29, 2014 00:21:08

Then I'll write a patch to hrpsys-base.

k-okada commented 10 years ago

From kei.ok...@gmail.com on January 29, 2014 06:15:56

Find attachment for the draft patch, because I found several issue that may confuse people. Patch is not tested on real robot at all, so be careful when you apply.

1) there are seveal lines that desgined for humanoid robots exits. we can remove this at this moment. stOff -> stabilizer off, liftRobotUp -> humanoid robot must in the air when servo off

2) remove gripper related code that would not general (ServoControler assume specific type of servo modules)

3) I'm still believe that servo_off/servo_on should work without removing joint groups...; https://github.com/130s/rtmros_common/blob/4903e2848353b594c89b439ccf99cfa1c4589ad0/hrpsys_ros_bridge/src/hrpsys_ros_bridge/rtmros_robot_client.py this code still call SWITCH_ON before goActual().

4) remove several lines from servoOn and servoOff which I do not understand.

A)I could not find the reason why 'Move to Actual State, Just a minute.' line is requried., because we call goActual and SWITCH_ON, so that the servo modules is already output "Actual State"

B)'if the servos aren't on switch power off' line is also unclear to me, it seems we do not need this code because of we do same thing after "Robot Motion Warning (Servo OFF)"

Attachment: update.hrpsys_config.py.patch

k-okada commented 10 years ago

From gm130s on February 05, 2014 19:17:34

First off, I added HrpsysConfigurator to the title now that we found that there are methods that we can promote to hrpsys level, not just in hironx-nextage level. I now think there are 2 levels of abstraction:

Level-1. Promote to hrpsys Level-2. Consolidate common methods and variables to a common class (that does not yet exist) of hiro-nextage robot.

So in this ticket we talk about L-1 abstraction aside from L-2.

+1 for the patch from kei.okada@gmail.com. Responding,

1) there are seveal lines that desgined for humanoid robots exits. we can remove this at this moment. stOff -> stabilizer off, liftRobotUp -> humanoid robot must in the air when servo off

2) remove gripper related code that would not general (ServoControler assume specific type of servo modules)

Agreed.

3) I'm still believe that servo_off/servo_on should work without removing joint groups...; https://github.com/130s/rtmros_common/blob/4903e2848353b594c89b439ccf99cfa1c4589ad0/hrpsys_ros_bridge/src/hrpsys_ros_bridge/rtmros_robot_client.py this code still call SWITCH_ON before goActual().

4) remove several lines from servoOn and servoOff which I do not understand. A)I could not find the reason why 'Move to Actual State, Just a minute.' line is requried., because we call goActual and SWITCH_ON, so that the servo modules is already output "Actual State"

B)'if the servos aren't on switch power off' line is also unclear to me, it seems we do not need this code because of we do same thing after "Robot Motion Warning (Servo OFF)"

Sorry,,,the link above, which is my patch, is based on an older hironx_client.py that didn't contain the recent patch about servoOn/Off. That said please ignore servo related parts of my patch.

Summary: Extract common code from hironx and nextage to HrpsysConfigurator (was: Extract common code from hironx and nextage)
Cc: emijah.s

130s commented 10 years ago

On real robot I will test the patch suggested in the previous comment with my comment applied and report back within a week

k-okada commented 10 years ago

commit current version to -> https://code.google.com/p/hrpsys-base/source/detail?r=1013, https://code.google.com/p/hrpsys-base/source/detail?r=1010.

*\ I removed removeJointGroups from servo off / on, for compatibility for other robots. so be careful when you remove moved codes from hironx_client.py.