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

Python macros #170

Open
wants to merge 1 commit into
base: lunar-devel
Choose a base branch
from
Open

Python macros #170

wants to merge 1 commit into from

Conversation

Zabot
Copy link

@Zabot Zabot commented Nov 3, 2017

Heres a rough sketch of what I was referring to in #168. I put this together to work with this python script which generates the link properties given a mesh. The macro is included by this xacro.

I added one additional tag, xacro:python which has a two attributes file and module. Adding an xacro:python tag will import module from file. Macro calls will tested against the attributes defined in imported modules if they don't match any existing macros, and if a matching attribute is found that attribute will be called and passed the attributes of the macro as keyword arguments.

In an xacro file:

<xacro:python file="$(find some_package)/scripts/useful.py" module="useful"/>
<xacro:complex_macro foo="foobar" bar="foobar"/>

useful.py:

def complex_macro(foo, bar):
    return '<link name="{}"/>'.format(foo + bar)

I'm not a huge fan of how XML elements are passed to and from the user function call (being stringified, and then parsed out both ways), but I wanted to get the idea out for comment. Because the user function is actually being imported as opposed to just being evaled, we could allow for stronger typing/pass actual DOM elements back and forth.

@rhaschke
Copy link
Contributor

In general I like the idea to extend xacro's capabilities by additional python functions.
Instead of calling your function via <xacro:complex_macro/> I suggest to import the new functions into the internal python namespace and just call them as usual: ${complex_macro("foo", "bar")}. Basic math functions (like sin and cos) can be directly used already - as you mentioned that in your previous post.

@alikureishy
Copy link

Any update on this pull request?

@rhaschke
Copy link
Contributor

rhaschke commented Apr 8, 2020

@Zabot didn't react since 2017. Anybody continuing this work is welcome.

@jbohren
Copy link
Member

jbohren commented Feb 26, 2022

@rhaschke instead of generating xml in python functions called from xml, should we move away from xacro and towards generation of xml directly from the URDF python library?

@rhaschke
Copy link
Contributor

Hi Jonathan,
what exactly do you have in mind? From my point of view, xacro is a very generic XML pre-processor, not limited to URDF.
Integrating that functionality into the python URDF parser library doesn't make very much sense to me. But, maybe I didn't get your point...

@jbohren
Copy link
Member

jbohren commented Feb 28, 2022

what exactly do you have in mind? From my point of view, xacro is a very generic XML pre-processor, not limited to URDF.

Understood. I would argue that my point (elaborated below) stands for any kind of xml generation.

Integrating that functionality into the python URDF parser library doesn't make very much sense to me. But, maybe I didn't get your point...

I wasn't entirely clear.

I'm contrasting two approaches to address the use case of generating complex XML documents from XML building blocks and with configurable parameters.talk more about it.

One approach is what we've been using for over a decade now: xacro. With xacro the XML building blocks are contained in XML xacro macros, and the parameters are xacro variables. These special XML tags require being processed by a xacro parser. Over time, xacro has gained more and more features organically. The xacro learning curve has recently been discussed in a retrospective here: https://arxiv.org/pdf/2109.09694.pdf (thanks @jpwco)

Another approach is to programmatically generate the tags using some XML DOM API. With this approach, functions defined in the programming language generate building blocks based with parameters based on arguments to the generating functions. With python, this could work without any schema-enforcing API as shown below. Note that while this is an exceedingly simple example, it should be obvious how to refactor and extend a simple approach like this.

from math import pi, radians

from lxml import etree
from lxml.builder import E

alpha = 30/180*pi

robot = E.robot(
    E.link(name='world'),
    E.link(name='l1'),

    E.joint(
        E.parent(link='world'),
        E.child(link='l1'),
        E.limit(
            lower=f'{-alpha}',
            upper=f'{alpha}',
            effort='0',
            velocity=f'{radians(75)}'),
        name='j1',
        type='revolute'),
    name='bot')

print(etree.tostring(robot, pretty_print=True))

In my recent experience, I've found this approach significantly easier to maintain and more scalable than what I've seen over many years of juggling complex xacro models. Further constraints could be added to enforce a given schema, and you end up with something like @hauptmech's https://github.com/hauptmech/odio_urdf

Consider this food for thought, I'd be happy to discuss it more.

@hauptmech
Copy link

@jbohren This might be a good conversation to strike up on ROS Discourse. I agree with @rhaschke. xacro's place is as a xml preprocessor and I think it does its job well.

I have further opinions on URDF and when it's appropriate to use a preprocessor like xacro, but that is not relevant to this thread. If you start a discussion on discourse, I'll join in there.

@jbohren
Copy link
Member

jbohren commented Feb 28, 2022

@jbohren This might be a good conversation to strike up on ROS Discourse.

I agree that this discussion would benefit a larger audience, but I don't think a larger audience would benefit this discussion. I've seen too many of those design objective discussions get out of hand and go nowhere.

I agree with @rhaschke. xacro's place is as a xml preprocessor and I think it does its job well.

Right, and if it's an xml preprocessor, I think it would be useful to focus on this question:

"should xacro be made powerful enough to call arbitrary xml-generating python code?"

My position is: no, it should not, but the use case is absolutely valid

I think @Zabot ran up against a really common limitation of using xacro to modularize an URDF. This was five years ago now, but the same issues persist. I can definitely empathize with what he's trying to do, but I think if he had better tools to generate the model in the way he wanted, he wouldn't need it.

While @Zabot might not be actively working with xacro and ROS in the open source community any more, I think it's appropriate to discuss if xacro should ever have such complex capability as he's proposed in this PR, co-located with the PR and use case.

If such capability doesn't belong in xacro, then this PR can be closed, and people running into xacro's limitations could be directed to tools like odio_urdf and the even more bare-bones example (which is not urdf-specific) that I gave above.


Aside: @hauptmech Hey thanks for joining! I came across odio_urdf a few months back and while I'm not currently using it, I love the design patterns it affords.

@rhaschke
Copy link
Contributor

rhaschke commented Mar 1, 2022

I agree with @hauptmech that both, xacro and odio_urdf have their distinct use cases. I also agree with @jbohren that we shouldn't further extend (and complicate) xacro to allow for "arbitrary xml-generating python code".
However, to facilitate learning xacro, I planned to write some more in-depth documentation with examples, but didn't find time yet...

@jpwco
Copy link

jpwco commented Mar 7, 2022

Hi All :)

From my point of view, xacro is a very generic XML pre-processor, not limited to URDF.

I also thought of xacro as being more general than URDF, but after searching GitHub for XML files containing "xacro" and not "urdf", I'm less convinced that this happens in practice. The search yields +100K XML files -- and scanning ~20 random pages of results, I have yet to find an XML containing a xacro: tag that is not URDF-related. Further narrowing the search (too much?) also did not reveal a non-URDF usage of xacro, no?

Perhaps xacro is generic in design, but specific to URDF in practice?

@hauptmech: odio_urdf -- cool!

@rhaschke: Thank you for all the support you've provided and continue to provide for xacro throughout the years---the ROS community has benefitted greatly from your efforts---again, thank you so much!

@rhaschke
Copy link
Contributor

rhaschke commented Mar 7, 2022

@jpwco: Some MoveIt robot configs use xacro for SRDF as well. Other than that, you are probably right: practical other uses of xacro might be rare.

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

Successfully merging this pull request may close these issues.

6 participants