mock_generic_system tests on humble use deprecated format #1931

christophfroehlich · 2024-12-11T09:15:41Z

Background

Overview of your issue here.

TestGenericSystem.load_generic_system_2dof

2024-12-11T02:54:12.2328086Z 6: [WARN] [1733885652.117849777] [mock_generic_system]: Parsing of optional initial interface values failed or uses a deprecated format. Add initial values for every state interface in the ros2_control.xacro. For example: 
2024-12-11T02:54:12.2328841Z 6: <state_interface name="velocity"> 
2024-12-11T02:54:12.2329092Z 6:   <param name="initial_value">0.0</param> 
2024-12-11T02:54:12.2329335Z 6: </state_interface>

Referencing also #629 -> Maybe one can have a look if the documentation is up-to-date already.

Instructions

Hi, this is a good-first-issue issue. This means we've worked to make it more legible to people who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

We're interested in helping you take the first step, and can answer questions and help you out along the way. Note that we're especially interested in contributions from underrepresented groups!

We know that creating a pull request is the biggest barrier for new contributors. This issue is for you 💝

If you have contributed before, consider leaving this PR for someone new, and looking through our general bug issues. Thanks!

🤔 What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

📋 Step by Step

🙋 Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!
🗄️ Create a local workspace for making your changes and testing following these instructions, for Step 3 use "Download Source Code" section with these instructions.
🍴 Fork the repository using the handy button at the top of the repository page and clone it into ~/ws_ros2_control/src/ros-controls/ros2_control, here is a guide that you can follow (You will have to remove or empty the existing ros2_control folder before cloning your own fork)
Checkout a new branch using git checkout -b <branch_name>
🤖 Apply pre-commit auto formatting, by running pip3 install pre-commit and running pre-commit install in the ros2_control repo.
💾 Commit and Push your changes
🔀 Start a Pull Request to request to merge your code into master. There are two ways that you can start a pull request:

If you are not familiar with GitHub or how to create a pull request, here is a guide you can follow on how GitHub works.

🏁 Done Ask in comments for a review :)

Is someone else already working on this?

🔗- We encourage contributors to link to the original issue in their pull request so all users can easily see if someone's already started on it.

👥- If someone seems stuck, offer them some help!

🤔❓ Questions?

Don’t hesitate to ask questions or to get help if you feel like you are getting stuck. For example leave a comment below!
Furthermore, you find helpful resources here:

Good luck with your first issue!

The text was updated successfully, but these errors were encountered:

kermitsgonnakermicide · 2024-12-11T16:22:01Z

Hi - would like to work on this if possible

christophfroehlich · 2024-12-11T19:36:47Z

Hi - would like to work on this if possible

Great. You can easily reproduce this locally with colcon test --packages-select hardware_interface (on humble branch!)

christophfroehlich · 2024-12-11T19:44:08Z

You find all necessary changes in this PR, have a look at the URDF constants in test_generic_system.cpp

kermitsgonnakermicide · 2024-12-13T16:43:37Z

will unfortunately have to unassign as I cannot get humble installed on 22.04 for the life of me, hope this isnt an issue

christophfroehlich · 2024-12-13T19:29:31Z

What is the problem, maybe I can help?

kumar-sanjeeev · 2024-12-15T12:46:44Z

Hi @christophfroehlich, I'd like to take this up.

I followed the comments and built the source code from the humble branch using the documentation, setting ROS_DISTRO=humble. After running colcon test --packages-select hardware_interface, I couldn’t reproduce the warning mentioned in the description.

Output in my case:

colcon test --packages-select hardware_interface
Starting >>> hardware_interface
Finished <<< hardware_interface [2.33s]          

Summary: 1 package finished [3.11s]

let me know, if I am doing something wrong ?

christophfroehlich · 2024-12-15T13:39:32Z

The default settings of colcon do not show any output of the running tests. You can either open the logfile in log/latest_test/hardware_interface/stdout_stderr.log or setup colcon for example with
colcon test --event-handlers console_direct+ --packages-select hardware_interface

kumar-sanjeeev · 2024-12-15T16:28:34Z

The default settings of colcon do not show any output of the running tests. You can either open the logfile in log/latest_test/hardware_interface/stdout_stderr.log or setup colcon for example with colcon test --event-handlers console_direct+ --packages-select hardware_interface

@christophfroehlich thanks for explaining.

