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

Enable live patching of messages ? #12

Open
asmodehn opened this issue Sep 4, 2017 · 4 comments
Open

Enable live patching of messages ? #12

asmodehn opened this issue Sep 4, 2017 · 4 comments
Labels

Comments

@asmodehn
Copy link
Member

asmodehn commented Sep 4, 2017

The current import system, when importing a message, looks for all its dependencies and generate ROS code for it.

This works fine, but it also means that a python code, cannot override or patch the generated ROS code.

It would be useful to be able to patch/extend a ROS message with some python code without modifying the generated code (usecase in pyros-msgs at the moment), at import time. But I am not sure it is even possible, while ROS message dependencies are other messages (directly, not the python source), and the python default import takes care of all cases (one cannot have an extension to python import system after FileFinder - currently, not a .py file will fail to import, without testing next importer in hooks).

Anyway it might be something worth keeping in mind, although for now we ll implement some workaround in pyros-msgs.

@asmodehn
Copy link
Member Author

asmodehn commented Sep 4, 2017

I am not sure fit his is a mismatching features (using the namespace package of filefinder2),
or if it is a bug.

When a main package has a subpackage that is a namespace, does the __init__ module of the main package is called before the namespace package (PEP420) gets imported ? If yes, than this doesn't happen here (and prevent patching of generated messages that should normally work). So it is a bug...
If not then ... not sure...

@asmodehn
Copy link
Member Author

asmodehn commented Sep 4, 2017

Currently this is broken : tests in #13

We also need to test the namespace package case and compare py2 (filefinder2) and py3, and find out if it is because namespace packages or just a bug in our implementation...

@asmodehn
Copy link
Member Author

asmodehn commented Sep 5, 2017

Actually not : fixing the dependent path makes it work : https://travis-ci.org/pyros-dev/rosimport/builds/271884845

@asmodehn
Copy link
Member Author

asmodehn commented Sep 5, 2017

But this works only because we "import" the generated class (which patches it) and then we instantiate that class when building the message. if neither the importing or the instantiation is done, then the patch will not be applied, which is logical.

If we don't import the dependent message definition, the ros generated message code will do the import. but then it breaks:

>         self.test_ros_msg_patch = provider.msg.TestRosMsgPatch()
E         AttributeError: 'dict' object has no attribute 'TestRosMsgPatch'

/tmp/rosimport/3750/consumer/msg/_TestRosMsgPatchDeps.py:40: AttributeError

There is probably a bug hiding around that corner...
Maybe some tests that are NOT doing the import (and let message generated code do it) are missing ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant