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

[rolling] image_publisher: Fix loading of the camera info parameters on startup #983

Conversation

Kotochleb
Copy link
Contributor

@Kotochleb Kotochleb commented May 25, 2024

As described in #965 camera info is not loaded from the file on node initialization, but only when the parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to Humble, Iron and Jazzy.

@Kotochleb Kotochleb changed the title Humble: Fix loading of the camera info parameters on startup [humble] image_publisher: Fix loading of the camera info parameters on startup May 26, 2024
Copy link
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.

can you target rolling ? then I will backport this to the other distros

@Kotochleb Kotochleb force-pushed the bugfix/image-publisher-camera-info-startup branch from 68e9e3a to 1306e63 Compare May 27, 2024 09:03
@Kotochleb Kotochleb force-pushed the bugfix/image-publisher-camera-info-startup branch from 1306e63 to 2d58d0e Compare May 27, 2024 10:40
@Kotochleb Kotochleb changed the base branch from humble to rolling May 27, 2024 10:40
@Kotochleb Kotochleb changed the title [humble] image_publisher: Fix loading of the camera info parameters on startup [rolling] image_publisher: Fix loading of the camera info parameters on startup May 27, 2024
@Kotochleb
Copy link
Contributor Author

@ahcorde the changes were retargeted to rolling as requested.

@Kotochleb Kotochleb requested a review from ahcorde May 27, 2024 10:49
@@ -71,36 +71,46 @@ ImagePublisher::ImagePublisher(
auto param_change_callback =
[this](std::vector<rclcpp::Parameter> parameters) -> rcl_interfaces::msg::SetParametersResult
{
RCLCPP_INFO(get_logger(), "param_change_callback");
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -224,9 +234,7 @@ void ImagePublisher::onInit()
camera_info_.p = {1, 0, static_cast<float>(camera_info_.width / 2), 0, 0, 1,
static_cast<float>(camera_info_.height / 2), 0, 0, 0, 1, 0};

timer_ = this->create_wall_timer(
Copy link
Contributor

Choose a reason for hiding this comment

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

if you remove these lines the dowork method will never be called and the image will be never flipped

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 timer is set in the reconfigureCallback. I didn't want to repeat the CameraInfoManager block of code in onInit and did not want to create timer there just to recreate it once again in reconfigureCallback.

@Kotochleb Kotochleb requested a review from ahcorde May 27, 2024 12:14
@Kotochleb
Copy link
Contributor Author

@ahcorde FYI changes from #985 are already merged in here.

@ahcorde ahcorde merged commit 847920b into ros-perception:rolling May 29, 2024
3 checks passed
@ahcorde
Copy link
Contributor

ahcorde commented May 29, 2024

https://github.com/Mergifyio backport jazzy humble iron

Copy link

mergify bot commented May 29, 2024

backport jazzy humble iron

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 29, 2024
…on startup (#983)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.

(cherry picked from commit 847920b)

# Conflicts:
#	image_publisher/src/image_publisher.cpp
mergify bot pushed a commit that referenced this pull request May 29, 2024
…on startup (#983)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.

(cherry picked from commit 847920b)

# Conflicts:
#	image_publisher/src/image_publisher.cpp
mergify bot pushed a commit that referenced this pull request May 29, 2024
…on startup (#983)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.

(cherry picked from commit 847920b)

# Conflicts:
#	image_publisher/src/image_publisher.cpp
mikeferguson pushed a commit that referenced this pull request Jun 10, 2024
…on startup (#983)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.

(cherry picked from commit 847920b)
mikeferguson added a commit that referenced this pull request Jun 10, 2024
…n startup (backport #983) (#996)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.<hr>This is an automatic backport of pull
request #983 done by [Mergify](https://mergify.com).

---------

Co-authored-by: Krzysztof Wojciechowski <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>
mikeferguson added a commit that referenced this pull request Jun 10, 2024
…startup (backport #983) (#997)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.<hr>This is an automatic backport of pull
request #983 done by [Mergify](https://mergify.com).

---------

Co-authored-by: Krzysztof Wojciechowski <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>
mikeferguson pushed a commit that referenced this pull request Jun 10, 2024
… startup (backport #983) (#995)

As described in
#965 camera info
is not loaded from the file on node initialization, but only when the
parameter is reloaded.

This PR resolves this issue and should be straightforward to port it to
`Humble`, `Iron` and `Jazzy`.<hr>This is an automatic backport of pull
request #983 done by [Mergify](https://mergify.com).

Co-authored-by: Krzysztof Wojciechowski <[email protected]>
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.

2 participants