image_publisher_node not read camera_info_url on initialization

[Kotochleb]

As in the title, image_publisher_node doesn't account for camera_info_url on the initialization when launched as a standalone node on Humble. For some reason, it skips reading it. When the same parameter is changed dynamically after the node started, everything works just fine. The simplest walk-round I found was to change return result with break in the pram_change_callback like so:

if (parameter.get_name() == "filename") {
    filename_ = parameter.as_string();
    RCLCPP_INFO(get_logger(), "Reset filename as '%s'", filename_.c_str());
    ImagePublisher::onInit();
-   return result;
+   break;
    } else if (parameter.get_name() == "flip_horizontal") {
    flip_horizontal_ = parameter.as_bool();
    RCLCPP_INFO(get_logger(), "Reset flip_horizontal as '%d'", flip_horizontal_);
    ImagePublisher::onInit();
-   return result;
+   break;
  } else if (parameter.get_name() == "flip_vertical") {
    flip_vertical_ = parameter.as_bool();
    RCLCPP_INFO(get_logger(), "Reset flip_vertical as '%d'", flip_vertical_);
    ImagePublisher::onInit();
-   return result;
+   break;

Yet, I feel like there should be a better solution to this, and I do not feel confident enough with the original implementation to submit a good enough bugfix PR.

[mikeferguson]

I'm not sure why there is a "return result" in any of those cases - I would think we should iterate through all the parameter settings by completing the for loop (and then there is a return after the reconfigure callback is called). I think we should actually remove the lines entirely (not change to a break).

[Kotochleb]

I'm not sure why there is a "return result" in any of those cases

@mikeferguson this is precisely what caused my confusion and said lack of said confidence in making changes. If you think removing the return entirely is a fine fix I can submit PR soon.

[mikeferguson]

Looks like fix is merged to Rolling, backports in progress