search

ros2 / launch_ros

Tools for launching ROS nodes and for writing tests involving ROS nodes.
Apache License 2.0
60 stars 74 forks source link

Add async support for subscribing messages #375

Closed Ryanf55 closed 9 months ago

Ryanf55 commented 1 year ago

This is an alternative to #356, used for discussion. It doesn't pass because the wait blocks.

Ryanf55 commented 1 year ago

@LastStarDust FYI, here is another approach without using callbacks. Note how I also create a publisher in the test python code rather than calling the CI, which is more conducive to an application where you want to set specific fields in the test.

LastStarDust commented 1 year ago

@Ryanf55 Thank you for letting me know. I have some comments about the code, but before I invest any time in this I want to hear the opinion of the maintainers.

Ryanf55 commented 10 months ago

@Ryanf55 Thank you for letting me know. I have some comments about the code, but before I invest any time in this I want to hear the opinion of the maintainers.

Looks like there are not really any active maintainers who have time to discuss this support. Are you still interested in getting better subscriber testing on humble?

LastStarDust commented 10 months ago

@Ryanf55 for sure I am still interested in getting better subscriber testing on humble.

@project owners If there are no active maintainers or if they have no time for this feature, would you let us take care of this? In other words, would you give @Ryanf55 and @LastStarDust temporary maintainer role?

clalancette commented 10 months ago

If there are no active maintainers or if they have no time for this feature, would you let us take care of this? In other words, would you give @Ryanf55 and @LastStarDust temporary maintainer role?

There are active maintainers of this repository. In general, this is very core ROS 2 stuff, so I don't think giving temporary maintainer role is suitable here.

That said, at the very least a new feature needs to target rolling first. Once this is targeted at rolling, I'll try to get a maintainer to look at it.

LastStarDust commented 10 months ago

If there are no active maintainers or if they have no time for this feature, would you let us take care of this? In other words, would you give @Ryanf55 and @LastStarDust temporary maintainer role?

There are active maintainers of this repository. In general, this is very core ROS 2 stuff, so I don't think giving temporary maintainer role is suitable here.

That said, at the very least a new feature needs to target rolling first. Once this is targeted at rolling, I'll try to get a maintainer to look at it.

Fair enough. BTW my PR is already targeting rolling.