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.

Sign up for GitHub

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

Jump to bottom

[Backport Foxy] Included improvements to increase code coverage #255

Merged
merged 25 commits into from
Oct 15, 2020
Merged

[Backport Foxy] Included improvements to increase code coverage #255

merged 25 commits into from
Oct 15, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Oct 9, 2020 •
edited
Loading

This PR depends on:

Included error checks, return the right code errors, avoid crashes, etc

hidmic and others added 21 commits October 8, 2020 14:55
@hidmic @ahcorde
Finalize context iff shutdown. (#196)
d35e7c3 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant init options API implementations. (#200)
694fb17 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant init/shutdown API implementations. (#202)
faf7bf3 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Amend rmw_init() implementation: require enclave. (#204)
5bc0b8b 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant node construction/destruction API. (#206)
e2b0e05 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant publisher API (#210)
4c1b4f0 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant subscription API. (#214)
dcf2bdb 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Fix bad conditional in rmw_serialize(). (#217)
c241476 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Change RET_WRONG_IMPLID() to return RMW_RET_INCORRECT_IMPLEMENTATION (#…
077d990 
…226)

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Ensure compliant matched pub/sub count API. (#223)
437f610 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Lobotuerk @ahcorde
Update rmw_publish() error returns (#239)
f150847 
Signed-off-by: lobotuerk <jtlorente@ekumenlabs.com>
@Lobotuerk @ahcorde
Update rmw_publish_serialized_message() error returns (#240)
0410ea9 
Signed-off-by: lobotuerk <jtlorente@ekumenlabs.com>
@Lobotuerk @ahcorde
Updated error returns on rmw_take (#241)
2743355 
Signed-off-by: lobotuerk <jtlorente@ekumenlabs.com>
@Lobotuerk @ahcorde
Updated error returns on rmw_take_serialized and with_message_info (#242
fcf4ea1 
)

Signed-off-by: lobotuerk <jtlorente@ekumenlabs.com>
@hidmic @ahcorde
Check for message_info on take where appropriate. (#245)
4f843e1 
Fix for regression introduced in #241.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Update graph API return codes. (#243)
fba2730 
This patch affects:
- rmw_get_node_names()
- rmw_get_node_names_with_enclaves()
- rmw_get_topic_names_and_types()
- rmw_get_service_names_and_types()
- rmw_get_publishers_info_by_topic()
- rmw_get_subscriptions_info_by_topic()
- rmw_get_subscriber_names_and_types_by_node()
- rmw_get_publisher_names_and_types_by_node()
- rmw_get_service_names_and_types_by_node()
- rmw_get_client_names_and_types_by_node()
- rmw_count_publishers()
- rmw_count_subscribers()

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Update gid API return codes. (#244)
817f738 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@ahcorde
Updated rmw_* return codes (#246)
c26cc87 
* Updated rmw_*_*_allocation return codes

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed return codes in rmw_*

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Improved return error in rmw_wait

Signed-off-by: ahcorde <ahcorde@gmail.com>
@Lobotuerk @ahcorde
Update service/client request/response API error returns (#249)
6979640 
Signed-off-by: lobotuerk <jtlorente@ekumenlabs.com>
@ahcorde
Return RMW_RET_UNSUPPORTED in rmw_get_serialized_message_size (#250)
e3acf40 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@hidmic @ahcorde
Update service/client construction/destruction API return codes. (#247)
2dc9c89 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@ahcorde ahcorde requested a review from brawner October 9, 2020 07:22
@ahcorde ahcorde self-assigned this Oct 9, 2020
@ahcorde ahcorde mentioned this pull request Oct 9, 2020
Merged
brawner
brawner approved these changes Oct 9, 2020
View reviewed changes
Copy link
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Can you update the title and description, this PR just seems like bug fixes for rmw_node.cpp and does not actually affect code coverage much.

@ahcorde
Copy link
Contributor Author

ahcorde commented Oct 13, 2020

@ros-pull-request-builder retest this please

@ahcorde
Updated Github Actions
b0d2eae 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Fixed cppcheck issue
ff103f6 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Oct 14, 2020

Building up-to rmw_cyclonedds_cppand testingrmw_cyclonedds_cpp`:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde
Update Github actions
f3016a4 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Oct 15, 2020

There is an open issue ros2/ros2#942 about the cppcheck failure in the Github Actions

  [5.800s] 2: [src/rmw_node.cpp:705]: (error: unknownMacro) There is an unknown macro here somewhere. Configuration is required. If RCUTILS_STRINGIFY is a macro then please configure it.

"rcutils/macros.h" is included in the rmw_node.cpp file.

ROS 2 buildfarm is fine, @ivanpauno or @eboasson any thoughts ?

@ivanpauno
Copy link
Member

Apply this patch to the Foxy branch.
There are other possible solutions as discussed here, but I would apply the same patch that was used on master.

@rotu @ahcorde
Suppress cppcheck unknownMacro (#237)
aee4305 
Fast forward to ament/ament_lint#268

This suppresses a defect in `ament_cppcheck` - namely that if a macro is missing, it fails loudly. This is compounded by fact that `ament_cppcheck` does not include all dependent headers, so macros are likely to be missing in the first place.

Signed-off-by: Dan Rose <dan@digilabs.io>
@ahcorde ahcorde merged commit 2477456 into foxy Oct 15, 2020
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/backport/foxy branch October 15, 2020 13:45
@ivanpauno ivanpauno mentioned this pull request Nov 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brawner brawner brawner approved these changes

Assignees

@ahcorde ahcorde

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ahcorde @ivanpauno @brawner @hidmic @Lobotuerk @rotu