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

avoid custom working directory #147

Merged
merged 1 commit into from
Nov 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions rclpy/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,16 @@ if(WIN32 AND CMAKE_BUILD_TYPE STREQUAL "Debug")
set(PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE_DEBUG}")
endif()

# enables using the Python package from the source space
# This creates a folder in ${CMAKE_CURRENT_BINARY_DIR}/rclpy which has all the
# as well as the compiled modules from the build space
# necessary Python code and C Python extensions for running tests.
configure_file("__init__.py.in" "rclpy/__init__.py" @ONLY)
# enables using the Python extensions from the build space for testing
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test_rclpy/__init__.py" "")

ament_python_install_package(${PROJECT_NAME})

function(set_properties _targetname _build_type)
set_target_properties(${_targetname} PROPERTIES
PREFIX ""
LIBRARY_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}"
RUNTIME_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}"
LIBRARY_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/test_${PROJECT_NAME}"
RUNTIME_OUTPUT_DIRECTORY${_build_type} "${CMAKE_CURRENT_BINARY_DIR}/test_${PROJECT_NAME}"
Copy link
Member

Choose a reason for hiding this comment

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

It looks a bit weird to me that if I build without build_testing all my rclpy libraries go to this test_rclpy folder. Then they'll be install at the right place at the end of the day so I guess it's fine

Copy link
Member Author

Choose a reason for hiding this comment

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

The directory can have any name except rclpy. I used the test_ prefix since it is being used during testing. But any other name (which isn't likely to collide with other packages) would be fine.

OUTPUT_NAME "_${_targetname}${PythonExtra_EXTENSION_SUFFIX}"
SUFFIX "${PythonExtra_EXTENSION_EXTENSION}")
endfunction()
Expand Down Expand Up @@ -109,9 +106,9 @@ if(BUILD_TESTING)
endif()
if(NOT _typesupport_impls STREQUAL "")
ament_add_pytest_test(rclpytests test
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE}"
APPEND_ENV AMENT_PREFIX_PATH=${ament_index_build_path}
PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}
Copy link
Member

Choose a reason for hiding this comment

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

weird indent ?

Copy link
Member Author

Choose a reason for hiding this comment

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

PYTHONPATH is a value to the multi-value argument APPEND_ENV. Therefore I used the extra one level indentation. If anyone would like to change this please feel free to do so.

TIMEOUT 90
)
endif()
Expand Down
35 changes: 0 additions & 35 deletions rclpy/__init__.py.in

This file was deleted.

18 changes: 12 additions & 6 deletions rclpy/rclpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,29 @@

import sys

from rclpy.executors import SingleThreadedExecutor as _SingleThreadedExecutor
from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy
from rclpy.node import Node
from rclpy.utilities import get_rmw_implementation_identifier # noqa
from rclpy.utilities import ok
from rclpy.utilities import shutdown # noqa
from rclpy.utilities import try_shutdown # noqa


def init(*, args=None):
return _rclpy.rclpy_init(args if args is not None else sys.argv)
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
return rclpy_implementation.rclpy_init(
args if args is not None else sys.argv)


def create_node(node_name, *, namespace=None):
# imported locally to avoid loading extensions on module import
from rclpy.node import Node
Copy link
Contributor

Choose a reason for hiding this comment

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

Is delaying the import a necessary change with the new approach, or an additional feature?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is necessary since the node module already imports the singleton module. But we need to delay the instantiation of the singletons until we had a chance to override the _import functions.

return Node(node_name, namespace=namespace)


def spin_once(node, *, timeout_sec=None):
executor = _SingleThreadedExecutor()
# imported locally to avoid loading extensions on module import
from rclpy.executors import SingleThreadedExecutor
executor = SingleThreadedExecutor()
try:
executor.add_node(node)
executor.spin_once(timeout_sec=timeout_sec)
Expand All @@ -41,7 +45,9 @@ def spin_once(node, *, timeout_sec=None):


def spin(node):
executor = _SingleThreadedExecutor()
# imported locally to avoid loading extensions on module import
from rclpy.executors import SingleThreadedExecutor
executor = SingleThreadedExecutor()
try:
executor.add_node(node)
while ok():
Expand Down
28 changes: 28 additions & 0 deletions rclpy/rclpy/impl/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2016-2017 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import importlib
import os


def _import(name):
try:
return importlib.import_module(name, package='rclpy')
except ImportError as e:
if e.path is not None and os.path.isfile(e.path):
e.msg += \
"\nThe C extension '%s' failed to be imported while being present on the system." \
" Please refer to '%s' for possible solutions" % \
(e.path, 'https://github.com/ros2/ros2/wiki/Rclpy-Import-error-hint')
raise
15 changes: 3 additions & 12 deletions rclpy/rclpy/impl/implementation_singleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,7 @@
# ...
"""

import importlib
import os
from rclpy.impl import _import

try:
rclpy_implementation = importlib.import_module('._rclpy', package='rclpy')
rclpy_logging_implementation = importlib.import_module('._rclpy_logging', package='rclpy')
except ImportError as e:
if e.path is not None and os.path.isfile(e.path):
e.msg += \
"\nThe C extension '%s' failed to be imported while being present on the system." \
" Please refer to '%s' for possible solutions" % \
(e.path, 'https://github.com/ros2/ros2/wiki/Rclpy-Import-error-hint')
raise
rclpy_implementation = _import('._rclpy')
rclpy_logging_implementation = _import('._rclpy_logging')
20 changes: 13 additions & 7 deletions rclpy/rclpy/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,33 @@

import threading

from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy

g_shutdown_lock = threading.Lock()


def ok():
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with g_shutdown_lock:
return _rclpy.rclpy_ok()
return rclpy_implementation.rclpy_ok()


def shutdown():
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with g_shutdown_lock:
return _rclpy.rclpy_shutdown()
return rclpy_implementation.rclpy_shutdown()


def try_shutdown():
"""Shutdown rclpy if not already shutdown."""
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
with g_shutdown_lock:
if _rclpy.rclpy_ok():
return _rclpy.rclpy_shutdown()
if rclpy_implementation.rclpy_ok():
return rclpy_implementation.rclpy_shutdown()


def get_rmw_implementation_identifier():
return _rclpy.rclpy_get_rmw_implementation_identifier()
# imported locally to avoid loading extensions on module import
from rclpy.impl.implementation_singleton import rclpy_implementation
return rclpy_implementation.rclpy_get_rmw_implementation_identifier()
17 changes: 12 additions & 5 deletions rclpy/test/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2016 Open Source Robotics Foundation, Inc.
# Copyright 2016-2017 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -12,11 +12,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import os
import importlib
import sys

assert 'rclpy' not in sys.modules, 'rclpy should not have been imported before running tests'
sys.path.insert(0, os.getcwd())

import rclpy # noqa
assert rclpy # silence pyflakes
# this will make the extensions load from the build folder
import rclpy.impl # noqa
import test_rclpy # noqa


def _custom_import(name):
return importlib.import_module(name, package='test_rclpy')


rclpy.impl._import = _custom_import