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

Change all read_only parameters to false to avoid issues when using s… #973

Closed

Conversation

saikishor
Copy link
Member

@saikishor saikishor commented Jan 11, 2024

Related to the issue reported: #966

We need to change all the read_only to false for the moment to avoid issues when using spawners with parameter file. The read-only parameters cannot be loaded as they are expected to exist at the startup of the LifeCycle node. Spawners tend to load the passed parameter files before configuring the controllers.

The above deduction is made from the following references:

http://design.ros2.org/articles/qos_configurability.html#read-only-parameters

https://docs.ros.org/en/galactic/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Parameters/Understanding-ROS2-Parameters.html#ros2-param-load

@christophfroehlich @bmagyar It would be great to get your insight on this

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fdd6142) 47.22% compared to head (3e4a3cb) 47.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #973      +/-   ##
==========================================
+ Coverage   47.22%   47.24%   +0.02%     
==========================================
  Files          41       41              
  Lines        3871     3871              
  Branches     1827     1827              
==========================================
+ Hits         1828     1829       +1     
  Misses        775      775              
+ Partials     1268     1267       -1     
Flag Coverage Δ
unittests 47.24% <ø> (+0.02%) ⬆️

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

see 1 file with indirect coverage changes

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.

Unfortunately we have to fix this now in this way :/ Discussion of a long-term fix in the linked PR

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron. labels Jan 11, 2024
@christophfroehlich
Copy link
Contributor

closing, because it seems that we can fix this upstream. see ros-controls/ros2_control#1293 and #978

@saikishor saikishor deleted the change_all_read_only_to_false branch January 17, 2024 10:44
henrygerardmoore pushed a commit to henrygerardmoore/ros2_controllers that referenced this pull request Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants