Move/pipeline fsm by CurryMonsters · Pull Request #10 · vortexntnu/vortex-cv

CurryMonsters · 2026-03-24T11:06:08Z

moved pipeline fsm from vortex auv, this package is a fsm for the pipeline inspection task for tacc.

…_fsm

codecov · 2026-03-24T18:00:06Z

Codecov Report

❌ Patch coverage is 0% with 254 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.65%. Comparing base (ebb7b5a) to head (acce21a).

Files with missing lines	Patch %	Lines
...peline_inspection_fsm/src/search_pattern_state.cpp	0.00%	67 Missing ⚠️
...pection/pipeline_inspection_fsm/src/fsm_runner.cpp	0.00%	50 Missing ⚠️
...nspection_fsm/src/start_waypoint_manager_state.cpp	0.00%	24 Missing ⚠️
...pipeline_inspection_fsm/src/trigger_wait_state.cpp	0.00%	24 Missing ⚠️
...line_inspection_fsm/src/landmark_polling_state.cpp	0.00%	23 Missing ⚠️
...pection/pipeline_inspection_fsm/src/blackboard.cpp	0.00%	21 Missing ⚠️
...ion/pipeline_inspection_fsm/src/converge_state.cpp	0.00%	20 Missing ⚠️
...inspection_fsm/src/stop_waypoint_manager_state.cpp	0.00%	16 Missing ⚠️
...inspection_fsm/src/wait_for_pipeline_end_state.cpp	0.00%	7 Missing ⚠️
...peline_inspection_fsm/src/wait_for_start_state.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
- Coverage   17.91%   13.65%   -4.26%     
==========================================
  Files          17       27      +10     
  Lines         815     1069     +254     
  Branches      236      255      +19     
==========================================
  Hits          146      146              
- Misses        583      837     +254     
  Partials       86       86

Flag	Coverage Δ
unittests	`13.65% <0.00%> (-4.26%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...peline_inspection_fsm/src/wait_for_start_state.cpp	`0.00% <0.00%> (ø)`
...inspection_fsm/src/wait_for_pipeline_end_state.cpp	`0.00% <0.00%> (ø)`
...inspection_fsm/src/stop_waypoint_manager_state.cpp	`0.00% <0.00%> (ø)`
...ion/pipeline_inspection_fsm/src/converge_state.cpp	`0.00% <0.00%> (ø)`
...pection/pipeline_inspection_fsm/src/blackboard.cpp	`0.00% <0.00%> (ø)`
...line_inspection_fsm/src/landmark_polling_state.cpp	`0.00% <0.00%> (ø)`
...nspection_fsm/src/start_waypoint_manager_state.cpp	`0.00% <0.00%> (ø)`
...pipeline_inspection_fsm/src/trigger_wait_state.cpp	`0.00% <0.00%> (ø)`
...pection/pipeline_inspection_fsm/src/fsm_runner.cpp	`0.00% <0.00%> (ø)`
...peline_inspection_fsm/src/search_pattern_state.cpp	`0.00% <0.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jorgenfj · 2026-03-24T18:18:43Z

.../tacc/pipeline_inspection/pipeline_inspection_fsm/config/pipeline_inspection_fsm_config.yaml

+      waypoint_manager: "waypoint_manager"
+      landmark_polling: "landmark_polling"
+
+    services:
+      start_mission: "pipeline_inspection_fsm/start_mission"


These should be read from the drone config in auv_setup

jorgenfj · 2026-03-24T18:21:48Z

.../tacc/pipeline_inspection/pipeline_inspection_fsm/config/pipeline_inspection_fsm_config.yaml

+      landmark_type: 1
+      landmark_subtype: 19


Do these need to be in the config? I think it is better to hardcode them in the source files.
That also allows us to be more explicit like this vortex_msgs::msg::LandmarkSubtype::PIPELINE_START_CAMERA

jorgenfj · 2026-03-24T18:30:23Z

.../tacc/pipeline_inspection/pipeline_inspection_fsm/config/pipeline_inspection_fsm_config.yaml

+    fsm:
+      landmark_type: 1
+      landmark_subtype: 19
+      convergence_threshold: 0.1


The convergence should be more descriptive. We should find a way to tie it to specific waypoints or landmark convergence actions.

Maybe we should extend load waypoint from yaml to actually return a full waypoint with mode and convergence offset, and not just pose.

And create a load_landmark_convergence_from_yaml as well

jorgenfj · 2026-03-24T18:32:03Z

.../pipeline_inspection/pipeline_inspection_fsm/include/pipeline_inspection_fsm/param_utils.hpp

This does not follow vortex convention. Should just declare parameters in the constructor. I dont see any cases here where we will call get_parameter for a parameter that is undeclared

Pro tip:
declare_parameter also returns the value like this:

std::string frame_id_ = this->declare_parameter<std::string>("frame_id");

