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

Provide lifecycle services in the rclc lifecycle nodes #51

Merged
merged 28 commits into from
Aug 19, 2021

Conversation

norro
Copy link
Collaborator

@norro norro commented Feb 17, 2021

Provides convenience functions for lifecycle nodes to provide lifecycle services.

micro-ROS#40

Adds an additional callback type and handling that allows passing
additional service context (void *) to a service callback.

ros2#35

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
* Adds user API to add service with additional context
* Tests (copied and adapted from service with request id tests)

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Declares service callbacks for lifecycle services: get state and get available states

#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
@norro norro mentioned this pull request Mar 26, 2021
@JanStaschulat JanStaschulat mentioned this pull request Apr 26, 2021
4 tasks
…vices

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2021

Codecov Report

Merging #51 (f29334e) into master (59632f2) will decrease coverage by 0.64%.
The diff coverage is 41.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   61.53%   60.89%   -0.65%     
==========================================
  Files          13       13              
  Lines        1378     1427      +49     
  Branches      412      424      +12     
==========================================
+ Hits          848      869      +21     
- Misses        343      371      +28     
  Partials      187      187              
Impacted Files Coverage Δ
rclc/src/rclc/executor.c 56.45% <0.00%> (+0.24%) ⬆️
rclc_lifecycle/src/rclc_lifecycle/rclc_lifecycle.c 48.80% <43.39%> (-1.94%) ⬇️
...lc_parameter/src/rclc_parameter/parameter_server.c 68.04% <0.00%> (+2.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59632f2...f29334e. Read the comment docs.

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
* Functions to initialize lifecycle services for nodes
* Exemplary lifecycle nodes with services

#40

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
@norro norro requested a review from pablogs9 August 10, 2021 08:10
@norro
Copy link
Collaborator Author

norro commented Aug 10, 2021

@JanStaschulat can you try to sign commit 7b0111a? This commit is complained about by the DCO check

@JanStaschulat
Copy link
Contributor

JanStaschulat commented Aug 10, 2021

I tried three times. But DCO test did not pass. It seems, that a new commit hash was added instead. I did:

git commit --amend -s -c 7b0111a13552bffbedb9d81971b1f50da0badaac
git push --force-with-lease origin feature/lifecycle-services

I manually set "DCO to pass".

@norro norro self-assigned this Aug 11, 2021
@norro norro added enhancement New feature or request in review Waiting for review (Kanban column) labels Aug 11, 2021
* resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* data_available has to be reset to false, if rcl_take fails - this needs to be updated for subscriptions as well.

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>
Copy link
Contributor

@JanStaschulat JanStaschulat left a comment

Choose a reason for hiding this comment

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

Looks great to me. Just some minor changes:

  • add checks for NULL pointer for all pointer arguments in your functions
  • some functions return void instead of rcl_ret_t. Was this your intention?
  • some minor improvement in rclc_examples and unit tests (simplification of executor initialization)

rclc_examples/src/example_lifecycle_node.c Outdated Show resolved Hide resolved
rclc_examples/src/example_lifecycle_node.c Outdated Show resolved Hide resolved
rclc_examples/src/example_lifecycle_node.c Outdated Show resolved Hide resolved
rclc_examples/src/example_lifecycle_node.c Show resolved Hide resolved
rclc_examples/src/example_lifecycle_node.c Outdated Show resolved Hide resolved
rclc_lifecycle/test/test_lifecycle.cpp Outdated Show resolved Hide resolved
rclc_lifecycle/test/test_lifecycle.cpp Show resolved Hide resolved
rclc_lifecycle/test/test_lifecycle.cpp Show resolved Hide resolved
Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>
@norro
Copy link
Collaborator Author

norro commented Aug 19, 2021

Thank you very much for your thorough review, Jan. I resolved most of the points, just have a question concerning your comment on the return type of my service callbacks.

@JanStaschulat
Copy link
Contributor

JanStaschulat commented Aug 19, 2021

Could not signoff the comments, tried it two times (as suggested from "Details" and with

 git commit --amend --no-edit --signoff -C 7b0111a13552bffbedb9d81971b1f50da0badaac
 git push --force-with-lease origin feature/lifecycle-services

both did not add a signoff message to the commit. Do you have any other suggestion?
For now, I manually "set DCO to pass".

@norro
Copy link
Collaborator Author

norro commented Aug 19, 2021

Unfortunately, I have no better idea. Tried it myself in other PR's as well and failed. So we have to go with setting it manually, I am afraid.

@norro norro added ready Work is about to start (Kanban column) and removed in review Waiting for review (Kanban column) labels Aug 19, 2021
@norro norro linked an issue Aug 19, 2021 that may be closed by this pull request
@JanStaschulat JanStaschulat merged commit 865b02b into ros2:master Aug 19, 2021
@JanStaschulat JanStaschulat deleted the feature/lifecycle-services branch August 19, 2021 10:28
@JanStaschulat
Copy link
Contributor

@mergify backport galactic foxy

mergify bot pushed a commit that referenced this pull request Aug 19, 2021
* Adds service callback with service context

Adds an additional callback type and handling that allows passing
additional service context (void *) to a service callback.

#35

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixed service callback handling

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds user API and tests

* Adds user API to add service with additional context
* Tests (copied and adapted from service with request id tests)

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Proper tear-down of msgs

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixes service call with context tests

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds service callbacks to lifecycle node

Declares service callbacks for lifecycle services: get state and get available states

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds callback for GetState service

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Provide GetAvailableStates service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Example lifecycle node proides services

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Implemented ChangeState service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Linting, documentation, example

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

* Cmake linting

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Adds missing declaration of callbacks

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Pre-initialize all strings

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Lifecycle services working

* Functions to initialize lifecycle services for nodes
* Exemplary lifecycle nodes with services

micro-ROS#40

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Tests for service initialization

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Ignoring unsuccessful SERVICE_TAKE (#175)

* resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* data_available has to be reset to false, if rcl_take fails - this needs to be updated for subscriptions as well.

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Fixes init and cleanup

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

Co-authored-by: Jan Staschulat <jan.staschulat@de.bosch.com>
(cherry picked from commit 865b02b)
mergify bot pushed a commit that referenced this pull request Aug 19, 2021
* Adds service callback with service context

Adds an additional callback type and handling that allows passing
additional service context (void *) to a service callback.

#35

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixed service callback handling

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds user API and tests

* Adds user API to add service with additional context
* Tests (copied and adapted from service with request id tests)

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Proper tear-down of msgs

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixes service call with context tests

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds service callbacks to lifecycle node

Declares service callbacks for lifecycle services: get state and get available states

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds callback for GetState service

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Provide GetAvailableStates service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Example lifecycle node proides services

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Implemented ChangeState service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Linting, documentation, example

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

* Cmake linting

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Adds missing declaration of callbacks

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Pre-initialize all strings

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Lifecycle services working

* Functions to initialize lifecycle services for nodes
* Exemplary lifecycle nodes with services

micro-ROS#40

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Tests for service initialization

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Ignoring unsuccessful SERVICE_TAKE (#175)

* resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* data_available has to be reset to false, if rcl_take fails - this needs to be updated for subscriptions as well.

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Fixes init and cleanup

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

Co-authored-by: Jan Staschulat <jan.staschulat@de.bosch.com>
(cherry picked from commit 865b02b)

# Conflicts:
#	rclc_lifecycle/src/rclc_lifecycle/rclc_lifecycle.c
#	rclc_lifecycle/test/test_lifecycle.cpp
@mergify
Copy link
Contributor

mergify bot commented Aug 19, 2021

Command backport galactic foxy: success

Backports have been created

Hey, I reacted but my real name is @Mergifyio

JanStaschulat pushed a commit that referenced this pull request Aug 19, 2021
* Adds service callback with service context

Adds an additional callback type and handling that allows passing
additional service context (void *) to a service callback.

#35

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixed service callback handling

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds user API and tests

* Adds user API to add service with additional context
* Tests (copied and adapted from service with request id tests)

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Proper tear-down of msgs

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Fixes service call with context tests

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds service callbacks to lifecycle node

Declares service callbacks for lifecycle services: get state and get available states

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Adds callback for GetState service

micro-ROS#40

Signed-off-by: Arne Nordmann <arne.nordmann@de.bosch.com>

* Provide GetAvailableStates service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Example lifecycle node proides services

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Implemented ChangeState service

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Linting, documentation, example

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

* Cmake linting

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Adds missing declaration of callbacks

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Pre-initialize all strings

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Lifecycle services working

* Functions to initialize lifecycle services for nodes
* Exemplary lifecycle nodes with services

micro-ROS#40

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* Tests for service initialization

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Ignoring unsuccessful SERVICE_TAKE (#175)

* resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* data_available has to be reset to false, if rcl_take fails - this needs to be updated for subscriptions as well.

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

* Fixes init and cleanup

Signed-off-by: Nordmann Arne (CR/ADT3) <arne.nordmann@de.bosch.com>

* commnted out check for received sequence id - error in build-job for rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR #46 and issue #49

Signed-off-by: Jan Staschulat <jan.staschulat@de.bosch.com>

Co-authored-by: Jan Staschulat <jan.staschulat@de.bosch.com>
(cherry picked from commit 865b02b)

Co-authored-by: Arne Nordmann <arne.nordmann@de.bosch.com>
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-humble-hawksbill-released/25729/14

MiguelCompany pushed a commit to eProsima/rclc that referenced this pull request Jun 6, 2023
* Update CI


Syntax


Update source


Fix

* Add pedatic

* add rolling from binaries


Update


Update

* Updating to Ralph approach

* Added eloquent and dashing

* Added dockers


Updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready Work is about to start (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[lifecycle] provide lifecycle services
5 participants