I worked on the issue and added initial values for every state interface in the ros2_control.xacro to suppress the warning. However, I still see the warning for hardware_system_2dof_with_sensor_<other-variants>. I believe this is because I haven’t specified the initial_value for tcp_force_sensor.

    hardware_system_2dof_with_sensor_mock_command_ =
      R"(
  <ros2_control name="GenericSystem2dof" type="system">
    <hardware>
      <plugin>mock_components/GenericSystem</plugin>
      <param name="mock_sensor_commands">true</param>
    </hardware>
    <joint name="joint1">
      <command_interface name="position"/>
      <command_interface name="velocity"/>
      <state_interface name="position">
        <param name="initial_value">0.0</param>
      </state_interface>
      <state_interface name="velocity">
        <param name="initial_value">0.0</param>
      </state_interface>
    </joint>
    <joint name="joint2">
      <command_interface name="position"/>
      <command_interface name="velocity"/>
      <state_interface name="position">
        <param name="initial_value">0.0</param>
      </state_interface>
      <state_interface name="velocity">
        <param name="initial_value">0.0</param>
      </state_interface>
    </joint>
    <sensor name="tcp_force_sensor">
      <state_interface name="fx"/>
      <state_interface name="fy"/>
      <state_interface name="tx"/>
      <state_interface name="ty"/>
      <param name="frame_id">kuka_tcp</param>
    </sensor>
  </ros2_control>
)";

I can set initial_value=0.0 for these as well, but I’d also need to update the respective ASSERT statements. Would it be okay to do that?

## Currently implemented
EXPECT_TRUE(std::isnan(sfx_s.get_value()));
EXPECT_TRUE(std::isnan(sfy_s.get_value()));
EXPECT_TRUE(std::isnan(stx_s.get_value()));
EXPECT_TRUE(std::isnan(sty_s.get_value()));
ASSERT_TRUE(std::isnan(j1p_c.get_value()));
ASSERT_TRUE(std::isnan(j2p_c.get_value()));

## if initial_value defined
ASSERT_EQ(0.0, sfx_s.get_value());
ASSERT_EQ(0.0, sfy_s.get_value());
ASSERT_EQ(0.0, stx_s.get_value());
ASSERT_EQ(0.0, sty_s.get_value());

I also reviewed this PR, here no initial_value was defined for tcp_force_sensor, hence no modification for ASSERT statements.

let me know your thoughts ?

christophfroehlich · 2024-12-15T18:36:38Z

you are right, on humble there is a warning if no initial_value is given at all, added with #623

ros2_control/hardware_interface/src/mock_components/generic_system.cpp

Lines 721 to 731 in 38a5600

    
           // Initialize the value in old way with warning message 
        
           auto it2 = component.parameters.find("initial_" + interface.name); 
        
           if (it2 != component.parameters.end()) 
        
           { 
        
             states[index][i] = hardware_interface::stod(it2->second); 
        
             print_hint = true; 
        
           } 
        
           else 
        
           { 
        
             print_hint = true; 
        
           }

From jazzy on, we don't throw a warning if there is not initial_value but just initialize it with NaN.
Could you please remove the print_hint=true if no initial value is given? I don't get the point why we need to enforce this.

kumar-sanjeeev · 2024-12-15T19:37:04Z

I adjusted print_hint=false, which reduced the warnings to three. As far as I can tell, these warnings are expected to appear. If that’s the case, I’ll submit PR.

3 WARN cases

# [WARN 1]
6: [ RUN      ] TestGenericSystem.disabled_commands_flag_is_active
6: [INFO] [1734290888.448726480] [resource_manager]: Loading hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450171585] [resource_manager]: Initialize hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450601666] [resource_manager]: Successful initialization of hardware 'GenericSystem2dof'
6: [INFO] [1734290888.450912858] [resource_manager]: 'configure' hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450936887] [resource_manager]: Successful 'configure' of hardware 'GenericSystem2dof'
6: [INFO] [1734290888.450949810] [resource_manager]: 'activate' hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450958681] [resource_manager]: Successful 'activate' of hardware 'GenericSystem2dof'
6: [WARN] [1734290888.451000593] [mock_generic_system]: Command propagation is disabled - no values will be returned!

#[WARN 2]
6: [ RUN      ] TestGenericSystem.generic_system_2dof_functionality_with_offset_custom_interface_missing
6: [INFO] [1734290888.156293601] [resource_manager]: Loading hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.157406139] [resource_manager]: Initialize hardware 'GenericSystem2dof' 
6: [WARN] [1734290888.157744434] [mock_generic_system]: Custom interface with following offset 'actual_position' does not exist. Offset will not be applied

