Skip to content

[JTC] Make most parameters read-only#771

Merged
bmagyar merged 3 commits intoros-controls:masterfrom
christophfroehlich:jtc/remove_dynamic_dof
Sep 11, 2023
Merged

[JTC] Make most parameters read-only#771
bmagyar merged 3 commits intoros-controls:masterfrom
christophfroehlich:jtc/remove_dynamic_dof

Conversation

@christophfroehlich
Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich commented Sep 9, 2023 •
edited
Loading

This is the first part of implementing the suggestions from #394 to JTC.

I made all parameters read-only, which aren't updated anyways or shouldn't be reconfigureable like DoF.

Remaining dynamic parameters (open for discussion):

Changes to get the tests running again:

  • The tests failed because they heavily relied on parameter reconfiguration -> Now use parameter overrides before initializing the node.
  • I removed the yaml file from the test/config folder because it wasn't used at all
  • I removed the use_local_param from the testable class, I think it made no difference (any more?)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 9, 2023 •
edited
Loading

Codecov Report

Merging #771 (f8bf3e8) into master (e7f9962) will decrease coverage by 4.90%.
Report is 366 commits behind head on master.
The diff coverage is 25.91%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #771      +/-   ##
==========================================
- Coverage   35.78%   30.88%   -4.90%     
==========================================
  Files         189        7     -182     
  Lines       17570      832   -16738     
  Branches    11592      505   -11087     
==========================================
- Hits         6287      257    -6030     
+ Misses        994      133     -861     
+ Partials    10289      442    -9847
Flag Coverage Δ
unittests 30.88% <25.91%> (-4.90%) ⬇️

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

Files Changed Coverage Δ
...ontroller/test/test_load_diff_drive_controller.cpp 11.11% <0.00%> (ø)
...ive_controller/test/test_diff_drive_controller.cpp 12.66% <7.81%> (ø)
diff_drive_controller/src/odometry.cpp 42.16% <11.11%> (ø)
diff_drive_controller/src/speed_limiter.cpp 46.55% <11.11%> (ø)
...iff_drive_controller/src/diff_drive_controller.cpp 47.13% <46.94%> (ø)
...de/diff_drive_controller/diff_drive_controller.hpp 100.00% <100.00%> (ø)
...troller/include/diff_drive_controller/odometry.hpp 100.00% <100.00%> (ø)

... and 189 files with indirect coverage changes

@christophfroehlich christophfroehlich mentioned this pull request Sep 9, 2023
Closed
saikishor
saikishor approved these changes Sep 11, 2023
View reviewed changes
Copy link
Copy Markdown
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.

LGTM

@bmagyar bmagyar merged commit 4e917db into ros-controls:master Sep 11, 2023
@christophfroehlich christophfroehlich deleted the jtc/remove_dynamic_dof branch September 14, 2023 16:57
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Oct 21, 2023
@christophfroehlich
[JTC] Make most parameters read-only (ros-controls#771)
4a297b1
@christophfroehlich christophfroehlich mentioned this pull request Oct 21, 2023
Merged
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Oct 21, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
55f5bcb
@christophfroehlich christophfroehlich mentioned this pull request Oct 22, 2023
Merged
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 11, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
1038579
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 15, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
1a36d0c
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 17, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
e358b8f
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 27, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
c26a91c
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 29, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
c49bf4c
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 30, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
7d9d076
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Dec 5, 2023
@christophfroehlich
[JTC] Make most parameters read-only (backport ros-controls#771)
6c3b73e
bmagyar pushed a commit that referenced this pull request Dec 6, 2023
@christophfroehlich @bmagyar
[JTC] Make most parameters read-only (backport #771)
dc859d4
@Noel215 Noel215 mentioned this pull request Jan 10, 2024
Closed
@Svastits Svastits mentioned this pull request Jan 11, 2024
Merged
@christophfroehlich christophfroehlich mentioned this pull request May 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bmagyar bmagyar bmagyar approved these changes

@saikishor saikishor saikishor approved these changes

@arne48 arne48 Awaiting requested review from arne48

@destogl destogl Awaiting requested review from destogl

@harderthan harderthan Awaiting requested review from harderthan

@LukasMacha97 LukasMacha97 Awaiting requested review from LukasMacha97

@progtologist progtologist Awaiting requested review from progtologist

@rosterloh rosterloh Awaiting requested review from rosterloh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@christophfroehlich @codecov-commenter @bmagyar @saikishor