Skip to content

Add testcase for services with buffer in the response only - #263

Open
mathias-luedtke wants to merge 1 commit into
ros2:rollingfrom
mathias-luedtke:feature/buffer-test-case
Open

Add testcase for services with buffer in the response only#263
mathias-luedtke wants to merge 1 commit into
ros2:rollingfrom
mathias-luedtke:feature/buffer-test-case

Conversation

@mathias-luedtke

Copy link
Copy Markdown

Description

This PR add a test case that shows that cpplint checks fail for generated code for services that have buffers in their response, but not in the request.
The same issue should happen for actions.

The tests will fail with:

- rosidl_generator_py.cpplint_rosidl_generated_py build/include_order [4] (/ws/build/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/srv/_get_buffer_s.c:83)
  <<< failure message
    Found C system header after other header. Should be: _get_buffer_s.h, c system, c++ system, other.
  >>>

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

This issue was introduced by #250

@mergify

mergify Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@skyegalaxy

Copy link
Copy Markdown
Member

@adbidwai to take a look

@adbidwai

adbidwai commented Aug 13, 2026

Copy link
Copy Markdown

Hi, I can take a look at this. Thanks!

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I do not think we can take this without fixes, because this generates intentionally red test.

Comment thread rosidl_generator_py/srv/GetBuffer.srv
Signed-off-by: Mathias Lüdtke <mathias.luedtke@pal-robotics.com>
@mathias-luedtke
mathias-luedtke force-pushed the feature/buffer-test-case branch from 26b66a6 to 6222bb3 Compare August 14, 2026 11:00
@mathias-luedtke

Copy link
Copy Markdown
Author

I do not think we can take this without fixes, because this generates intentionally red test.

I can add the fix as well, but I need to know which option do you prefer (see #264)

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