`WaitForTopics`: there is no way to manually trigger a topic
LastStarDust opened this issue · 8 comments
Feature request
Feature description
The current implementation of WaitForTopics
assumes that the node under test publishes messages without any external stimulus. But it might be that a node only publishes a topic if it received a certain stimulus, like receiving a message or a request.
In other words, let us image a node acting as both subscriber and publisher, with an input topic (subscriber) and an output topic (publisher). The input is received and processed and the output is emitted.
In this case, WaitForTopics
would be waiting for a topic without any means to actually trigger it.
Implementation considerations
If the input topic is injected into the test node before calling WaitForTopics::wait()
, the output topic might be emitted before the wait()
call and it is not intercepted by wait()
. On the other hand, if the input topic is injected after the call to wait()
, obviously wait()
cannot intercept the output.
One solution would be to inject a callback function into WaitForTopics
that can take care of feeding the node under test with the needed input to produce the desired output.
Another solution is to provide a synchronization mechanism, such that the caller can create a dedicated thread to generate the input.
In any case, the input should be generated just after the subscribers have been started
def wait(self):
self.__ros_node.start_subscribers(self.topic_tuples)
# <<<<<<<<< HERE >>>>>>>>>>
return self.__ros_node.msg_event_object.wait(self.timeout)
Similarly to the issue #346 I am available to develop a solution if you think this feature is worth the effort.
I have a couple of questions wrt #356 :
-
It would be nice to name the trigger function with something other than a
callback
, as that term exists in ROS nodes and can be confusing. Secondly, its a method that would be triggered before the subscribers start, and hence are not callbacks exactly. They'll be acting more like triggers, or pre-set type of methods, and I'm open to using some other name for them. -
In the PR, it would be nice to have an example/test node that actually waits for a topic, or a service to be set using the trigger function, and have a nontrivial test case.
-
Ideally, I can imagine such a node would wait for a service, or a parameter to be set to act as a trigger. Would adding an API here be better than setting those explicitly before calling
WaitForTopics()
? -
An open ended method seems pretty generic to me. Would it be worth restricting the actions it can do, like making a service/action call ?
Open to suggestions @methylDragon @wjwwood
-
-
I agree that the name
callback
is too generic. What abouttrigger_callback
or justtrigger
? -
its a method that would be triggered before the subscribers start
maybe there was a misunderstanding but the whole point of this trigger feature is to execute an action AFTER the subscribers have started. Otherwise just calling the trigger before constructing the
WaitForTopic
object would suffice.
-
- I agree, however, I would like to define the scope and base implementation before writing a full-blown non-trivial sample.
- Yes an API would be nicer. I am open to suggestions about it.
- Again yes, the whole point of this trigger callback is to make sure that the input messages into the node under test are published AFTER the node subscribers have started, otherwise the messages are lost. So the main action to "allow" is the publishing of messages.
@LastStarDust Could you give us a little more concrete example of what you are trying to do here? We are just trying to figure out whether this is the right way to go, or if we should do something different like return a future from start_subscribers
. Thanks.
@LastStarDust Could you give us a little more concrete example of what you are trying to do here? We are just trying to figure out whether this is the right way to go, or if we should do something different like return a future from
start_subscribers
. Thanks.
@clalancette Yes, of course. You can check the launch_ros/launch_testing_ros/test/examples/wait_for_topic_inject_callback.py
test for a more comprehensive example of how I envision this feature to be used.
Hey there, this is a great improvement. I have implemented something similar. Would you like to compare branches, take the best of both, and get this PR into rolling
, iron
and humble
? Support for non-periodic publishers in automated testing is very important to me.
@Ryanf55 Thank you for your interest in this feature.
I am the implementer, and still waiting for the review by the maintainers.
But of course, I would be happy to see my changes merged with yours.
Actually, there is one improvement that I would like to make to this PR, after the first round of review.
I would like to make the node object inside the WaitForTopics
class available to the callback function.
Yea, I took quite a different approach and can post it here once I get approval (a few days). I'm glad you're still interested in getting something like this merged. Chat soon.