Fix unit test on build farm for new service functions #49

JanStaschulat · 2021-02-16T14:26:46Z

@norro
Hi Arne, could you have a look, why these unit tests with services fail?
https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/71/#showFailuresLink

Option A)

you could use this PR for the fix: Fix ci-job for Dashing #46
that means to create a branch based on micro-ros/rclc:master and then
create a PR to micro-ros/rclc:master.
then these changes will be considered in the PR.

Option B)

I merge this PR Fix ci-job for Dashing #46 now
You start fresh from ros2/rclc:master

JanStaschulat · 2021-02-16T18:49:55Z

@norro: The expected sequence id is 1, however the returned sequence id is different in each unit test on the build farm. Strange, but as this is not part of the executor functionality, but an RCL/RMW issue, I would propose to omit this ASSERT in all relevant unit_tests with services/clients:

rclc/rclc/test/rclc/test_executor.cpp

Line 2081 in c5b0d3d

EXPECT_EQ(seq, (int64_t) 1); // sequence id = 1

norro · 2021-02-17T07:07:13Z

Agreed, I wouldn't have a clue how to resolve this.
Should we / can we report an issue upstream (in rcl/rmw) to have this fixed upstream?

JanStaschulat · 2021-02-17T09:51:30Z

Yes, that is a good idea. you could also point out, that it is only an issue on the build farm and not in a normal colcon test with Github Action.

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49

norro · 2021-02-19T13:21:35Z

@JanStaschulat Could you please write the upstream issue in rcl/rmw? I don't have enough knowledge of rcl/rmw to be able to phrase the issue, I'm afraid.
After the issue is created, I suggest to close this issue.

JanStaschulat · 2021-02-22T08:07:56Z

@norro created the issue ros2/rcl#892. I would keep this issue open, because I referenced it in the ros2/rcl issue.

JanStaschulat · 2021-02-22T17:27:50Z

@norro. As explained in the issue ros2/rcl#892, the specification of rcl_send_request does not say to which value seq_id is initialized to. It is only guaranteed to be unique.

Therefore we can not check this value (even though it seems to be always "1" for Dashing and Foxy). So we can keep the unit test the way they are. I will remove these commented out lines. Then we can close this issue.

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

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

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49 Signed-off-by: Jan Staschulat <[email protected]>

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49 Signed-off-by: Jan Staschulat <[email protected]>

* 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 <[email protected]> * Fixed service callback handling Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Proper tear-down of msgs Signed-off-by: Arne Nordmann <[email protected]> * Fixes service call with context tests Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Adds callback for GetState service micro-ROS#40 Signed-off-by: Arne Nordmann <[email protected]> * Provide GetAvailableStates service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Example lifecycle node proides services Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Implemented ChangeState service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Linting, documentation, example Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Adds missing declaration of callbacks Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Pre-initialize all strings Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Tests for service initialization Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Ignoring unsuccessful SERVICE_TAKE (#175) * resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED Signed-off-by: Jan Staschulat <[email protected]> * 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 <[email protected]> * Fixes init and cleanup Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> Co-authored-by: Jan Staschulat <[email protected]>

* 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 <[email protected]> * Fixed service callback handling Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Proper tear-down of msgs Signed-off-by: Arne Nordmann <[email protected]> * Fixes service call with context tests Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Adds callback for GetState service micro-ROS#40 Signed-off-by: Arne Nordmann <[email protected]> * Provide GetAvailableStates service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Example lifecycle node proides services Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Implemented ChangeState service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Linting, documentation, example Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Adds missing declaration of callbacks Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Pre-initialize all strings Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Tests for service initialization Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Ignoring unsuccessful SERVICE_TAKE (#175) * resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED Signed-off-by: Jan Staschulat <[email protected]> * 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 <[email protected]> * Fixes init and cleanup Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> (cherry picked from commit 865b02b)

* 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 <[email protected]> * Fixed service callback handling Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Proper tear-down of msgs Signed-off-by: Arne Nordmann <[email protected]> * Fixes service call with context tests Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Adds callback for GetState service micro-ROS#40 Signed-off-by: Arne Nordmann <[email protected]> * Provide GetAvailableStates service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Example lifecycle node proides services Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Implemented ChangeState service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Linting, documentation, example Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Adds missing declaration of callbacks Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Pre-initialize all strings Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Tests for service initialization Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Ignoring unsuccessful SERVICE_TAKE (#175) * resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED Signed-off-by: Jan Staschulat <[email protected]> * 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 <[email protected]> * Fixes init and cleanup Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> (cherry picked from commit 865b02b) # Conflicts: # rclc_lifecycle/src/rclc_lifecycle/rclc_lifecycle.c # rclc_lifecycle/test/test_lifecycle.cpp