jorgenfj · 2026-03-24T18:34:51Z

.../tacc/pipeline_inspection/pipeline_inspection_fsm/include/pipeline_inspection_fsm/states.hpp

+    std::optional<pipeline_inspection_fsm::WaypointManagerAction::Goal>
+    build_search_goal(const std::vector<vortex::utils::types::Pose>& waypoints);
+
+    std::string waypoint_file_path_;


Maybe make this global or tied to a parent class so that it is common for all states

jorgenfj · 2026-03-24T18:44:20Z

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/src/converge_state.cpp

+    convergence_threshold_ = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.convergence_threshold");
+
+    const double offset_x = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.landmark_offset.x");
+    const double offset_y = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.landmark_offset.y");
+    const double offset_z = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.landmark_offset.z");
+    const double offset_roll = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.landmark_offset.roll");
+    const double offset_pitch =
+        pipeline_inspection_fsm::param_utils::get_double(
+            node, "fsm.landmark_offset.pitch");
+    const double offset_yaw = pipeline_inspection_fsm::param_utils::get_double(
+        node, "fsm.landmark_offset.yaw");


If we instead create a function to load landmark convergence goal from yaml we dont need all these declare_parameter calls

jorgenfj · 2026-03-24T18:47:19Z

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/src/fsm_runner.cpp

+namespace {
+
+std::string get_string_parameter(const rclcpp::Node::SharedPtr& node,
+                                 const std::string& name) {
+    if (!node->has_parameter(name))
+        node->declare_parameter<std::string>(name);
+
+    return node->get_parameter(name).as_string();
+}
+
+}  // namespace


can be removed

jorgenfj · 2026-03-24T18:49:25Z

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/src/fsm_runner.cpp

Is it possible to have a separate file for the state definitions and transitions?

jorgenfj · 2026-03-24T18:53:00Z

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/CMakeLists.txt

+set(DEPENDENCIES
+  ament_index_cpp
+  rclcpp
+  rclcpp_action
+  std_srvs
+  yasmin
+  yasmin_ros
+  yasmin_viewer
+  vortex_msgs
+  vortex_utils
+  vortex_utils_ros
+  vortex_utils_ros_tf
+  geometry_msgs
+  tf2
+  tf2_ros
+  tf2_geometry_msgs
+)
+
+add_executable(pipeline_inspection_fsm
+  src/tf_utils.cpp
+  src/trigger_wait_state.cpp
+  src/wait_for_start_state.cpp
+  src/search_pattern_state.cpp
+  src/landmark_polling_state.cpp
+  src/converge_state.cpp
+  src/start_waypoint_manager_state.cpp
+  src/wait_for_pipeline_end_state.cpp
+  src/stop_waypoint_manager_state.cpp
+  src/blackboard.cpp
+  src/fsm_runner.cpp
+)
+
+ament_target_dependencies(pipeline_inspection_fsm ${DEPENDENCIES})


Dont really need a separate dependencies list if we only use it once

jorgenfj · 2026-03-24T18:56:49Z

...acc/pipeline_inspection/pipeline_inspection_fsm/include/pipeline_inspection_fsm/tf_utils.hpp

Thinking about it I think it is easier and more robust to just pass in search waypoint from odom directly. Then it is easier to keep track of where the drone is going/are at all times and we are less susceptible to spontaneous changes in orientation that will change the positions of the waypoints when transforming from base to odom

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/src/landmark_polling_state.cpp

CurryMonsters requested a review from jorgenfj March 24, 2026 11:06

CurryMonsters added 3 commits March 24, 2026 18:49

moved pipeline_fsm to vortex-cv + changed name to pipeline_inspection…

3c1b7a8

…_fsm

changed waypointmode from only pos to full pose

8842397

ran pre-commit

468e0f2

jorgenfj force-pushed the move/pipeline_fsm branch from 9cd7063 to 468e0f2 Compare March 24, 2026 17:49

jorgenfj requested changes Mar 24, 2026

View reviewed changes

CurryMonsters added 5 commits March 25, 2026 10:55

fixed pr comments

16c43c4

fixed issue for CI failing

5035f15

moved srv start mission to orca.yaml

ea9e15f

changed back to using vortex_msg for landmark type + subtype

f893e7b

added readme

4392851

jorgenfj reviewed Mar 25, 2026

View reviewed changes

mission/tacc/pipeline_inspection/pipeline_inspection_fsm/src/landmark_polling_state.cpp Outdated Show resolved Hide resolved

removed struct auto insert from clangd

acce21a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move/pipeline fsm#10

Move/pipeline fsm#10
CurryMonsters wants to merge 9 commits intomainfrom
move/pipeline_fsm

CurryMonsters commented Mar 24, 2026

Uh oh!

codecov bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

jorgenfj Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

CurryMonsters commented Mar 24, 2026

Uh oh!

codecov bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 24, 2026 •

edited

Loading