Skip to content

Add Estefania onboarding package - #18

Closed
EstefaniaC-M wants to merge 2 commits into
palouse-robosub:mainfrom
EstefaniaC-M:estefania/add-estefania-pkg
Closed

EstefaniaC-M wants to merge 2 commits into
palouse-robosub:mainfrom
EstefaniaC-M:estefania/add-estefania-pkg

Conversation

@EstefaniaC-M

Copy link
Copy Markdown
Contributor

added my onboarding package

Updated the launch files and codeowners

@cole-wilson

Copy link
Copy Markdown
Member

Hello! Can you try and amend your commit messages to make the CI pass? Also, make sure to fill out the checkboxes in the PR description.

I'll take a look at the code soon. Thanks for submitting!

@EstefaniaC-M

EstefaniaC-M commented Sep 16, 2026 via email

Copy link
Copy Markdown
Contributor Author

@cole-wilson cole-wilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code generally looks great, only metadata and logging changes needed!

Comment thread src/estefania_pkg/CMakeLists.txt Outdated

add_executable(example_publisher src/example_publisher.cpp)
add_executable(example_subscriber src/example_subscriber.cpp)
ament_target_dependencies(example_publisher rclcpp std_msgs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

duplicate target. Also, can you rename from example_?

Comment thread src/estefania_pkg/README.md Outdated
# estefania_pkg


explain about your nodes and topics here. No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add a brief explanation here

Comment thread src/estefania_pkg/launch/all.xml Outdated
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<launch>
<node pkg="estefania_pkg" exec="example_publisher"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, just change the example_

Comment thread src/estefania_pkg/package.xml Outdated
<package format="3">
<name>estefania_pkg</name>
<version>0.0.0</version>
<description>TODO: Package description</description>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add name/description/license/email

std_msgs::msg::Float32 message;
message.data = distribution_(generator_);

RCLCPP_INFO(this->get_logger(), "Publishing: %.2f", message.data);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove the log here, just so that the only console output comes from the echo service

std::string summary =
"Estefania received the value: " + std::to_string(message->data);

RCLCPP_INFO(this->get_logger(), "%s", summary.c_str());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, standard logs should only come from the echo service

@EstefaniaC-M
EstefaniaC-M force-pushed the estefania/add-estefania-pkg branch from ff9a740 to 43b55aa Compare September 16, 2026 23:37
@EstefaniaC-M

Copy link
Copy Markdown
Contributor Author

I reran the CI job, but it failed again at the cpp-linter step. The log shows 0 clang-format-checks-failed, 0 clang-tidy-checks-failed, and 0 checks-failed. The failure happens when cpp-linter tries to post a comment to PR #18 and GitHub returns a 403 "Resource not accessible by integration" error.

@cole-wilson cole-wilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good

@EstefaniaC-M

EstefaniaC-M commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor Author
  • I have a descriptive name for this PR (like "Torpedo Service")
  • I've updated the CODEOWNERS file with my username
  • I've added documentation to the README.md file for my package (if applicable)
  • I've tested running this in one way or another (explain above)
  • I have not used GenAI to generate any significant portions of this code

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