#[WARN 3]
6: [ RUN      ] TestGenericSystem.prepare_command_mode_switch_works_with_all_example_tags
...
6: [WARN] [1734290888.786956794] [mock_generic_system]: Custom interface with following offset 'actual_position' does not exist. Offset will not be applied
...

christophfroehlich · 2024-12-15T20:54:10Z

I adjusted print_hint=false, which reduced the warnings to three.
just delete the whole else branch.

As far as I can tell, these warnings are expected to appear. If that’s the case, I’ll submit PR.

3 WARN cases

# [WARN 1]
6: [ RUN      ] TestGenericSystem.disabled_commands_flag_is_active
6: [INFO] [1734290888.448726480] [resource_manager]: Loading hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450171585] [resource_manager]: Initialize hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450601666] [resource_manager]: Successful initialization of hardware 'GenericSystem2dof'
6: [INFO] [1734290888.450912858] [resource_manager]: 'configure' hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450936887] [resource_manager]: Successful 'configure' of hardware 'GenericSystem2dof'
6: [INFO] [1734290888.450949810] [resource_manager]: 'activate' hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.450958681] [resource_manager]: Successful 'activate' of hardware 'GenericSystem2dof'
6: [WARN] [1734290888.451000593] [mock_generic_system]: Command propagation is disabled - no values will be returned!

#[WARN 2]
6: [ RUN      ] TestGenericSystem.generic_system_2dof_functionality_with_offset_custom_interface_missing
6: [INFO] [1734290888.156293601] [resource_manager]: Loading hardware 'GenericSystem2dof' 
6: [INFO] [1734290888.157406139] [resource_manager]: Initialize hardware 'GenericSystem2dof' 
6: [WARN] [1734290888.157744434] [mock_generic_system]: Custom interface with following offset 'actual_position' does not exist. Offset will not be applied

#[WARN 3]
6: [ RUN      ] TestGenericSystem.prepare_command_mode_switch_works_with_all_example_tags
...
6: [WARN] [1734290888.786956794] [mock_generic_system]: Custom interface with following offset 'actual_position' does not exist. Offset will not be applied
...

seems to be fine, please open the PR.

christophfroehlich added the good first issue label Dec 11, 2024

christophfroehlich assigned kermitsgonnakermicide Dec 11, 2024

kermitsgonnakermicide removed their assignment Dec 13, 2024

christophfroehlich assigned kumar-sanjeeev Dec 15, 2024

christophfroehlich linked a pull request Dec 15, 2024 that will close this issue

Update initial_value parameters of generic_system tests #1943

Merged

christophfroehlich closed this as completed Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mock_generic_system tests on humble use deprecated format #1931

mock_generic_system tests on humble use deprecated format #1931

christophfroehlich commented Dec 11, 2024

kermitsgonnakermicide commented Dec 11, 2024

christophfroehlich commented Dec 11, 2024

christophfroehlich commented Dec 11, 2024

kermitsgonnakermicide commented Dec 13, 2024

christophfroehlich commented Dec 13, 2024

kumar-sanjeeev commented Dec 15, 2024

christophfroehlich commented Dec 15, 2024

kumar-sanjeeev commented Dec 15, 2024 •

edited

Loading

christophfroehlich commented Dec 15, 2024

kumar-sanjeeev commented Dec 15, 2024

christophfroehlich commented Dec 15, 2024

mock_generic_system tests on humble use deprecated format #1931

mock_generic_system tests on humble use deprecated format #1931

Comments

christophfroehlich commented Dec 11, 2024

Background

Instructions

🤔 What you will need to know.

📋 Step by Step

Is someone else already working on this?

🤔❓ Questions?

kermitsgonnakermicide commented Dec 11, 2024

christophfroehlich commented Dec 11, 2024

christophfroehlich commented Dec 11, 2024

kermitsgonnakermicide commented Dec 13, 2024

christophfroehlich commented Dec 13, 2024

kumar-sanjeeev commented Dec 15, 2024

christophfroehlich commented Dec 15, 2024

kumar-sanjeeev commented Dec 15, 2024 • edited Loading

christophfroehlich commented Dec 15, 2024

kumar-sanjeeev commented Dec 15, 2024

christophfroehlich commented Dec 15, 2024

kumar-sanjeeev commented Dec 15, 2024 •

edited

Loading