Add ability to configure domain ID #594

Barry-Xu-2018 · 2020-07-02T07:59:46Z

Related to #484

Signed-off-by: Barry Xu <[email protected]>

Barry-Xu-2018 · 2020-07-02T08:15:59Z

After checking failure log for Rpr__rclpy__ubuntu_focal_amd64 — Build finished., I find the error is the same as below.
It seems that test environment has problem.

Traceback:
../../src/rclpy/rclpy/test/__init__.py:23: in <module>
    import rclpy.impl  # noqa
../../src/rclpy/rclpy/rclpy/__init__.py:47: in <module>
    from rclpy.context import Context
../../src/rclpy/rclpy/rclpy/context.py:21: in <module>
    from rclpy.impl.implementation_singleton import rclpy_implementation
../../src/rclpy/rclpy/rclpy/impl/implementation_singleton.py:31: in <module>
    rclpy_implementation = _import('._rclpy')
../../src/rclpy/rclpy/rclpy/impl/__init__.py:28: in _import
    return importlib.import_module(name, package='rclpy')
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'rclpy._rclpy'
E   The C extension '/tmp/ws/src/rclpy/rclpy/rclpy/_rclpy.cpython-38-x86_64-linux-gnu.so' isn't present on the system. Please refer to 'https://index.ros.org/doc/ros2/Troubleshooting/Installation-Troubleshooting/#import-failing-without-library-present-on-the-system' for possible solutions

Signed-off-by: Barry Xu <[email protected]>

fujitatomoya · 2020-07-06T05:34:23Z

rclpy/src/rclpy/_rclpy.c

+ * \return the defined domain id
+ */
+static PyObject *
+rclpy_get_default_domain_id(PyObject * Py_UNUSED(self), PyObject * args)


i was expecting this would call https://github.com/ros2/rcl/blob/6f3c3fb10391edaacb2676e2345a754bef1b76f2/rcl/include/rcl/domain_id.h#L40-L42 to get default domain id from rcl.

This function is called at
https://github.com/Barry-Xu-2018/rclpy/blob/02d0bb6aa33b8e44a97ecac6cae4bbb8fce20f34/rclpy/rclpy/__init__.py#L63-L68.
rcl_get_default_domain_id() may return error.
rclpy_get_default_domain_id() directly return RCL_DEFAULT_DOMAIN_ID defined in rcl layer.

Besides, this domain id (RCL_DEFAULT_DOMAIN_ID) will be set to init_options in rclpy_init() (_rclpy.c)
In rcl_init(), if domain id equal to RCL_DEFAULT_DOMAIN_ID, rcl_get_default_domain_id() will be called.
Refer to https://github.com/ros2/rcl/blob/6f3c3fb10391edaacb2676e2345a754bef1b76f2/rcl/src/rcl/init.c#L146-L154.

So not directly call rcl_get_default_domain_id() here.

that makes sense.

fujitatomoya · 2020-07-06T05:37:27Z

rclpy/rclpy/context.py

+             args: Optional[List[str]] = None,
+             *,
+             initialize_logging: bool = True,
+             domain_id: int = rclpy_implementation.rclpy_get_default_domain_id()):


why call implementation instead of calling utilities.get_default_domain_id?

It is because utilities.py has imported context.py.
https://github.com/Barry-Xu-2018/rclpy/blob/02d0bb6aa33b8e44a97ecac6cae4bbb8fce20f34/rclpy/rclpy/utilities.py#L22

In order to avoid interdependence, have to call rclpy_implementation.

fujitatomoya

it would be better to add some test.

fujitatomoya · 2020-07-07T02:27:29Z

rclpy/src/rclpy/_rclpy.c

+ * \return the defined domain id
+ */
+static PyObject *
+rclpy_get_default_domain_id(PyObject * Py_UNUSED(self), PyObject * args)


that makes sense.

fujitatomoya · 2020-07-07T02:31:45Z

rclpy/rclpy/utilities.py

@@ -39,6 +39,12 @@ def get_default_context(*, shutting_down=False):
        return g_default_context


+def get_default_domain_id():


maybe we could add test in https://github.com/ros2/rclpy/blob/master/rclpy/test/test_utilities.py.

OK. I will supplement test codes.

fujitatomoya · 2020-07-07T02:35:00Z

rclpy/rclpy/__init__.py


 # Avoid loading extensions on module import
 if TYPE_CHECKING:
    from rclpy.executors import Executor  # noqa: F401
    from rclpy.node import Node  # noqa: F401


-def init(*, args: Optional[List[str]] = None, context: Context = None) -> None:
+def init(


Can we add test with specific domain_id in https://github.com/ros2/rclpy/blob/master/rclpy/test/test_init_shutdown.py?

OK. I will supplement test codes.

Barry-Xu-2018 · 2020-07-07T07:34:25Z

#594 (comment)

Sorry. It is my mistake.
The failure is caused from my modification.
I will fix it.

Barry-Xu-2018 · 2020-07-08T02:39:02Z

@fujitatomoya

Sorry.
Since the change is a bit big, I will close this PR and create new PR to review.
New PR is #596.

Add ability to configure domain ID

eeb6f2d

Signed-off-by: Barry Xu <[email protected]>

Barry-Xu-2018 mentioned this pull request Jul 2, 2020

Ability to configure domain ID in rclpy #484

Closed

Barry-Xu-2018 added 3 commits July 3, 2020 10:45

Fix check error from pep257 and flake8

34e4652

Signed-off-by: Barry Xu <[email protected]>

Fix format check problems.

f285855

Signed-off-by: Barry Xu <[email protected]>

Fix a build warning.

02d0bb6

Signed-off-by: Barry Xu <[email protected]>

fujitatomoya reviewed Jul 6, 2020

View reviewed changes

fujitatomoya reviewed Jul 7, 2020

View reviewed changes

Barry-Xu-2018 closed this Jul 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to configure domain ID #594

Add ability to configure domain ID #594

Barry-Xu-2018 commented Jul 2, 2020

Barry-Xu-2018 commented Jul 2, 2020

fujitatomoya Jul 6, 2020

Barry-Xu-2018 Jul 7, 2020

fujitatomoya Jul 7, 2020

fujitatomoya Jul 6, 2020

Barry-Xu-2018 Jul 7, 2020

fujitatomoya left a comment

fujitatomoya Jul 7, 2020

fujitatomoya Jul 7, 2020

Barry-Xu-2018 Jul 7, 2020

fujitatomoya Jul 7, 2020

Barry-Xu-2018 Jul 7, 2020

Barry-Xu-2018 commented Jul 7, 2020

Barry-Xu-2018 commented Jul 8, 2020 •

edited

Loading

		@@ -39,6 +39,12 @@ def get_default_context(*, shutting_down=False):
		return g_default_context


		def get_default_domain_id():

Add ability to configure domain ID #594

Add ability to configure domain ID #594

Conversation

Barry-Xu-2018 commented Jul 2, 2020

Barry-Xu-2018 commented Jul 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fujitatomoya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barry-Xu-2018 commented Jul 7, 2020

Barry-Xu-2018 commented Jul 8, 2020 • edited Loading

Barry-Xu-2018 commented Jul 8, 2020 •

edited

Loading