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

Compatible with dynamic reconfigure #177

Open
goodahn opened this issue May 2, 2019 · 6 comments
Open

Compatible with dynamic reconfigure #177

goodahn opened this issue May 2, 2019 · 6 comments

Comments

@goodahn
Copy link

goodahn commented May 2, 2019

Currently, it's not able to change parameter that is created by dynamic reconfigure.
Although I added UI part for updating parameter value, it can't be done.
Because of a type conflict and missing member, rostful can't update parameter.
Could you resolve this problem?

@asmodehn
Copy link
Member

asmodehn commented May 3, 2019

Hello, thanks for reporting this issue.
It used to work, from what I can remember, so lets investigate...

I have a few questions :

  • Which version are you using ? latest from master ?
  • Where is the error exactly ? It seems to me at first glance that a dynamic reconfigure related error would not appear in rostful, since rostful is only a WSGI frontend for ROS. So I would imagine the error would appear lower, in one of the pyros packages... but I might be wrong.
  • Where is the code ? Can you show me what you have done somewhere so I can replicate the issue?
    Any MCVE ?

Finally, this software is free, which means it is also yours, so feel free to fork and change what you need to make it work, an send a PR ;-)

Let me know if you find more clues !

@goodahn
Copy link
Author

goodahn commented May 3, 2019

Thank you for your kind answer!

  • I use rostful 0.2.1 version which is the latest from master.

  • There are two reasons for error.

  1. Rostful uses dict type. However, when rostful receive parameters from dynamic reconfigure, it's OrderedDict type. So it causes error.

  2. Rostful checks type of messages by msgtype, however, dynamic reconfigure has prmtype.

Traceback (most recent call last):
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/app.py", line 2292, in wsgi_app
response = self.full_dispatch_request()
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/app.py", line 1815, in full_dispatch_request
rv = self.handle_user_exception(e)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask_restful/init.py", line 269, in error_router
return original_handler(e)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask_cors/extension.py", line 161, in wrapped_function
return cors_after_request(app.make_response(f(*args, **kwargs)))
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/app.py", line 1718, in handle_user_exception
reraise(exc_type, exc_value, tb)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/app.py", line 1813, in full_dispatch_request
rv = self.dispatch_request()
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/app.py", line 1799, in dispatch_request
return self.view_functionsrule.endpoint
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/rostful/frontend/flask_views.py", line 173, in ros_interface
return render_template('param.html', param=param)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/templating.py", line 135, in render_template
context, ctx.app)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/flask/templating.py", line 117, in _render
rv = template.render(context)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/jinja2/environment.py", line 1008, in render
return self.environment.handle_exception(exc_info, True)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/jinja2/environment.py", line 780, in handle_exception
reraise(exc_type, exc_value, tb)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/rostful/templates/param.html", line 92, in top-level template code
{% from 'macros.html' import content_navbar, generate_inputs, generate_msgdata %}
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/rostful/templates/layout.html", line 31, in top-level template code
{% block POST_ajax %}{% endblock %}
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/rostful/templates/param.html", line 60, in block "POST_ajax"
{{ generate_msgdata(param.msgtype) }}
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/jinja2/runtime.py", line 579, in _invoke
rv = self._func(*arguments)
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/rostful/templates/macros.html", line 149, in template
{% for name, type in fielddict.iteritems() %}
File "/home/ahn/example/venv/local/lib/python2.7/site-packages/jinja2/environment.py", line 430, in getattr
return getattr(obj, attribute)
UndefinedError: 'collections.OrderedDict object' has no attribute 'msgtype'
ERROR:tornado.access:500 GET /frontend/dynamic_tutorials/str_param (127.0.0.1) 187.76ms

  • This is my simple MCVE.
    (It was implemented without considering PR.)

Please let me know if there are any problems :)

@asmodehn
Copy link
Member

asmodehn commented May 8, 2019

Oh I see what you are doing there :-)
You create a ROS node directly in the flask WSGI app.

Actually this was originally the design of rostful : a simple wsgi app that is also a ROS node.
You can have a look at the original version from BenKehoe

Although its simplicity is a strength, there was problems with this design :

  • Environment Setup : all web code now also depends on ROS, our web code must run in a ROS environment (and ROS messes up your PYTHONPATH)
  • Security: Database code injection is already known, when robot code injection will become famous ?
  • Maintenance : All our python dependencies must now be ported to ROS, and in web there is a lot...
  • Performance : Scaling our webserver potentially impact our ROS system. and most webserver scale automatically, based on request arriving from potentially everywhere...

So I had to accept this design was too simple, and something had to be done, even if it ended up a bit more complex.

Thats where Pyros comes from.
With PyROS design, there is a strict separation of concern (relying on OS separation of processes) :

  • pyros deals with pure python things only.
  • interfaces are somewhat 'pluggable' in that framework (most notably pyros_interfaces_ros) to deal with specific systems.
  • rostful becomes just a normal python flask app, running with Pyros taking care of ROS stuff.

So I advise that you have a look at the code and track the 'get_pyros_client()' call.
You will see it retrieves an object that is previously set with set_pyros_client from Server.launch

It is around that time that pyros is launched (another process !), and pyros is creating the actual ROS node. (you can check it on rqt).
You can see it there : https://github.com/pyros-dev/pyros-rosinterface/blob/master/pyros_interfaces_ros/pyros_ros.py#L34

So you ll need to dive deeper into the code (don't forget to take your debugger along for the ride).
Having multiple processes makes it harder to debug, but gives a strong isolation to address all the issues I talked about before.

You should be able to add code for dynamic reconfigure into the original pyros node...
There used to be one a long time ago.
Eventually I decided to move it out because :

  • there are many ways to configure the node (file, launch params, etc.), and it is tricky to test them all, some being dynamic, and others not, especially when there are multiple process involved...
  • there is https://github.com/awesomebytes/ddynamic_reconfigure that might be simpler to integrate (I haven't tried)
  • and I wasn't using it :)

So feel free to play around and send a PR when you get something working, but more importantly, I think pyros needs tests, and we should try to identify where things can be made simpler, but not too simple...

The next piece of the puzzle for me is the launch workflow : how these processes are launched? how can they find each other ? how can each find their configuration ? quite a tricky problem...

@goodahn
Copy link
Author

goodahn commented May 10, 2019

Thank you for your kind explanation!
As you said, I already experienced such dependency problems...
I will look into about this problem more and if I get a good solution, I will send a PR.
Thank you!

@asmodehn
Copy link
Member

Hi,
In case it might help, I have updated a rostful usecase with turtlesim : asmodehn/rostful-examples#6
Let me know if you find it useful.
Thanks.

@goodahn
Copy link
Author

goodahn commented Jun 24, 2019

Thank you for letting me know this update :)
And sorry for reading your answer late.
Thank you!

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