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

Adds a context pointer to subscriptions #107

Merged
merged 23 commits into from
Jul 13, 2021

Conversation

BrettRD
Copy link
Contributor

@BrettRD BrettRD commented Jun 30, 2021

This pull request adds a subscription callback carrying a context pointer
This pull request includes all of the changes from from #106, it might be saner if that happens first

@BrettRD BrettRD changed the base branch from master to foxy June 30, 2021 01:59
@JanStaschulat
Copy link
Contributor

JanStaschulat commented Jun 30, 2021

@BrettRD Please add yourself in the NOTICE file.

@JanStaschulat
Copy link
Contributor

JanStaschulat commented Jul 5, 2021

Could you pull foxy branch (with merged PR #106) into this one? This will start the ci-jobs again.

BrettRD and others added 15 commits July 6, 2021 00:43
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
@BrettRD
Copy link
Contributor Author

BrettRD commented Jul 5, 2021

I tried to rebase onto foxy to satisfy DCO but it reverted a few things.
Fixing that now

@BrettRD BrettRD marked this pull request as draft July 5, 2021 14:51
BrettRD added 4 commits July 6, 2021 11:47
This reverts commit 0ed3bf0.

Signed-off-by: BrettRD <[email protected]>
This reverts commit a201b6d.

Signed-off-by: BrettRD <[email protected]>
Signed-off-by: BrettRD <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2021

Codecov Report

Merging #107 (d75c2f0) into foxy (1a6a50b) will decrease coverage by 0.40%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             foxy     #107      +/-   ##
==========================================
- Coverage   29.08%   28.68%   -0.41%     
==========================================
  Files          27       28       +1     
  Lines        3060     3246     +186     
  Branches     1620     1679      +59     
==========================================
+ Hits          890      931      +41     
- Misses        814      914     +100     
- Partials     1356     1401      +45     
Impacted Files Coverage Δ
...os_ws/src/rxlgfln327g/rclc/rclc/src/rclc/service.c
...gfln327g/rclc/rclc/test/rclc/test_subscription.cpp
...ln327g/rclc/rclc_lifecycle/test/test_lifecycle.cpp
.../rxlgfln327g/rclc/rclc/test/rclc/test_executor.cpp
ros_ws/src/rxlgfln327g/rclc/rclc/src/rclc/client.c
...s_ws/src/rxlgfln327g/rclc/rclc/src/rclc/executor.c
.../src/rxlgfln327g/rclc/rclc/src/rclc/subscription.c
...7g/rclc/rclc_examples/src/example_lifecycle_node.c
...src/rxlgfln327g/rclc/rclc/test/rclc/test_timer.cpp
...n327g/rclc/rclc/test/rclc/test_executor_handle.cpp
... and 45 more

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 1a6a50b...d75c2f0. Read the comment docs.

@BrettRD BrettRD marked this pull request as ready for review July 6, 2021 01:58
BrettRD added 2 commits July 6, 2021 12:01
Signed-off-by: BrettRD <[email protected]>
@BrettRD
Copy link
Contributor Author

BrettRD commented Jul 6, 2021

I think this feature would benefit from a test that checks context is only passed when the subscription requests it

} else {
handle->callback(NULL);
switch (handle->callback_type) {
case CB_WITHOUT_REQUEST_ID:
Copy link
Member

Choose a reason for hiding this comment

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

Should this be something like CB_WITHOUT_CONTEXT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

CB_WITHOUT_REQUEST_ID comes from when request_id was added to services in Nov2020,
4bc9c13#diff-682b2c9ebab8a01f15a96d651ebc0e2aec2748f7abfafe288a48f9eb9fcc8786R48-R53
CB_WITHOUT_REQUEST_ID was treated like a default callback_type for the original api. It made sense when the request ID was the only special case, and only for services

I don't think it needs another callback-type, just a better name.

handle.callback_type is never checked without first checking handle.type, so I can also see an argument for eliminating handle.callback_type and extending handle.type to match 1:1 with all the types in the handle's function-pointer union:

union {
rclc_callback_t callback;
rclc_subscription_callback_with_context_t subscription_callback_with_context;
rclc_service_callback_t service_callback;
rclc_service_callback_with_request_id_t service_callback_with_reqid;
rclc_service_callback_with_context_t service_callback_with_context;
rclc_client_callback_t client_callback;
rclc_client_callback_with_request_id_t client_callback_with_reqid;
rclc_gc_callback_t gc_callback;
};

Copy link
Contributor

@JanStaschulat JanStaschulat Jul 9, 2021

Choose a reason for hiding this comment

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

Yes, I agree. Initally, we only had services, subscriptions, clients. Now we have multiple types of callbacks for subscriptions, services and clients:

  • with request id/without request id (for services/clients),
  • with context, without context (for subscriptions/clients/services).

It would make sense to refactor handle->type and add e..g
SERVICE_WITH_REQ_ID
SERVICE_WITHOUT_REQ_ID
SERVICE_WITHOUT_CONTEXT
SERVICE_WITH_CONTEXT
SUBSCRIPTION_WITH_CONTEXT
SUBSCRIPTION_WITHOUT_CONTEXT
same for CLIENTS ...
(quite a number of types ...)

the request-id is currently not used in services with context.
@norro @pablogs9 @BrettRD I propose to keep the structure like this in this pull request. Opened a new issue for discussing how to simplify callback-type and handle-type variables.
Issue: #116

@norro. It looks like that the implementation of the client callback with context is not properly called: it only calls the client callback without context:

handle->client_callback(handle->data);

You never use a client callback with context?

Copy link
Contributor

Choose a reason for hiding this comment

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

Added an issue for that :#115

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. Please just add test cases with the the executor_spin_some and an example in rclc_examples.

rc = rclc_executor_fini(&executor);
EXPECT_EQ(RCL_RET_OK, rc) << rcl_get_error_string().str;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

would need a test case where you add a subscription with a context, publish data and check if the data was correctly handled, like this one for service_with_context

TEST_F(TestDefaultExecutor, executor_test_service_with_context) {

or a simplified version of this test case spin_some_sequential_execution

TEST_F(TestDefaultExecutor, spin_some_sequential_execution) {

in your test case you only need one subsction. e.g. add_subscription to executor , publish a topic, wait for some time, call spin_some and then copy the received data and the context data in the subscription_with_context_callback to a global variable. These ones you can then check in the test case.

Would be great, if you could add an example in rclc_examples repository that shows how to use this feature. You could use this one as a base-line
https://github.com/ros2/rclc/blob/master/rclc_examples/src/example_executor_convenience.c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test added, Example added.
I don't think I can reliably test that context is not passed to a regular subscription; That would need a very carefully crafted stack canary, and it would be quite brittle to nearby changes.

@@ -222,6 +222,7 @@ rclc_executor_add_subscription(
executor->handles[executor->index].subscription = subscription;
executor->handles[executor->index].data = msg;
executor->handles[executor->index].callback = callback;
executor->handles[executor->index].callback_type = CB_WITHOUT_REQUEST_ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you want to change this? Default is CB_UNDEFINED

The type CB_WITH_REQUEST_ID and CB_WITHOUT_REQUEST_IDis currently used for services/clients.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also prefer CB_WITHOUT_CONTEXT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The subscriptions without context need something in their callback_type that is well-defined, but has neither request-id nor context. CB_WITHOUT_CONTEXT sounds more generic, but is no more informative than CB_WITHOUT_REQUEST_ID, and would require another enum value.
(excuse me if this argument is already settled elsewhere, I'm catching up on the recent activity)

Copy link
Contributor

@JanStaschulat JanStaschulat Jul 9, 2021

Choose a reason for hiding this comment

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

Sure, but CB_WITHOUT_REQUEST_ID only makes sense for services and clients. CB_WITHOUT_CONTEXT is informative, that it is a normal callback. If you prefer another wording, how about CB_DEFAULT as a neutral enum value.

} else {
handle->callback(NULL);
switch (handle->callback_type) {
case CB_WITHOUT_REQUEST_ID:
Copy link
Contributor

@JanStaschulat JanStaschulat Jul 9, 2021

Choose a reason for hiding this comment

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

Yes, I agree. Initally, we only had services, subscriptions, clients. Now we have multiple types of callbacks for subscriptions, services and clients:

  • with request id/without request id (for services/clients),
  • with context, without context (for subscriptions/clients/services).

It would make sense to refactor handle->type and add e..g
SERVICE_WITH_REQ_ID
SERVICE_WITHOUT_REQ_ID
SERVICE_WITHOUT_CONTEXT
SERVICE_WITH_CONTEXT
SUBSCRIPTION_WITH_CONTEXT
SUBSCRIPTION_WITHOUT_CONTEXT
same for CLIENTS ...
(quite a number of types ...)

the request-id is currently not used in services with context.
@norro @pablogs9 @BrettRD I propose to keep the structure like this in this pull request. Opened a new issue for discussing how to simplify callback-type and handle-type variables.
Issue: #116

@norro. It looks like that the implementation of the client callback with context is not properly called: it only calls the client callback without context:

handle->client_callback(handle->data);

You never use a client callback with context?

} else {
handle->callback(NULL);
switch (handle->callback_type) {
case CB_WITHOUT_REQUEST_ID:
Copy link
Contributor

Choose a reason for hiding this comment

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

Added an issue for that :#115

@JanStaschulat
Copy link
Contributor

JanStaschulat commented Jul 9, 2021

You can ignore the failing build on Dashing. I already updated the "required checks" settings for Foxy, But, I guess, it does not take effect here, as the PR had already been created.

@JanStaschulat
Copy link
Contributor

@mergify backport galactic master

@JanStaschulat JanStaschulat merged commit 5b11c5a into ros2:foxy Jul 13, 2021
mergify bot pushed a commit that referenced this pull request Jul 13, 2021
(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt
mergify bot pushed a commit that referenced this pull request Jul 13, 2021
(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt
@mergify
Copy link
Contributor

mergify bot commented Jul 13, 2021

Command backport galactic master: success

Backports have been created

Hey, I reacted but my real name is @Mergifyio

JanStaschulat added a commit that referenced this pull request Jul 15, 2021
* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge conflict.

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>
JanStaschulat added a commit that referenced this pull request Jul 15, 2021
* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge confict

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>
@JanStaschulat
Copy link
Contributor

solves #50

Acuadros95 pushed a commit that referenced this pull request Nov 11, 2021
* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge confict

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>
Acuadros95 pushed a commit that referenced this pull request Nov 12, 2021
* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge confict

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>
pablogs9 added a commit that referenced this pull request Nov 15, 2021
* ci workflow for galactic (#90)

Signed-off-by: Jan Staschulat <[email protected]>

* galactic: updated ci-job and codecov (#91)

* bump version to 2.0.1 of galactic branch for new bloom release (#95)

* Dummy (#100)

* test of galactic bloom release see also #100 (#101)

* Adds a context pointer to subscriptions (backport #107) (#120)

* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge confict

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>

* version bump and updated changelog. (#139)

Signed-off-by: Jan Staschulat <[email protected]>

* prepare bloom release v2.0.3 for galactic (#159)

Signed-off-by: Jan Staschulat <[email protected]>

* refactor #116 remove callback_type (#154) (#163)

* refactor #116 remove callback_type

Signed-off-by: BrettRD <[email protected]>

* rename subscription callback

Signed-off-by: BrettRD <[email protected]>

* deduplicate remove functions

Signed-off-by: BrettRD <[email protected]>
(cherry picked from commit 700ea24)

Co-authored-by: Brett Downing <[email protected]>

* updated rcl dependency to galactic (#184)

Signed-off-by: Jan Staschulat <[email protected]>

* added pingpong example (#172) (#188)

* added pingpong example

Signed-off-by: Jan Staschulat <[email protected]>

* added two executor configuration to pingpong

Signed-off-by: Jan Staschulat <[email protected]>

* example_pingpong multiple files

Signed-off-by: Jan Staschulat <[email protected]>

* added class member function to executor

Signed-off-by: Jan Staschulat <[email protected]>
(cherry picked from commit e0252bd)

Co-authored-by: Jan Staschulat <[email protected]>

* prepare bloom release 2.0.4-galactic (#193)

Signed-off-by: Jan Staschulat <[email protected]>

* Copy client to action client


Initial mods to action client

Initial executor support for action client

Updates

Fix

Updates

Copy client action to server action


Initial action server changes

Fix

Revert "Copy client to action client"

This reverts commit 304c70a.

Initial


Update


Working state


Uncrustify


fix


Update


Update


Add client


Uncrustify


Uncrus


Apply suggestions from code review

Co-authored-by: Ralph Lange <[email protected]>
Initial test


Fix data available logic


Update tests


Update


Uncrust


Revert "Update"

This reverts commit 940aa96.

Working state

Working

Building state


Update


Working state


Working state


Working state


Uncrust


Updates


Fix cancelling reject


Test approach modified


Complete server tests


Uncrusti


Add multigoal test


Fix


Initial action client tests


Add executor client cancel handlers


Update action client


Add cancel tests


Tests update


Uncrustify


Refactor cancel response handling


Update

* Revert "Copy client to action client"

This reverts commit d65adc7.

* Initial version

* Hide non public API

* Fix abort API

* Minor fix

* Update

* Fix

* Add rclcpp dep

* Update actions implementation

* Delete COLCON_IGNORE

* Fix dependencies export

* cpplint

* Initial Ralph changes

Co-authored-by: Ralph Lange <[email protected]>

* Add RCLC error

* Refactor pop/take inner functions

* Argument renaming

* Argument renaming

* Rename get API to find

* Reafctor functio naming

* Refactor naming

* Remove explicit == true in conditionals

* Update rclc/src/rclc/executor.c

Co-authored-by: Ralph Lange <[email protected]>

* Update rclc/src/rclc/executor.c

Co-authored-by: Ralph Lange <[email protected]>

* Remove white lines

* Update rclc/src/rclc/action_server.c

* Apply suggestions from code review

Co-authored-by: Jan Staschulat <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jan Staschulat <[email protected]>

* Add pointer check suggestions

* Apply suggestions on executor

* Add check for feedback msg on client

* Put goal on default switch case

* Revert code delete

* Add comments to executor loops

* Update loop comments

* Apply for loop style where possible

* Fix variable name

* Fix rebase

* ci workflow for galactic (#90)

Signed-off-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* galactic: updated ci-job and codecov (#91)

Signed-off-by: Antonio Cuadros <[email protected]>

* bump version to 2.0.1 of galactic branch for new bloom release (#95)

Signed-off-by: Antonio Cuadros <[email protected]>

* Dummy (#100)

Signed-off-by: Antonio Cuadros <[email protected]>

* test of galactic bloom release see also #100 (#101)

Signed-off-by: Antonio Cuadros <[email protected]>

* Adds a context pointer to subscriptions (backport #107) (#120)

* Adds a context pointer to subscriptions (#107)

(cherry picked from commit 5b11c5a)

# Conflicts:
#	rclc_examples/CMakeLists.txt

* resolved merge confict

Signed-off-by: Jan Staschulat <[email protected]>

Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* version bump and updated changelog. (#139)

Signed-off-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* prepare bloom release v2.0.3 for galactic (#159)

Signed-off-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* refactor #116 remove callback_type (#154) (#163)

* refactor #116 remove callback_type

Signed-off-by: BrettRD <[email protected]>

* rename subscription callback

Signed-off-by: BrettRD <[email protected]>

* deduplicate remove functions

Signed-off-by: BrettRD <[email protected]>
(cherry picked from commit 700ea24)

Co-authored-by: Brett Downing <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* updated rcl dependency to galactic (#184)

Signed-off-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* added pingpong example (#172) (#188)

* added pingpong example

Signed-off-by: Jan Staschulat <[email protected]>

* added two executor configuration to pingpong

Signed-off-by: Jan Staschulat <[email protected]>

* example_pingpong multiple files

Signed-off-by: Jan Staschulat <[email protected]>

* added class member function to executor

Signed-off-by: Jan Staschulat <[email protected]>
(cherry picked from commit e0252bd)

Co-authored-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* prepare bloom release 2.0.4-galactic (#193)

Signed-off-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Copy client to action client


Initial mods to action client

Initial executor support for action client

Updates

Fix

Updates

Copy client action to server action


Initial action server changes

Fix

Revert "Copy client to action client"

This reverts commit 304c70a.

Initial


Update


Working state


Uncrustify


fix


Update


Update


Add client


Uncrustify


Uncrus


Apply suggestions from code review

Co-authored-by: Ralph Lange <[email protected]>
Initial test


Fix data available logic


Update tests


Update


Uncrust


Revert "Update"

This reverts commit 940aa96.

Working state

Working

Building state


Update


Working state


Working state


Working state


Uncrust


Updates


Fix cancelling reject


Test approach modified


Complete server tests


Uncrusti


Add multigoal test


Fix


Initial action client tests


Add executor client cancel handlers


Update action client


Add cancel tests


Tests update


Uncrustify


Refactor cancel response handling


Update

Signed-off-by: Antonio Cuadros <[email protected]>

* Revert "Copy client to action client"

This reverts commit d65adc7.

Signed-off-by: Antonio Cuadros <[email protected]>

* Initial version

Signed-off-by: Antonio Cuadros <[email protected]>

* Hide non public API

Signed-off-by: Antonio Cuadros <[email protected]>

* Fix abort API

Signed-off-by: Antonio Cuadros <[email protected]>

* Minor fix

Signed-off-by: Antonio Cuadros <[email protected]>

* Update

Signed-off-by: Antonio Cuadros <[email protected]>

* Fix

Signed-off-by: Antonio Cuadros <[email protected]>

* Add rclcpp dep

Signed-off-by: Antonio Cuadros <[email protected]>

* Update actions implementation

Signed-off-by: Antonio Cuadros <[email protected]>

* Delete COLCON_IGNORE

Signed-off-by: Antonio Cuadros <[email protected]>

* Fix dependencies export

Signed-off-by: Antonio Cuadros <[email protected]>

* cpplint

Signed-off-by: Antonio Cuadros <[email protected]>

* Initial Ralph changes

Co-authored-by: Ralph Lange <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Add RCLC error

Signed-off-by: Antonio Cuadros <[email protected]>

* Refactor pop/take inner functions

Signed-off-by: Antonio Cuadros <[email protected]>

* Argument renaming

Signed-off-by: Antonio Cuadros <[email protected]>

* Argument renaming

Signed-off-by: Antonio Cuadros <[email protected]>

* Rename get API to find

Signed-off-by: Antonio Cuadros <[email protected]>

* Reafctor functio naming

Signed-off-by: Antonio Cuadros <[email protected]>

* Refactor naming

Signed-off-by: Antonio Cuadros <[email protected]>

* Remove explicit == true in conditionals

Signed-off-by: Antonio Cuadros <[email protected]>

* Update rclc/src/rclc/executor.c

Co-authored-by: Ralph Lange <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Update rclc/src/rclc/executor.c

Co-authored-by: Ralph Lange <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Remove white lines

Signed-off-by: Antonio Cuadros <[email protected]>

* Update rclc/src/rclc/action_server.c

Signed-off-by: Antonio Cuadros <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jan Staschulat <[email protected]>
Signed-off-by: Antonio Cuadros <[email protected]>

* Add pointer check suggestions

Signed-off-by: Antonio Cuadros <[email protected]>

* Apply suggestions on executor

Signed-off-by: Antonio Cuadros <[email protected]>

* Add check for feedback msg on client

Signed-off-by: Antonio Cuadros <[email protected]>

* Put goal on default switch case

Signed-off-by: Antonio Cuadros <[email protected]>

* Revert code delete

Signed-off-by: Antonio Cuadros <[email protected]>

* Add comments to executor loops

Signed-off-by: Antonio Cuadros <[email protected]>

* Update loop comments

Signed-off-by: Antonio Cuadros <[email protected]>

* Apply for loop style where possible

Signed-off-by: Antonio Cuadros <[email protected]>

* Fix variable name

Signed-off-by: Antonio Cuadros <[email protected]>

* Fix rebase

Signed-off-by: Antonio Cuadros <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jan Staschulat <[email protected]>

* removed unneccary reset of handle->data_available , in _rclc_check_for_new_data this flag is assigned again.

Signed-off-by: Jan Staschulat (CR/ADA1.2) <[email protected]>

* Change put goal function name

Co-authored-by: Jan Staschulat <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Brett Downing <[email protected]>
Co-authored-by: Antonio Cuadros <[email protected]>
Co-authored-by: Ralph Lange <[email protected]>
Co-authored-by: Antonio Cuadros <[email protected]>
@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

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.

5 participants