Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[noetic] Fix install and improve Python 3 support + python2.7 StringIO fix #97

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

stwirth
Copy link

@stwirth stwirth commented Jan 25, 2022

Contains @mikaelarguedas fixes from #94 plus a few commits to add a python2.7 compatible way of using StringIO and make the bionic build pass.
Also fixes some package.xml/CMakeLists.txt errors that are reported by catkin_lint.

@stwirth
Copy link
Author

stwirth commented Jan 25, 2022

Melodic build failed with

03:13:08 ERROR: max time [60.0s] allotted for test [ros_services] of type [capabilities/test_ros_services.py]
03:13:08   File "/usr/lib/python2.7/unittest/case.py", line 329, in run
03:13:08     testMethod()
03:13:08   File "/opt/ros/melodic/lib/python2.7/dist-packages/rostest/runner.py", line 148, in fn
03:13:08     self.test_parent.run_test(test)
03:13:08   File "/opt/ros/melodic/lib/python2.7/dist-packages/rostest/rostest_parent.py", line 132, in run_test
03:13:08     return self.runner.run_test(test)
03:13:08   File "/opt/ros/melodic/lib/python2.7/dist-packages/roslaunch/launch.py", line 689, in run_test
03:13:08     (test.time_limit, test.test_name, test.package, test.type))
03:13:08 --------------------------------------------------------------------------------
03:13:08 
03:13:08 [ROSTEST]-----------------------------------------------------------------------

Maybe it just didn't get enough CPU.

@ros-pull-request-builder retest this please

@stwirth
Copy link
Author

stwirth commented Jan 25, 2022

Second build failed with

12:29:33 [capabilities.rosunit-default_provider/test_default_provider][FAILURE]----------
12:29:33 File "/usr/lib/python2.7/unittest/case.py", line 329, in run
12:29:33     testMethod()
12:29:33   File "/tmp/ws/src/capabilities/test/rostest/test_server/test_default_provider.py", line 19, in test_default_provider
12:29:33     assert wait_for_capability_server(10)
12:29:33 --------------------------------------------------------------------------------
12:29:33 

@stwirth
Copy link
Author

stwirth commented Jan 25, 2022

All tests pass for me on 16.04/kinetic BTW.

@stwirth
Copy link
Author

stwirth commented Jan 25, 2022

@ros-pull-request-builder retest this please

Fixes the following errors reported by catkin_lint:
capabilities: error: build_export_depend 'message_runtime' is not listed in catkin_package()
capabilities: error: message dependency 'std_srvs' is not listed in catkin_package()
capabilities: error: message dependency 'std_msgs' is not listed in catkin_package()
capabilities: error: message dependency 'std_srvs' is not listed as build_export_depend
capabilities: error: message dependency 'std_msgs' is not listed as build_export_depend
@stwirth
Copy link
Author

stwirth commented Jan 25, 2022

The bionic tests keep failing with:

13:26:41 [capabilities.rosunit-client/test_use_and_free_capability][FAILURE]-------------
13:26:41 File "/usr/lib/python2.7/unittest/case.py", line 329, in run
13:26:41     testMethod()
13:26:41   File "/tmp/ws/src/capabilities/test/rostest/test_server/test_client.py", line 35, in test_use_and_free_capability
13:26:41     assert wait_for_capability_server(30)
13:26:41 --------------------------------------------------------------------------------

and

13:27:26 [capabilities.rosunit-default_provider/test_default_provider][FAILURE]----------
13:27:26 File "/usr/lib/python2.7/unittest/case.py", line 329, in run
13:27:26     testMethod()
13:27:26   File "/tmp/ws/src/capabilities/test/rostest/test_server/test_default_provider.py", line 19, in test_default_provider
13:27:26     assert wait_for_capability_server(30)
13:27:26 --------------------------------------------------------------------------------

even though I increased the timeout to 30s.

From the log times it looks like it is indeed waiting the 30s before failing:

