ros-drivers / rosserial

A ROS client library for small, embedded devices, such as Arduino. See: http://wiki.ros.org/rosserial
517 stars 525 forks source link

Can not find rosserial_arduino from PyImport_ImportModule #450

Closed tongtybj closed 4 years ago

tongtybj commented 4 years ago

When I tried SevriveClient with rosserial_server, I got following warning:

chou@chou-ThinkPad-T480:~/ros/rosserial_ws/src/rosserial/rosserial_arduino/src/rosserial_arduino$ roslaunch rosserial_server  serial.launch port:=/dev/ttyDUMMY --screen -v 
... logging to /home/chou/.ros/log/df6e69d0-09fa-11ea-83f4-04ea56a8c113/roslaunch-chou-ThinkPad-T480-10634.log
Checking log directory for disk usage. This may take awhile.
Press Ctrl-C to interrupt
Done checking log file disk usage. Usage is <1GB.

... loading XML file [/opt/ros/kinetic/etc/ros/roscore.xml]
... executing command param [rosversion roslaunch]
Added parameter [/rosversion]
... executing command param [rosversion -d]
Added parameter [/rosdistro]
Added core node of type [rosout/rosout] in namespace [/]
... loading XML file [/home/chou/ros/rosserial_ws/src/rosserial/rosserial_server/launch/serial.launch]
Added parameter [/rosserial_server/port]
Added node of type [rosserial_server/serial_node] in namespace [/]
started roslaunch server http://chou-ThinkPad-T480:44421/

SUMMARY
========

PARAMETERS
 * /rosdistro: kinetic
 * /rosserial_server/port: /dev/ttyDUMMY
 * /rosversion: 1.12.14

NODES
  /
    rosserial_server (rosserial_server/serial_node)

auto-starting new master
process[master]: started with pid [10648]
ROS_MASTER_URI=http://localhost:11311

setting /run_id to df6e69d0-09fa-11ea-83f4-04ea56a8c113
process[rosout-1]: started with pid [10661]
started core service [/rosout]
process[rosserial_server-2]: started with pid [10668]
[ INFO] [1574078369.186797626]: rosserial_server session configured for /dev/ttyDUMMY at 57600bps.
[ INFO] [1574078369.186875598]: Opened /dev/ttyDUMMY
[ WARN] [1574078369.358219379]: Unable to look up service definition: Unable to import message module rosserial_arduino.
[ WARN] [1574078369.358297995]: Service client setup: Request message MD5 mismatch between rosserial client and ROS
[ WARN] [1574078369.376327976]: Service client setup: Response message MD5 mismatch between rosserial client and ROS
[ INFO] [1574078369.376374691]: Startup complete

It seems like rosserial_arduino can not be found by PyImport_ImportModule. On the other hand, I also tried with other packages (e.g., std_srvs, moveit_msgs) and my personal packages which contain srv directory. These packages can be found. I can not figure out why only rosserial_arduino is invisible.

mikepurvis commented 4 years ago

Can you confirm that regular, non-embedded Python can find the generated classes in the environment that this is running in? Eg,

python -c "from rosserial_arduino import srv; print(srv.__file__)"
tongtybj commented 4 years ago

I can find the generated classes by your suggested command:

chou@chou-ThinkPad-T480:~$ python -c "from rosserial_arduino import srv; print(srv.__file__)"
/home/chou/ros/rosserial_ws/devel/lib/python2.7/dist-packages/rosserial_arduino/srv/__init__.pyc
mikepurvis commented 4 years ago

That's really odd, as the environment should be the same, so I'd expect the standalone interpreter to behave identically to the embedded one. Does anything change if you try this from the installspace rather than the develspace? Is your PYTHONPATH at all weird in this scenario?

Wondering as well if there's something going on with the sys.path variable in the embedded one.

Either way, it's super odd that it works with the other packages and just not that one.

tongtybj commented 4 years ago

@mikepurvis I thought this is my personal problem, but Travis in #452 gave the same results. Could you check the rostest code in #452 in your environment?

tongtybj commented 4 years ago

Does anything change if you try this from the installspace rather than the develspace? Is your PYTHONPATH at all weird in this scenario?

I will check it.

TobinHall commented 4 years ago

@mikepurvis @tongtybj I don't think this is path related. I have had issues when trying to import sensor_msgs. Initially I thought that it was a path issue so I added code to print sys.path when the module is being imported. It printed the paths I was expecting to see however, I also got the following traceback:

Traceback (most recent call last): File "/opt/ros/melodic/lib/python2.7/dist-packages/sensor_msgs/msg/init.py", line 1, in from ._BatteryState import File "/opt/ros/melodic/lib/python2.7/dist-packages/sensor_msgs/msg/_BatteryState.py", line 5, in import genpy File "/opt/ros/melodic/lib/python2.7/dist-packages/genpy/init.py", line 34, in from . message import Message, SerializationError, DeserializationError, MessageException, struct_I File "/opt/ros/melodic/lib/python2.7/dist-packages/genpy/message.py", line 44, in import yaml File "/usr/local/lib/python2.7/dist-packages/yaml/init.py", line 14, in from cyaml import File "/usr/local/lib/python2.7/dist-packages/yaml/cyaml.py", line 5, in from _yaml import CParser, CEmitter File "ext/_yaml.pyx", line 64, in init _yaml AttributeError: type object '_yaml.Mark' has no attribute '__reduce_cython__'

