[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

rospy: TCPROS, header callerid optional field #522

Closed
akru opened this issue Nov 4, 2014 · 4 comments
Closed

rospy: TCPROS, header callerid optional field #522

akru opened this issue Nov 4, 2014 · 4 comments

Comments

@akru
Copy link
Contributor
akru commented Nov 4, 2014

According to http://wiki.ros.org/ROS/TCPROS:

  • callerid field is NOT required.

But when publisher response header does not contain this field, messages from publisher will not be received, because tcpros_base.py has crashed.

[rospy.client][INFO] 2014-11-04 15:44:37,097: init_node, name[/rostopic_28955_1415105077037], pid[28955]
[xmlrpc][INFO] 2014-11-04 15:44:37,098: XML-RPC server binding to 0.0.0.0:0
[xmlrpc][INFO] 2014-11-04 15:44:37,098: Started XML-RPC server [http://krakov:44930/]
[rospy.init][INFO] 2014-11-04 15:44:37,098: ROS Slave URI: [http://krakov:44930/]
[rospy.impl.masterslave][INFO] 2014-11-04 15:44:37,098: _ready: http://krakov:44930/
[rospy.registration][INFO] 2014-11-04 15:44:37,099: Registering with master node http://krakov:11311
[xmlrpc][INFO] 2014-11-04 15:44:37,099: xml rpc node: starting XML-RPC server
[rospy.init][INFO] 2014-11-04 15:44:37,199: registered with master
[rospy.rosout][INFO] 2014-11-04 15:44:37,199: initializing /rosout core topic
[rospy.rosout][INFO] 2014-11-04 15:44:37,201: connected to core topic /rosout
[rospy.simtime][INFO] 2014-11-04 15:44:37,203: /use_sim_time is not set, will not subscribe to simulated time [/clock] topic
[rospy.internal][INFO] 2014-11-04 15:44:37,265: topic[/chatter] adding connection to [http://krakov:39475], count 0
[rospy.internal][WARNING] 2014-11-04 15:44:37,268: Unknown error initiating TCP/IP socket to krakov:41717 (http://krakov:39475): Traceback (most recent call last):
  File "/opt/ros/indigo/lib/python2.7/site-packages/rospy/impl/tcpros_base.py", line 557, in connect
    self.read_header()
  File "/opt/ros/indigo/lib/python2.7/site-packages/rospy/impl/tcpros_base.py", line 618, in read_header
    self._validate_header(read_ros_handshake_header(sock, self.read_buff, self.protocol.buff_size))
  File "/opt/ros/indigo/lib/python2.7/site-packages/rospy/impl/tcpros_base.py", line 585, in _validate_header
    self.callerid_pub = header['callerid']
KeyError: 'callerid'

[rospy.internal][INFO] 2014-11-04 15:44:37,269: topic[/chatter] removing connection to http://krakov:39475
[rospy.internal][ERROR] 2014-11-04 15:44:37,269: unable to create subscriber transport: 'callerid'.  Will try again in 0.5s
[rospy.internal][INFO] 2014-11-04 15:44:37,462: topic[/rosout] adding connection to [/rosout], count 0
[rospy.core][INFO] 2014-11-04 16:00:25,946: signal_shutdown [signal-2]
[rospy.internal][INFO] 2014-11-04 16:00:25,966: topic[/rosout] removing connection to /rosout
[rospy.impl.masterslave][INFO] 2014-11-04 16:00:25,967: signal-2
[rospy.core][INFO] 2014-11-04 16:00:26,070: signal_shutdown [atexit]

This is simple fix a problem:

*** tcpros_base.py.orig 2014-11-04 15:58:56.411691733 +0300
--- tcpros_base.py  2014-11-04 15:59:46.422690161 +0300
***************
*** 582,588 ****
              if not required in header:
                  raise TransportInitError("header missing required field [%s]"%required)
          self.md5sum = header['md5sum']
!         self.callerid_pub = header['callerid']
          self.type = header['type']
          if header.get('latching', '0') == '1':
              self.is_latched = True
--- 582,589 ----
              if not required in header:
                  raise TransportInitError("header missing required field [%s]"%required)
          self.md5sum = header['md5sum']
!         if 'callerid' in header:
!             self.callerid_pub = header['callerid']
          self.type = header['type']
          if header.get('latching', '0') == '1':
              self.is_latched = True
@dirk-thomas
Copy link
Member

Can you please create a pull request with your proposed change.

But from a first look it seems that the same code is also used for the service connection where the header field is actually required. So the patch will likely need some conditional logic before being merged.

@akru
Copy link
Contributor Author
akru commented Nov 4, 2014

Really, for service transaction this field is required. So 'unknown' value of var is a best solution that crash.

@dirk-thomas
Copy link
Member

Allow the callerid header to be missing and setting the value to unknown is not considered to be the right behavior for service connections. It should assert that the callerid header is present when parsing the header and not fail sometime later.

@dirk-thomas
Copy link
Member

Closed by #525.

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

No branches or pull requests

2 participants