Macro arg resolution with namespaces

[Doomerdinger]

After the 1.13.15 update I encounter issues when using macros imported with namespaces.

generic_utils.xacro

<xacro:macro name="do_a_thing" params="first_arg">
  <xacro:if value="${first_arg}">
    Foobar
  </xacro:if>
</xacro:marco>

<xacro:macro name="my_macro" params="dict_config">
  <xacro:include filename="$(find example_pack)/folder/generic_utils.xacro" ns="generic_utils"/>
  
  <xacro:generic_utils.do_a_thing first_arg="${dict_config.item}"/>
  <xacro:generic_utils.do_a_thing first_arg="${dict_config['item']}"/>
</xacro:maro>

I've only tested with a dict input myself (using load yaml), I cannot say if this issue persists elsewhere.

The above setup will result in an error, complaining that dict_config is not defined. This will come from within the generic_utils.xacro file.
If the namespace on the include is omitted so it is just <xacro:include filename="$(find example_pack)/folder/generic_utils.xacro"/> and subsequent references to the namespace are removed, it seems to work fine.

I assume this is something to do with the changes either on #306 or #297.

[rhaschke]

Thanks for reporting this issue. Indeed, this was a consequence of the stricter namespace handling introduced recently.
#308 should fix this. Could you please verify?

[Doomerdinger]

Thanks for the quick update, I'll test this in the next day or two.

[Doomerdinger]

While it seems that this (may) solve the initial issue cited, it appears to break the set property value in parent functionality, eg

<xacro:property name="parent_prop" scope="parent" value="whatever"/>

[rhaschke]

In which context does this fail now?

[Doomerdinger]

If I have a parent calling a macro, and that macro does the above <xacro:property name="parent_prop" scope="parent" value="whatever"/> and the parent expects that property to be set after the call, it is not set.

[rhaschke]

But there is a non-failing unit test for this:

xacro/test/test_xacro.py

Lines 495 to 501 in 2729327

	def test_property_scope_parent(self):
	self.assert_matches(self.quick_xacro('''<a xmlns:xacro="http://www.ros.org/wiki/xacro">
	<xacro:macro name="foo" params="factor">
	<xacro:property name="foo" value="${21*factor}" scope="parent"/>
	</xacro:macro>
	<xacro:foo factor="2"/><a foo="${foo}"/></a>'''),
	'''<a><a foo="42"/></a>''')

[Doomerdinger]

Interesting. I will test further.

[Doomerdinger]

Ok I think I have it.

If the parent includes the macro with a namespace and then the parent calls that macro, properties that are set to the parent scope must be accessed with the namespace accessor. This is not how it worked before (though I am not sure if it worked that way before these changes you've made here, or before the other namespace handling introduced recently), but I am not sure if it is a bug or not.

Would make sense that setting something in the parent namespace would put the property in the namespace of whatever the parent is in, not whatever my current namespace is, but I could see it the other way as well.

macro_utils.xacro

<xacro:macro name="set_property" params="">
  <xacro:property name="parent_prop" scope="parent" value="whatever"/>
</xacro:macro>

parent.xacro

<xacro:include filename="$(find utils)/urdf/macro_utils.xacro" ns="macro_utils"/>
<xacro:macro_utils.set_property/>
<a foo="${parent_prop}"/></a> <!-- ERROR -->
<a foo="${macro_utils.parent_prop}"/></a> <!-- A-OK -->

[rhaschke]

Interesting example. In this case, essentially, two new scopes are created:

• root scope
  • namespaced include scope
    • macro scope

Thus, defining a property within the root scope from within the namespaced macro requires lifting the property by two scopes, which is not yet supported. Actually, We could even have multiple nested namespaced includes, which would create even more scopes.
Setting a property within the parent scope may occur in two contexts:

1. From within a macro. In that case, one wants to set the property in the caller's scope.
2. From within the included file. In that case, one wants to set the property in the includer's scope.

I will adapt the PR appropriately.

[rhaschke]

Fixed via #308