13:26:55 �]2;/tmp/ws/src/capabilities/src/capabilities/capability_server_nodelet_manager.launch�
13:26:55 �[1mstarted roslaunch server http://cf9aa1763d55:34951/�[0m
13:26:55 
13:26:55 SUMMARY
13:26:55 ========
13:26:55 
13:26:55 PARAMETERS
13:26:55  * /rosdistro: melodic
13:26:55  * /rosversion: 1.14.12
13:26:55 
13:26:55 NODES
13:26:55   /
13:26:55     capability_server_nodelet_manager (nodelet/nodelet)
13:26:55 
13:26:55 �[1mROS_MASTER_URI=http://cf9aa1763d55:38519/�[0m
13:26:55 �]2;/tmp/ws/src/capabilities/src/capabilities/capability_server_nodelet_manager.launch http://cf9aa1763d55:38519/�
13:26:55 �[1mprocess[capability_server_nodelet_manager-1]: started with pid [1275]�[0m
13:26:56 [DEBUG] [1643113645.793882]: connecting to cf9aa1763d55 44723
13:27:25 [capability_server_nodelet_manager-1] killing on exit
13:27:25 �[0m[ INFO] [1643113616.146610651]: Initializing nodelet with 4 worker threads.�[0m
13:27:26 shutting down processing monitor...
13:27:26 ... shutting down processing monitor complete
13:27:26 �[1mdone�[0m
13:27:26 [Testcase: testdefault_provider] ... ok
13:27:26 
13:27:26 [ROSTEST]-----------------------------------------------------------------------
13:27:26 
13:27:26 [capabilities.rosunit-default_provider/test_default_provider][FAILURE]----------
13:27:26 File "/usr/lib/python2.7/unittest/case.py", line 329, in run
13:27:26     testMethod()
13:27:26   File "/tmp/ws/src/capabilities/test/rostest/test_server/test_default_provider.py", line 19, in test_default_provider
13:27:26     assert wait_for_capability_server(30)
13:27:26 --------------------------------------------------------------------------------
13:27:26 
13:27:26 
13:27:26 SUMMARY
13:27:26 �[1;31m * RESULT: FAIL�[0m
13:27:26  * TESTS: 1
13:27:26  * ERRORS: 0
13:27:26 �[1;31m * FAILURES: 1�[0m

@wjwwood any ideas?

@stwirth stwirth marked this pull request as draft January 25, 2022 12:38
@stwirth
Copy link
Author

stwirth commented Jan 27, 2022

@wjwwood PTAL, I got everything passing now.
I think the failures listed above were due to a race condition, the capability server started faster than the test node and the SERVER_READY message was sent before the test node subscribed. 148d629 makes that message latched which solves this race.

@stwirth stwirth marked this pull request as ready for review January 27, 2022 13:50
@@ -380,7 +380,7 @@ def spin(self):
self.handle_get_remappings)

rospy.loginfo("Capability Server Ready")
rospy.Publisher("~events", CapabilityEvent, queue_size=1000).publish(
rospy.Publisher("~events", CapabilityEvent, queue_size=1000, latch=True).publish(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure latching is the right solution here. The problem is that these events are meant to be ephemeral. Consider the situation where a subscription to the event is created long after the server started, it will always receive the last event even if it didn't happen recently. This might be the SEVER_READY event, which might be harmless, but it might also be some other event. The way the publishers work in rospy, is that it's a singleton, so if you set it latched here, I believe it will affect all other instances of this publisher for this topic.

I think a better solution would be to either periodically publish this event (or a "still alive" heart beat), or create a new service that can be called at anytime to see if the server is ready, and have the client call that in case it missed the event.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I have only seen one publish call in this file so I thought this would be fine. Now I see the topic is also used in launch_manager.py.
I don't want to dig too deep here. I'll try the periodically publishing.

@stwirth
Copy link
Author

stwirth commented Feb 1, 2022

@wjwwood I implemented the heartbeat (publishing SERVER_READY every 1s (configurable)) and tests pass.
The noetic / debian buster build fails with something unrelated:

14:20:23 Crawling for packages in workspace '/tmp/ws/src'
14:20:23 Found the following packages:
14:20:23   - capabilities
14:20:23 Package maintainer emails: [email protected]
14:20:24 Resolved the dependencies to the following binary packages:
14:20:24   - ros-noetic-catkin
14:20:24 Always install the following generic dependencies:
14:20:24   - build-essential
14:20:24   - python3
14:20:24   - ros-noetic-catkin
14:20:24 Identified the following build dependencies (ignoring packages available from source):
14:20:24   - catkin
14:20:24   - message_generation
14:20:24   - roslaunch
14:20:24   - rospy
14:20:24   - rostest
14:20:24   - std_msgs
14:20:24   - std_srvs
14:20:24 Resolved the dependencies to the following binary packages:
14:20:24   - ros-noetic-catkin
14:20:24   - ros-noetic-message-generation
14:20:24   - ros-noetic-roslaunch
14:20:24   - ros-noetic-rospy
14:20:24   - ros-noetic-rostest
14:20:24   - ros-noetic-std-msgs
14:20:24   - ros-noetic-std-srvs
14:20:24 Traceback (most recent call last):
14:20:24   File "/usr/lib/python3/dist-packages/apt/cache.py", line 297, in __getitem__
14:20:24     rawpkg = self._cache[key]
14:20:24 KeyError: 'ros-noetic-catkin'
14:20:24 
14:20:24 During handling of the above exception, another exception occurred:
14:20:24 
14:20:24 Traceback (most recent call last):
14:20:24   File "/tmp/ros_buildfarm/scripts/devel/create_devel_task_generator.py", line 304, in <module>
14:20:24     main()
14:20:24   File "/tmp/ros_buildfarm/scripts/devel/create_devel_task_generator.py", line 140, in main
14:20:24     get_binary_package_versions(apt_cache, debian_pkg_names))
14:20:24   File "/tmp/ros_buildfarm/ros_buildfarm/common.py", line 175, in get_binary_package_versions
14:20:24     pkg = apt_cache[debian_pkg_name]
14:20:24   File "/usr/lib/python3/dist-packages/apt/cache.py", line 299, in __getitem__
14:20:24     raise KeyError('The cache has no package named %r' % key)
14:20:24 KeyError: "The cache has no package named 'ros-noetic-catkin'"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants