Node sub-namespaces are not handled correctly

[peci1]

With #113 (and after removing the deprecated functions), this package will lose ability to correctly resolve sub-node namespaces (because they are not reflected in any way in NodeInterfaces). (It doesn't handle them correctly even now, but once Node interface is removed, there will be no trace of the sub-namespaces).

Create a sub-node:

rclcpp::Node::SharedPtr subNh = this->create_sub_node("sub");

Compare:

this->fixSub = subNh->create_subscription<sensor_msgs::msg::NavSatFix>("fix", rclcpp::SensorDataQoS(),
[this](const sensor_msgs::msg::NavSatFix& msg) { this->fixCb(msg); });

and

this->magSub = std::make_unique<message_filters::Subscriber<sensor_msgs::msg::MagneticField>>(subNh, "mag", 100);

fixSub will correctly subscribe to /sub/fix, while magSub will counter-intuitively subscribe /mag.

I'm not sure what's the best way to solve this.

Node::create_subscription() calls extend_name_with_sub_namespace() here.

One way would be to keep the Node constructors and un-deprecate them. But then the result would be different based on whether you pass *node or node as argument, which would be another confusing pattern.

It seems to me the more correct way would be adding sub-namespace-related functionality to NodeTopicsInterface or NodeBaseInterface.

What do you think?

Originally posted by @peci1 in #113 (comment)

[authaldo]

In my view the second option (addition to NodeInterfaces) should be the preferred one. Having the node pointer as part of the interface was one of the main reasons why lifecycle nodes lacked the support for image_transport for multiple years. Hence, going back to that interface would open up that problem once again.