Is anyone else able reproduce this issue when trying to transfer any message from sensor_msgs?

stertingen commented 4 years ago

Do you have multiple topics with the same type? The message lookup using embedded python seems to be a little buggy when looking up the same module several times.

I've set up a little test program to narrow the problem down (tested with Python 3):

#include <Python.h>
#include <cstdio>

void try_import() {
    Py_Initialize();
    PyObject* module = PyImport_ImportModule("std_msgs.msg");
    if (!module)
    {
        puts("Import error!");
    }
    Py_XDECREF(module);
    Py_Finalize();
}

int main() {
    puts("1st try");
    try_import();
    puts("2nd try");
    try_import();
}

Then I get the following log:

1st try
2nd try
Import error!
Traceback (most recent call last):
  File "/opt/ros/melodic/lib/python3.8/site-packages/std_msgs/msg/__init__.py", line 1, in <module>
    from ._Bool import *
  File "/opt/ros/melodic/lib/python3.8/site-packages/std_msgs/msg/_Bool.py", line 5, in <module>
    import genpy
  File "/opt/ros/melodic/lib/python3.8/site-packages/genpy/__init__.py", line 34, in <module>
    from . message import Message, SerializationError, DeserializationError, MessageException, struct_I
  File "/opt/ros/melodic/lib/python3.8/site-packages/genpy/message.py", line 44, in <module>
    import yaml
  File "/usr/lib/python3.8/site-packages/yaml/__init__.py", line 13, in <module>
    from .cyaml import *
  File "/usr/lib/python3.8/site-packages/yaml/cyaml.py", line 16, in <module>
    class CBaseLoader(CParser, BaseConstructor, BaseResolver):
TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

So the first import is successful, any later import fails. Removing any call to Py_Finalize seems to solve the issue. According to the docs, Py_Finalize is not guaranteed to work properly. (https://docs.python.org/3.5/c-api/init.html#c.Py_Finalize)

There are a few possible solutions for this:

  1. Remove calls to Py_Finalize for the cost of memory leaks (least preferred solution)
  2. Spawn python subprocess and parse stdout (Maybe using Boost.Process https://www.boost.org/doc/libs/1_72_0/doc/html/process.html)
  3. Figure out how to clean up python properly (might be not easy)
  4. Read message files from file system without Python (using ropack or similar)
mikepurvis commented 4 years ago

Fascinating, thanks for tracking this down, @stertingen. I had considered spawning a whole subprocess for this, but that seemed like a really heavyweight solution when a simple embedding of the interpreter should work fine. Some other options:

I definitely considered trying to read the messages sans Python, but it felt like it would be a lot of complexity and fragility trying to extract (or compute?) the MD5 via some other mechanism.

stertingen commented 4 years ago

We could give Boost.Python a try: https://www.boost.org/doc/libs/1_72_0/libs/python/doc/html/tutorial/tutorial/embedding.html

mikepurvis commented 4 years ago

Boost.Python is just a wrapper on the native APIs; I doubt it would give us anything here. I think the easiest is probably to just initialize once and stay initialized.

stertingen commented 4 years ago

Working solution using Boost.Process: https://github.com/stertingen/rosserial/tree/msg_lookup_with_boost_process

I'm not 100% happy with it due to the lack of a nice timeout functionality. (wait_for does not what I thought it does; It seems to wait exactly n seconds and return instead of returning earlier when the child process finishes.)

Fetching the path to the python executable at runtime would be nice, but not necessary. (Currently passed by CMake.)

The startup performance (~30 Subscribers & Publishers) is reasonable (nothing compared to rqt!).

mikepurvis commented 4 years ago

@stertingen Interesting, thanks for sharing that approach. I hadn't imagined forking off a process that way and passing the result with IPC, I was definitely picturing just invoking python with popen and then passing the result back through stdout somehow.

In any case, I modified your MWE to avoid the finalize call, and then it does work multiple times (as expected):

void try_import() {
    static bool py_initialized = false;
    if (!py_initialized)
    {
      Py_Initialize();
    }
    PyObject* module = PyImport_ImportModule("std_msgs.msg");
    if (!module)
    {
        puts("Import error!");
    }
    Py_XDECREF(module);
}

Before Py_Initialize, the process size is 9.6MB, and afterward, 24.5MB, per pmap. But it barely shrinks at all if I take the measurement after the first Py_Finalize anyway. In any case, our "real world" rosserial_server instances have an overall footprint in the 100s of MB, so I don't think the cost of keeping the interpreter instance around is worth worrying too much about.

mikepurvis commented 4 years ago

Fixed by #491, please reopen if the issue persists.