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

Gpio command controller (backport #1251) #1372

Draft
wants to merge 2 commits into
base: humble
Choose a base branch
from

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Nov 18, 2024

As I discussed with @christophfroehlich I've opened new pr with gpio controller.

This PR is a follow-up to the thread and implements a controller to send commands to GPIO interfaces, allowing specific command interfaces for each GPIO.

I have made the following changes compared to the original PR:

  • I utilized the 'DynamicJointState' message for both the command and state interfaces (this allows sending commands with specific GPIO values without having to worry about the order of ports in the command message).
  • I added a parameters file, and the new input parameters file will look something like this:
gpios:
  - gpio1
  - gpio2
command_interfaces:
  - gpio1:
    - ports:
      - dig1
      - dig2
  - gpio2:
    - ports:
      - ana1
  • I've done significant refactoring and have added many new UTs.

This is an automatic backport of pull request #1251 done by [Mergify](https://mergify.com).

---------

Co-authored-by: m.bednarczyk <[email protected]>
Co-authored-by: Maciej Bednarczyk <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
Co-authored-by: Christoph Fröhlich <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
Co-authored-by: Sai Kishor Kothakota <[email protected]>
(cherry picked from commit 0590c6a)

# Conflicts:
#	doc/release_notes.rst
Copy link
Contributor Author

mergify bot commented Nov 18, 2024

Cherry-pick of 0590c6a has failed:

On branch mergify/bp/humble/pr-1251
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit 0590c6a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   doc/controllers_index.rst
	new file:   gpio_controllers/CMakeLists.txt
	new file:   gpio_controllers/doc/userdoc.rst
	new file:   gpio_controllers/gpio_controllers_plugin.xml
	new file:   gpio_controllers/include/gpio_controllers/gpio_command_controller.hpp
	new file:   gpio_controllers/include/gpio_controllers/visibility_control.h
	new file:   gpio_controllers/package.xml
	new file:   gpio_controllers/src/gpio_command_controller.cpp
	new file:   gpio_controllers/src/gpio_command_controller_parameters.yaml
	new file:   gpio_controllers/test/test_gpio_command_controller.cpp
	new file:   gpio_controllers/test/test_load_gpio_command_controller.cpp
	modified:   ros2_controllers/package.xml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   doc/release_notes.rst

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot added the conflicts label Nov 18, 2024
@destogl
Copy link
Member

destogl commented Nov 18, 2024

Can it be that we are missing some include that is implicitly added to the rolling?

In file included from /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/gpio_controllers/src/gpio_command_controller.cpp:15:
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/gpio_controllers/include/gpio_controllers/gpio_command_controller.hpp:94:35: error: ‘ComponentInfo’ is not a member of ‘hardware_interface’
     94 |   std::vector<hardware_interface::ComponentInfo> get_gpios_from_urdf() const;
        |                                   ^~~~~~~~~~~~~
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/gpio_controllers/include/gpio_controllers/gpio_command_controller.hpp:94:48: error: template argument 1 is invalid
     94 |   std::vector<hardware_interface::ComponentInfo> get_gpios_from_urdf() const;
        |                                                ^
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/gpio_controllers/include/gpio_controllers/gpio_command_controller.hpp:94:48: error: template argument 2 is invalid
  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/gpio_controllers/src/gpio_command_controller.cpp:193:48: error: no declaration matches ‘std::vector<hardware_interface::ComponentInfo> gpio_controllers::GpioCommandController::get_gpios_from_urdf() const’

@christophfroehlich
Copy link
Contributor

@Wiktor-99 maybe you can have a look? Are there includes missing, or has the API changed? You can simply open a PR to the mergify/bp/humble/pr-1251 branch.

@Wiktor-99
Copy link
Contributor

Yes, I'll check it right away

@Wiktor-99
Copy link
Contributor

Wiktor-99 commented Nov 18, 2024

It seems that there is more:

  • Firstly the hardware_interface/hardware_info.hpp include is missing but it's minor
  • In humble controller doesn't get the urdf during initialization (this behavior was introduce in the Pass URDF to controllers on init ros2_control#1088), but its api breaking
  • There is also problem with cmake's -Werror=shadow flag (there is some problem with pluginlib), also minor

@christophfroehlich
Copy link
Contributor

It seems that there is more:

* Firstly the hardware_interface/hardware_info.hpp include is missing but it's minor

👍

* In humble controller doesn't get the urdf during initialization (this behavior was introduce in the [Pass URDF to controllers on init ros2_control#1088](https://github.com/ros-controls/ros2_control/pull/1088)), but its api breaking

you are right, we would have to change the strategy and only support explicitly given state interfaces.

* There is also problem with cmake's -Werror=shadow flag (there is some problem with pluginlib), also minor

I had this error already, but I can't find it any more where I solved it and how (probably by just deactivating the -Werror=shadow)

If you have time to work on that, very welcome. Otherwise we close this backport, everyone can compile the rolling stack from source on humble to have the latest features.

@christophfroehlich christophfroehlich marked this pull request as draft November 19, 2024 15:44
@Wiktor-99
Copy link
Contributor

Frankly, there is not much to align. I'll provide limited solution in few days (with explicitly given state interfaces).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants