Skip to content

Conversation

@christophfroehlich
Copy link
Member

@christophfroehlich christophfroehlich commented Oct 18, 2025

There were several issues, introduced with #2571:

  • as reported in Generic system assumes acceleration interface, though none is present #2694, the component tried to set states even if they didn't exist
  • while writing test cases for the above, I realized that the combination of position-only state with velocity-only command was not working, too. The problem here was that the internal variable of the velocity state was never set and thus, no integration happened. The behavior change came from the fact that the initial value of states is NaN if not explicitly set, and in this case there is no velocity state which could be parameterized.
  • The comment "backward integration" was wrong, backward Euler integration is $x(k+1) = x(k)+T_s f(k+1)$, but forward euler integration $x(k+1) = x(k)+T_s f(k)$ was implemented. Doing real implicit Euler fixes the above, but changes obviously the behavior a bit, see the changes in simple_dynamics_pos_vel_acc_control_modes_interfaces. But IMHO this is fine, unless anyone has implemented a similar test pattern with this component.

Fixes #2694

@christophfroehlich christophfroehlich changed the title Fix dynamics calculation of mock hardware Fix dynamics calculation of GenericSystem component Oct 18, 2025
@codecov
Copy link

codecov bot commented Oct 18, 2025

Codecov Report

❌ Patch coverage is 97.91667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.40%. Comparing base (6d27e9d) to head (a506539).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...e_interface/src/mock_components/generic_system.cpp 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2705      +/-   ##
==========================================
+ Coverage   89.33%   89.40%   +0.07%     
==========================================
  Files         151      151              
  Lines       17147    17225      +78     
  Branches     1432     1431       -1     
==========================================
+ Hits        15318    15400      +82     
+ Misses       1247     1246       -1     
+ Partials      582      579       -3     
Flag Coverage Δ
unittests 89.40% <97.91%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rface/test/mock_components/test_generic_system.cpp 99.76% <100.00%> (+0.05%) ⬆️
...e_interface/src/mock_components/generic_system.cpp 86.33% <87.50%> (+1.54%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@saikishor saikishor merged commit 8ad5ebb into master Oct 19, 2025
16 of 17 checks passed
@saikishor saikishor deleted the fix/mock/dynamics branch October 19, 2025 01:57
@christophfroehlich christophfroehlich added the backport-kilted Triggers PR backport to ROS 2 kilted. label Oct 19, 2025
mergify bot pushed a commit that referenced this pull request Oct 19, 2025
@christophfroehlich christophfroehlich added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Oct 19, 2025
mergify bot pushed a commit that referenced this pull request Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generic system assumes acceleration interface, though none is present

3 participants