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

[fix][Python] link name's default value for get* methods. #1118

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

130s
Copy link
Contributor

@130s 130s commented Mar 16, 2017

Because get{Current, Reference}{Pose, Position, Rotation, RPY} methods handle the case where lname passed is None in their method's logic, those methods should also allow the case where lname is missing.

Also, this PR extracts common portion of procedure when lname was None for better maintenance.

@130s 130s force-pushed the py/fix/getcurrentposi_alwaysexceptionraised branch 2 times, most recently from 775dff3 to dc5dc55 Compare March 16, 2017 20:25
@k-okada
Copy link
Contributor

k-okada commented Mar 16, 2017

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2823/
Test PASSed.

@k-okada
Copy link
Contributor

k-okada commented Mar 16, 2017

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2824/
Test PASSed.

@130s 130s force-pushed the py/fix/getcurrentposi_alwaysexceptionraised branch from dc5dc55 to fc022e8 Compare March 16, 2017 22:03
@k-okada
Copy link
Contributor

k-okada commented Mar 16, 2017

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2825/
Test PASSed.

@k-okada
Copy link
Contributor

k-okada commented Mar 16, 2017

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2826/
Test PASSed.

@k-okada
Copy link
Contributor

k-okada commented Mar 17, 2017

Did you confirmed if this works with old hrpsys ~= 315.1.x, 315.2.x,
These case should be chanced on Travis but I am not confident on testing all cases. I think better to check manually

@130s
Copy link
Contributor Author

130s commented Mar 17, 2017

start-jsk/rtmros_hironx#487 (comment) に書いたように下流 Hironx の暫定クラスへのパッチが 315_1_10 のテストが通っていたので大丈夫ではないかということで同 PR は merge 済です.

がこの PR の Travis が失敗してますね…

@130s 130s closed this Mar 21, 2017
@130s 130s reopened this Mar 21, 2017
@130s
Copy link
Contributor Author

130s commented Mar 21, 2017

Travis が失敗してますね…

2回ほど Travis 上の build を見ていますが,どちらも同じ1個の job だけ失敗しているようです."
TEST_TYPE=work_with_315_1_10 TEST_PACKAGE=hrpsys-ros-bridge"

https://s3.amazonaws.com/archive.travis-ci.org/jobs/213581324/log.txt

+++cat /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-hztest_tf.xml /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-hztest_tf.xml /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-hztest_tf.xml /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-hztest_tf.xml /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-hztest_tf.xml /home/travis/.ros/test_results/hrpsys_ros_bridge/rosunit-samplerobot.xml
<?xml version="1.0" encoding="utf-8"?>
<testsuite errors="0" failures="1" name="unittest.suite.TestSuite" tests="1" time="5.531">
  <testcase classname="__main__.HzTest" name="test_hz" time="5.5305">
    <failure type="AssertionError">average rate (236.127Hz) exceeded minimum (300.000Hz)
  File "/usr/lib/python2.7/unittest/case.py", line 327, in run
    testMethod()
  File "/opt/ros/hydro/share/rostest/nodes/hztest/hztest", line 115, in test_hz
    self._test_hz(hz, hzerror, topic, test_duration, wait_time)
  File "/opt/ros/hydro/share/rostest/nodes/hztest/hztest", line 185, in _test_hz
    (rate, self.min_rate))
  File "/usr/lib/python2.7/unittest/case.py", line 420, in assertTrue
    raise self.failureException(msg)
    </failure>
  </testcase>
  <system-out><![CDATA[Hz: 500.0
Hz Error: 200.0
Topic: /joint_states
Test Duration: 5.0
Waiting for messages
Starting rate measurement
Done waiting, validating results
]]></system-out>
  <system-err><![CDATA[]]></system-err>
</testsuite>

エラーログ見ても原因が分かりません.

(14番のテストがおとおりずらくないか? と同じか?と思ったものの,今回しっぱしているのは15番のようなので微妙に違うか.)

130s added 2 commits May 9, 2017 08:22
Because `get``{Current, Reference}``{Pose, Position, Rotation, RPY}` methods handle the case where `lname` passed is None in their method's logic, those methods should allow missing `lname`.
[Python] Delegate raising RuntimeError to a common method.
@130s 130s force-pushed the py/fix/getcurrentposi_alwaysexceptionraised branch from fc022e8 to 71024d3 Compare May 9, 2017 15:23
@k-okada
Copy link
Contributor

k-okada commented May 9, 2017

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2858/
Test PASSed.

@130s
Copy link
Contributor Author

130s commented May 10, 2017

Travis も通りました.

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