Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing virtual keywords to SystemInterface methods #1888

Conversation

TakashiSato
Copy link
Contributor

@TakashiSato TakashiSato commented Nov 26, 2024

This PR fixes an issue where the on_export_state_interfaces and on_export_command_interfaces functions in the SystemInterface class were missing the virtual keyword, even though they should be overrideable according to the migration.rst : Custom export of Command-/StateInterfaces.

In contrast, the ActuatorInterface and SensorInterface classes already have the virtual keyword for these functions, as shown in the following references:

virtual std::vector<StateInterface::ConstSharedPtr> on_export_state_interfaces()

virtual std::vector<CommandInterface::SharedPtr> on_export_command_interfaces()

virtual std::vector<StateInterface::ConstSharedPtr> on_export_state_interfaces()

This PR ensures consistency across these interfaces and aligns with the intended design outlined in the documentation.

Let me know if you need additional information or adjustments!

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

You are right! Thanks for fixing it

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
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.

Thank you so much!

@bmagyar bmagyar merged commit e284af2 into ros-controls:master Nov 27, 2024
17 of 19 checks passed
@TakashiSato TakashiSato deleted the feature/fix_missing_virtual_of_system_interface branch November 28, 2024 00:57
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.

4 participants