Skip to content

image_proc::CropDecimateNode: parameter handle#978

Draft
mhubii wants to merge 1 commit intoros-perception:rollingfrom
mhubii:rolling
Draft

image_proc::CropDecimateNode: parameter handle#978
mhubii wants to merge 1 commit intoros-perception:rollingfrom
mhubii:rolling

Conversation

@mhubii
Copy link
Copy Markdown

@mhubii mhubii commented May 2, 2024

Refers to #967.

Thank you @ahcorde for kindly offering review of this PR. I'll review my main questions shortly.

Copy link
Copy Markdown
Author

@mhubii mhubii left a comment

Choose a reason for hiding this comment

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

2 general questions:

rcl_interfaces::msg::SetParametersResult result;
result.successful = true;
for (const auto & param : parameters) {
if (param.get_name() == "offset_x") {
Copy link
Copy Markdown
Author

@mhubii mhubii May 2, 2024

Choose a reason for hiding this comment

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

  • Should other parameters be handled as well?
  • Are additional checks on set desired?
  • Introduce parameter name as members?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can also add decimation_x_, decimation_y_ and target_frame_id_

@mhubii mhubii marked this pull request as draft May 2, 2024 17:20
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

uncrustify is failing https://build.ros2.org/job/Rpr__image_pipeline__ubuntu_noble_amd64/6/testReport/junit/image_proc/uncrustify/src_crop_decimate_cpp/

you can fix this with:

ament_uncrustify --reformat <path to file>

rcl_interfaces::msg::SetParametersResult result;
result.successful = true;
for (const auto & param : parameters) {
if (param.get_name() == "offset_x") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can also add decimation_x_, decimation_y_ and target_frame_id_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants