Skip to content

broadcaster for magnetic field values from a magnetometer#2214

Open
christianrauch wants to merge 2 commits intoros-controls:masterfrom
christianrauch:magnetometer_broadcaster
Open

broadcaster for magnetic field values from a magnetometer#2214
christianrauch wants to merge 2 commits intoros-controls:masterfrom
christianrauch:magnetometer_broadcaster

Conversation

@christianrauch
Copy link
Copy Markdown

@christianrauch christianrauch commented Mar 15, 2026

This PR adds a broadcaster for magnetometers via the semantic_components::MagneticFieldSensor, similar to how this is done for other semantic components (semantic_components::IMUSensor, semantic_components::ForceTorqueSensor).


  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

@christianrauch christianrauch marked this pull request as ready for review March 15, 2026 21:33
@thedevmystic
Copy link
Copy Markdown
Contributor

thedevmystic commented Mar 17, 2026

Hey @christianrauch, I see its some usage. Let's see what maintainers have to say.

And another thing, why is the class declaration is in implementation file (everything is in .cpp file).

It'll be really better if declaration was in .hpp and implementation was in .cpp

@christianrauch christianrauch force-pushed the magnetometer_broadcaster branch from 9d9b9fc to 22c1d40 Compare March 17, 2026 10:24
bmagyar
bmagyar previously approved these changes Mar 20, 2026
Copy link
Copy Markdown
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

The hpp/cpp comment is a good catch indeed but since this is delivered as a plugin I'm not too concerned about it. Thanks for pointing it out @thedevmystic

@christianrauch
Copy link
Copy Markdown
Author

I addressed the one comment about the maintainers and fixed one CI issue from my side.

The CI shows a couple of other issues, such as missing dependencies on controller_manager and backward_ros, which I suspect come from other packages since the magnetometer_broadcaster is not using controller_manager and backward_ros is already defined.

@christianrauch christianrauch force-pushed the magnetometer_broadcaster branch from cddcb89 to 45bea18 Compare March 25, 2026 12:12
@christianrauch christianrauch requested a review from bmagyar March 25, 2026 12:13
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.

3 participants