Skip to content
This repository has been archived by the owner on Jan 27, 2020. It is now read-only.

Improve apex launchtest ros #29

Merged
merged 8 commits into from
Apr 3, 2019
Merged

Conversation

pbaughman
Copy link
Contributor

@pbaughman pbaughman commented Mar 13, 2019

Add an apex_launchtest_ros example that shows how to interact with the talker and listener nodes in an integration test. Remove the old example nodes in favor of standard ROS2 example nodes from the demo_nodes_py package

Fixes #26

@pbaughman pbaughman force-pushed the improve_apex_launchtest_ros branch 2 times, most recently from e29c627 to fc4ff47 Compare March 13, 2019 20:38
executor = rclpy.executors.SingleThreadedExecutor(context=self.context)
executor.add_node(self.node)
try:
executor.spin_once(timeout_sec=timeout_sec)
Copy link
Contributor Author

@pbaughman pbaughman Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the launch description has any lifecycle nodes in it, it will also require a ROSSpecificLaunchStartup in the launch description. This starts a thread that will spin in the background and will conflict with this spin and crash the test until this issue is resolved

This example doesn't use lifecycle nodes, so it'll launch without the ROSSpecificLaunchStartup

@pbaughman
Copy link
Contributor Author

@dejanpan This is a lower priority PR, but it shows how to do some simple ROS testing with apex_launchtest. The important file is apex_launchtest_ros/examples/talker_listener.test.py. Can you glance at this and approve it?

@pbaughman pbaughman force-pushed the improve_apex_launchtest_ros branch from 634dac9 to e2ae424 Compare March 19, 2019 17:47
@pbaughman pbaughman added the Needs Upstream Merge For PRs merged after apex_rostest was copied to ros2/launch label Mar 22, 2019
@pbaughman pbaughman force-pushed the improve_apex_launchtest_ros branch 2 times, most recently from 9b0329b to 5b7b92b Compare March 26, 2019 23:32
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM

"""
return len(self.__republished_list)

def get_republished(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pbaughman just curious, any reason not to use @property instead of getters?

Copy link
Contributor Author

@pbaughman pbaughman Mar 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hidmic Old habit - In a previous life I had a blanket ban on python properties because they were difficult to mock. Especially boolean properties. You would mock.patch them and then no matter what you set the return value to to they would all silently evaluate to 'True.' That combined with poor negative testing = bugs bugs bugs.

apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
apex_launchtest_ros/examples/README.md Outdated Show resolved Hide resolved
@pbaughman pbaughman force-pushed the improve_apex_launchtest_ros branch from 5b7b92b to c101b25 Compare March 27, 2019 20:14
@pbaughman
Copy link
Contributor Author

@hidmic It reset your approval when I accepted your suggested edits - can you re-approve? Sorry - thanks

@pbaughman pbaughman added the enhancement New feature or request label Mar 28, 2019
Pete Baughman and others added 7 commits April 1, 2019 13:16
Fix Typo

Co-Authored-By: pbaughman <dblue135@yahoo.com>
Improve sentence

Co-Authored-By: pbaughman <dblue135@yahoo.com>
Fix bad spelling

Co-Authored-By: pbaughman <dblue135@yahoo.com>
formatting

Co-Authored-By: pbaughman <dblue135@yahoo.com>
Formatting

Co-Authored-By: pbaughman <dblue135@yahoo.com>
@pbaughman pbaughman force-pushed the improve_apex_launchtest_ros branch from c078a61 to ca5878b Compare April 1, 2019 20:16
Signed-off-by: Pete Baughman <pete.baughman@apex.ai>
@pbaughman pbaughman merged commit dac7c65 into master Apr 3, 2019
@pbaughman pbaughman deleted the improve_apex_launchtest_ros branch April 3, 2019 23:03
@pbaughman pbaughman removed the Needs Upstream Merge For PRs merged after apex_rostest was copied to ros2/launch label May 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apex_launchtest_ros examples need some TLC
3 participants