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

Feature/rosproxy #119

Open
wants to merge 18 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@m3d
Copy link
Member

m3d commented Dec 15, 2018

This is squashed version of more than 50 commits (work in progress reference is in feature/subt-ver0) to get ROS proxy working with SubT Challenge example (robot X1). Currently it can be test it on go1m "application". I am aware that it is too big, so splitting (or any) suggestions are welcome.

I did not succeed yet with XML RPC logging (in separate branch) so I would probably postpone it. Also note, that this version supports only one "publish topic" (cmd_vel) and for more we would need more ports (now hardcoded, i.e. it does not support multiple instances of ROSProxy now.

p.s. I will also add FIXUP for new TCP drivers ...

m3d added some commits Nov 24, 2018

Add node ROSProxy
rosproxy.py - FIXUP add unittest (WIP)

rosproxy.py - FIXUP clean

rosproxy.py - hacked parse topic

rosproxy.py - hack to rend cmd_vel (locally)

rosproxy.py - wait for init message FIXUP

rosproxy.py - add MyXMLRPCServer

rosproxy.py - FIXUP missing PUBLISH_PORT

rosproxy.py - prepare topic subscription

rosprox.py hacked IP addres to be 127.0.0.1

rosproxy.py - try more subscribers

rosproxy.py - FIXUP md5 for laser and image

rosproxy.py - handle compressed images

rosproxy.py - FIXUP md5 for compressed image

rosproxy.py - add odometry

rosproxy.py - FIXUP odometry md5sum

rosproxy.py - integrate desired speed
Add SubT Challenge ver 0 example for Virtual Track
subt-vt-ver0.json - FIXUP config with ROS master URI etc

subt-vt-ver0.json - experiment with prepared TCP connection

Add Ubuntu local configuration

ubuntu-subt-ver0.json - use MyTimer

ubuntu-subt-ver0.json - FIXUP syntax

ubuntu-subt-ver0.json - prepare TCP connection for topic /imu/data

ubuntu-subt-ver0.json - FIXUP

subt-ver0.json - add subscribe and unify config files (Ubunto version only)

subt-ver0.json - FIX JSON

subt-ver0.json - parse IMU data

subt-ver0.json - subscribe laser and image data

subt-ver0.json - fixed outputs

subt-ver0.json - compressed image

subt-ver0.json - add topic_type to parse JPEG image

subt-ver0.json - integrate laser

subt-ver0.json - collect odometry

subt-ver0.json - fixup output

subt-ver0.json - integrate pose and scan into app

subt-ver0.json - FIXUP rosproxy is not publishing pose2d
Add rosmsg for parsing ROS messages
rosmsg.py - hack parsing IMU data

rosmsg - disable parsing test

rosmsg.py - hack parse image

rosmsg.py - parse JPEG data

rosmsg.py - parse JPEG images during run

rosmsg.py - parse laser data

rosmsg.py - integrate laser

rosmsg.py - start parsing odom data

rosmsg.py - parse odom and publish pose2d

@m3d m3d requested a review from zwn Dec 15, 2018

m3d added some commits Dec 15, 2018

logsocket.py - FIXUP TCP Server timeout
Exception in thread Thread-3:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/home/martind/md/osgar/osgar/drivers/logsocket.py", line 136, in run_input
    self.socket, addr = self.socket.accept()
  File "/usr/lib/python3.6/socket.py", line 205, in accept
    fd, addr = self._accept()
socket.timeout: timed out
@m3d

This comment has been minimized.

Copy link
Member

m3d commented Dec 15, 2018

I am sorry for the fixups :(. There was a problem, that even go1m was not properly terminated, due to blocked accept() and socket.recv() ... can be moved to separate PR, if you prefer. Also if you want to rebase this PR before review, let me know. Thanks.

@m3d

This comment has been minimized.

Copy link
Member

m3d commented Dec 27, 2018

I added couple of commits necessary to get the first point in SubT Challenge (developed in feature/subt-ver1 branch). Let me know if it is time for squash and clean-up. The pending tasks are multiple published topics (needed for SubT lights) and Request/Response (needed for the query of robot pose at the very beginning). For motivation see https://robotika.cz/competitions/subtchallenge/cs#181227 ... but I would prefer to do it as revision step after merge to master, maybe even after release of v0.2?

@zwn
Copy link
Member

zwn left a comment

I need a way more time on this 😟 . In the meantime, can we merge at least the timeout stuff separately? If you have any idea how to split this or simplify somehow, I am all for it.

from osgar.bus import BusShutdownException


class MyTimer(Thread):

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

Why are all the nodes using Thread as a base class? I was under the impression that Node is the new base class.

This comment has been minimized.

@m3d

m3d Dec 31, 2018

Member

I guess that these classes were written before Node was ready and merged in master. You are right that it could/should be refactored now.

}


def prefix4BytesLen(s):

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

We have this function twice.


def prefix4BytesLen(s):
"adding ROS length"
if type(s) == str:

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

Why would anyone give unicode string to this function? Or maybe I don't understand the intent of the function.

This comment has been minimized.

@m3d

m3d Dec 31, 2018

Member

This was due to port from Python2, when original "hello world" was sending string. I will check it, thanks.

self.server.serve_forever()


class ROSProxy(Thread):

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

I have not been able to grok this node yet.

header = None
# initial message contains structure
while True:
timestamp, channel, data = self.bus.listen()

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

It ignores channel.

if packet is not None:
if header is None:
header = packet
elif self.topic_type == 'sensor_msgs/CompressedImage':

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

Hmm. So. One instance of this node supports parsing one topic type. And based on the type of the topic, it selects the name of the output where it publishes. That seems strange.

},
"rosmsg_imu": {
"driver": "rosmsg",
"in": ["addr", "raw"],

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

addr is not used in rosmsg.

["ros.imu_data_addr", "tcp_imu_data.addr"],
["ros.imu_data", "tcp_imu_data.raw"],
["tcp_imu_data.raw", "ros.imu_data"],
["tcp_imu_data.raw", "rosmsg_imu.raw"],

This comment has been minimized.

@zwn

zwn Dec 30, 2018

Member

I think I don't understand the communication structure here. Is the raw data once parsed in the rosproxy and once in rosmsg?

@m3d

This comment has been minimized.

Copy link
Member

m3d commented Dec 31, 2018

I was going to split it into 4 PR (bugfix timeout, rosproxy, rosmsg and example/subt), but they are dependent and in particular it is not clear if the split rosproxy/rosmsg is right one. For each TCP connection is extra rosmsg, which is collecting buffer and based on type publishes particular type of message. Note, that there could be multiple topics of the same type, so one global parser is not option (?).

@zwn

This comment has been minimized.

Copy link
Member

zwn commented Jan 2, 2019

I was thinking about it some more. From the user point of view, I don't think it is good to require such a massive configuration just to talk to ROS or handle one more topic. Thinking about the usage, I think it would be nice to just instantiate one "ROS node" and in the configuration connect ros-topic to osgar-input/osgar-output. However, what this seems to be leading to is dynamic inputs and outputs - something we have not discussed yet.

In general, I would like to aim for minimum configuration and maximum autodetection of anything related to ROS. Otherwise we are forcing the poor osgar user more and more into knowing how ROS is implemented inside.

@m3d

This comment has been minimized.

Copy link
Member

m3d commented Jan 10, 2019

I added subt-x1.json configuration as example of alternative for ROSProxy configuration, which we discussed yesterday. With this exercise I realized two details:

  • the ROS <-> OSGAR messages would have to be 1:1
  • there is a need for timer which for example publishes desired speed with given frequency (both old as well this module configuration do not have correct "in")
@zwn

This comment has been minimized.

Copy link
Member

zwn commented Jan 11, 2019

the ROS <-> OSGAR messages would have to be 1:1

That is not new, right? We have been working with 1:1 in rosmsg as well. Is it a problem?

there is a need for timer which for example publishes desired speed with given frequency (both old as well this module configuration do not have correct "in")

I am not sure I understand correctly. Can you describe the problem in more detail? We need to send speed on time to the CAN controller on Eduro as well - so, what is the difference? What is the meaning of the sentence in ()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment