Skip to content

refactor: apply snake_case to the function names#62

Merged
ktro2828 merged 1 commit intomainfrom
refactor/node/snake-case
Oct 2, 2025
Merged

refactor: apply snake_case to the function names#62
ktro2828 merged 1 commit intomainfrom
refactor/node/snake-case

Conversation

@ktro2828
Copy link
Copy Markdown
Owner

@ktro2828 ktro2828 commented Oct 2, 2025

Description

This pull request refactors several node classes in the mmros package to use consistent naming conventions and simplifies callback handling for image and pointcloud subscriptions. The main changes involve renaming methods to snake_case, updating callback implementations, and standardizing utility function names.

Node API Consistency

  • Renamed all connection-related methods in node classes from camelCase (e.g., onConnect, onConnectLidar, onConnectForSingleCamera) to snake_case (e.g., on_connect, on_connect_lidar, on_connect_for_single_camera) in both header and source files for CameraLidarNode, LidarNode, MultiCameraNode, and SingleCameraNode. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]

Utility Function Standardization

  • Renamed utility functions in mmros/include/mmros/node/utility.hpp from camelCase (getTopicQos, resolveTopicName) to snake_case (to_topic_qos, resolve_topic_name) and updated all usages accordingly. [1] [2] [3] [4] [5]

Callback Refactoring for Image Nodes

  • Replaced virtual onImage methods with concrete callback methods in all 2D image processing nodes (Detection2dNode, InstanceSegmentation2dNode, PanopticSegmentation2dNode, SemanticSegmentation2dNode). Updated constructors to bind the new callback method directly, removing use of std::bind and std::placeholders. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

These changes improve code readability, maintainability, and consistency across the node interfaces and utility functions.

How was this PR tested?

Notes for reviewers

None.

Effects on system behavior

None.

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
Copilot AI review requested due to automatic review settings October 2, 2025 22:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Refactors node and utility function names to snake_case and replaces virtual image processing hooks with concrete callback methods to improve consistency and simplify subscription handling.

  • Rename connection and utility methods from camelCase to snake_case.
  • Replace virtual onImage methods with non-virtual callback methods and direct lambda bindings.
  • Adjust method access levels (some formerly public methods moved to protected).

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
mmros/src/node/single_camera_node.cpp Renamed onConnect to on_connect and updated utility function calls.
mmros/src/node/semantic_segmentation2d_node.cpp Replaced onImage with callback and updated timer binding.
mmros/src/node/panoptic_segmentation2d_node.cpp Same callback refactor pattern as other 2D node.
mmros/src/node/multi_camera_node.cpp Renamed connection helpers to snake_case.
mmros/src/node/lidar_node.cpp Renamed onConnect to on_connect and utility usages.
mmros/src/node/instance_segmentation2d_node.cpp Callback refactor and naming updates.
mmros/src/node/detection2d_node.cpp Callback refactor and naming updates.
mmros/src/node/camera_lidar_node.cpp Renamed composite connection methods to snake_case.
mmros/include/mmros/node/utility.hpp Renamed utility helpers to snake_case.
mmros/include/mmros/node/single_camera_node.hpp Renamed and moved on_connect to protected.
mmros/include/mmros/node/semantic_segmentation2d_node.hpp Replaced virtual onImage with private callback.
mmros/include/mmros/node/panoptic_segmentation2d_node.hpp Same callback change.
mmros/include/mmros/node/multi_camera_node.hpp Renamed methods and moved access to protected.
mmros/include/mmros/node/lidar_node.hpp Moved and renamed on_connect to protected.
mmros/include/mmros/node/instance_segmentation2d_node.hpp Same callback method change.
mmros/include/mmros/node/detection2d_node.hpp Same callback method change.
mmros/include/mmros/node/camera_lidar_node.hpp Renamed methods and moved to protected scope.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread mmros/include/mmros/node/single_camera_node.hpp
* @param use_raw Indicates whether to use raw image.
*/
void onConnect(const Callback & callback, bool use_raw);
void on_connect(const Callback & callback, bool use_raw);
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

on_connect was previously public (before inserting protected:) and making it protected is a breaking API change for external users instantiating a SingleCameraNode and manually triggering connection; consider keeping it public or providing a deprecated public wrapper to preserve backward compatibility.

Copilot uses AI. Check for mistakes.
Comment thread mmros/include/mmros/node/multi_camera_node.hpp
* @param use_raw Whether to use raw images or not.
*/
void onConnect(
void on_connect(
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Changing on_connect (formerly onConnect) to protected restricts external orchestration code that may have relied on invoking it directly; if intentional, document the change or provide a retained public shim to avoid silent API breakage.

Copilot uses AI. Check for mistakes.
Comment thread mmros/include/mmros/node/camera_lidar_node.hpp
* @param use_raw Whether to use raw image data.
*/
void onConnect(
void on_connect(
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Similar to other node classes, moving on_connect to protected alters the public API surface; confirm no downstream packages depend on calling this directly or offer a transitional public method with deprecation notice.

Copilot uses AI. Check for mistakes.
Comment thread mmros/src/node/semantic_segmentation2d_node.cpp
@ktro2828 ktro2828 merged commit 3ca78a4 into main Oct 2, 2025
1 check failed
@ktro2828 ktro2828 deleted the refactor/node/snake-case branch October 2, 2025 22:43
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