Skip to content

Dashing migration#11

Merged
BorjaOuterelo merged 21 commits intodashingfrom
feature/dashing_migration
Jan 28, 2020
Merged

Dashing migration#11
BorjaOuterelo merged 21 commits intodashingfrom
feature/dashing_migration

Conversation

@julionce
Copy link
Copy Markdown

@julionce julionce commented Aug 6, 2019

This pull request adapts these demos to Dashing release.
These demos has been reorganized in different folder c -> rclc and cpp -> rclcpp.
rcl porting has not been done yet.

Comment thread rcl/int32_publisher/package.xml Outdated
<version>0.0.1</version>
<description>Example of a publisher using int32</description>
<maintainer email="julianbermudez@eprosima.com">Julian Bermudez</maintainer>
<license>Apache License 2.0</license>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<license>Apache License 2.0</license>
<license>Apache-2.0</license>

Cf. previous comment and https://spdx.org/licenses/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the information. Change!

Comment thread rcl/int32_subscriber/main.c Outdated
}
} while (rv);

rv = rcl_subscription_fini(&subscription, &node);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
rv = rcl_subscription_fini(&subscription, &node);
rcl_subscription_fini(&subscription, &node);

rv is not used.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is outdated, new we use macros for return-value-checking.

Comment thread rcl/int32_subscriber/main.c Outdated
} while (rv);

rv = rcl_subscription_fini(&subscription, &node);
rv = rcl_node_fini(&node);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
rv = rcl_node_fini(&node);
rcl_node_fini(&node);

rv is not used.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is outdated, new we use macros for return-value-checking.

Comment thread rclcpp/complex_msg_publisher/main.cpp Outdated
rclcpp::QoS qos(10);
publisher_ = this->create_publisher<complex_msgs::msg::NestedMsgTest>(
"complex_msgs_msg_NestedMsgTest");
"complex_msgs_msg_NestedMsgTest", qos);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You may want to use rclcpp::SystemDefaultsQoS() instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Change!

Comment thread rclcpp/complex_msg_subscriber/main.cpp Outdated
rclcpp::QoS qos(10);
subscription_ = this->create_subscription<complex_msgs::msg::NestedMsgTest>(
"complex_msgs_msg_NestedMsgTest",
"complex_msgs_msg_NestedMsgTest", qos,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Possible to use rclcpp::SystemDefaultsQoS() instead?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed!

Copy link
Copy Markdown
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Looks good to me. I like the proper renaming of the packages with suffixes _rclcpp or _rclc. In the next months, we should get rid of rclc completely.

BorjaOuterelo and others added 9 commits October 20, 2019 18:46
* Add services examples

* Refactor pub/sub examples

* Add action server example

* Action server updated

* Add action client example

* Adjust action client and server demos

* Indentation fix

* Ensure finalizing elements

* Name typo

* Ensure fini elements

* Using RCL_MS_TO_NS function in rcl_wait

* Style consistency

* Independent action client event checking

* Style fix

* Error handling

* Typo

* Added multinode example

Check required

* Sleep to wait matching comment

* Indentation fix

*  Return codes removed
* Refactor add two ints requester

* Style refactor and macro rc check

* Typos

* Typos

* rc variable independace

* Typo
* Add configuration examples.

* Added configuration examples

* Client key API

* Fix

* Examples cleanup ad error code check

* Added error check macros

* Indentation fix

* Indentation fix

* Indentation fix

Co-authored-by: Borja Outerelo <borjaouterelo@gmail.com>
@julionce julionce changed the base branch from develop to dashing January 28, 2020 09:00
@julionce julionce requested a review from ralph-lange January 28, 2020 09:10
@BorjaOuterelo BorjaOuterelo merged commit 5d7b442 into dashing Jan 28, 2020
@Acuadros95 Acuadros95 deleted the feature/dashing_migration branch June 6, 2023 14:38
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.

4 participants