* 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 <[email protected]> * Fixed service callback handling Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Proper tear-down of msgs Signed-off-by: Arne Nordmann <[email protected]> * Fixes service call with context tests Signed-off-by: Arne Nordmann <[email protected]> * 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 <[email protected]> * Adds callback for GetState service micro-ROS#40 Signed-off-by: Arne Nordmann <[email protected]> * Provide GetAvailableStates service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Example lifecycle node proides services Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Implemented ChangeState service Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Linting, documentation, example Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Adds missing declaration of callbacks Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * Pre-initialize all strings Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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) <[email protected]> * Tests for service initialization Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Ignoring unsuccessful SERVICE_TAKE (#175) * resolved bug by ignoring RCL_RET_SERVICE_TAKE_FAILED Signed-off-by: Jan Staschulat <[email protected]> * 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 <[email protected]> * Fixes init and cleanup Signed-off-by: Nordmann Arne (CR/ADT3) <[email protected]> * 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 <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> (cherry picked from commit 865b02b) Co-authored-by: Arne Nordmann <[email protected]>

* goal_state is int, NULL check results in compiler warning (comparing int with pointer), removed this check. * resolving bug for unit test on Eloquent. rcl_node_fini was not called at the end of unit tests, this created the error, that node was still initialized in next unit test. * fixed unit test failures for Dashing. node and other objects were not properly cleaned up with _fini methods, in next unit test the same node was used again. Added clean-up code.

JanStaschulat mentioned this issue Feb 16, 2021

Fix ci-job for Dashing #46

Merged

JanStaschulat added a commit to micro-ROS/rclc that referenced this issue Feb 17, 2021

commnted out check for received sequence id - error in build-job for …

f66ce3c

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49

JanStaschulat self-assigned this Feb 17, 2021

norro pushed a commit to micro-ROS/rclc that referenced this issue Feb 17, 2021

commnted out check for received sequence id - error in build-job for …

7b0111a

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49

JanStaschulat mentioned this issue Feb 22, 2021

sequence number of service-request is not initialized to one in a unit test on ROS 2 build farm for Rolling release ros2/rcl#892

Closed

JanStaschulat mentioned this issue Feb 23, 2021

should use initialized objects. #52

Merged

JanStaschulat added a commit to micro-ROS/rclc that referenced this issue Feb 23, 2021

removed checks for sequence id, see issue ros2#49

ddd6a96

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

JanStaschulat mentioned this issue Feb 23, 2021

removed checks for sequence id #53

Merged

norro linked a pull request Feb 23, 2021 that will close this issue

removed checks for sequence id #53

Merged

JanStaschulat closed this as completed in #53 Feb 23, 2021

BrettRD pushed a commit to BrettRD/rclc that referenced this issue Jul 5, 2021

removed checks for sequence id, see issue ros2#49

abd1d01

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

JanStaschulat added a commit to micro-ROS/rclc that referenced this issue Aug 19, 2021

commnted out check for received sequence id - error in build-job for …

34c1d54

…rolling https://build.ros2.org/job/Rpr__rclc__ubuntu_focal_amd64/72/ see also PR ros2#46 and issue ros2#49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unit test on build farm for new service functions #49

Fix unit test on build farm for new service functions #49

JanStaschulat commented Feb 16, 2021 •

edited

Loading

JanStaschulat commented Feb 16, 2021 •

edited

Loading

norro commented Feb 17, 2021

JanStaschulat commented Feb 17, 2021

norro commented Feb 19, 2021

JanStaschulat commented Feb 22, 2021 •

edited

Loading

JanStaschulat commented Feb 22, 2021 •

edited

Loading

Fix unit test on build farm for new service functions #49

Fix unit test on build farm for new service functions #49

Comments

JanStaschulat commented Feb 16, 2021 • edited Loading

JanStaschulat commented Feb 16, 2021 • edited Loading

norro commented Feb 17, 2021

JanStaschulat commented Feb 17, 2021

norro commented Feb 19, 2021

JanStaschulat commented Feb 22, 2021 • edited Loading

JanStaschulat commented Feb 22, 2021 • edited Loading

JanStaschulat commented Feb 16, 2021 •

edited

Loading

JanStaschulat commented Feb 16, 2021 •

edited

Loading

JanStaschulat commented Feb 22, 2021 •

edited

Loading

JanStaschulat commented Feb 22, 2021 •

edited

Loading