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.

Sign up for GitHub

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

Jump to bottom

Fix automatically declared parameters descriptor type #853

Merged
merged 2 commits into from
Nov 29, 2021
Merged

Fix automatically declared parameters descriptor type #853

merged 2 commits into from
Nov 29, 2021

Conversation

ivanpauno
Copy link
Member

Fixes ros2/ros2#1173.
Fixes #829.

@ivanpauno
fix automatically declared parameters descriptor type
ea1d169 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the bug Something isn't working label Nov 24, 2021
@ivanpauno ivanpauno self-assigned this Nov 24, 2021
@ivanpauno
please flake8
be6b9c0 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
This was referenced Nov 24, 2021
Closed
Closed
@adityapande-1995
Copy link

adityapande-1995 commented Nov 24, 2021 •
edited
Loading

Tested with rolling, resolves the issue. Need to backport this to galactic (which the issue references) as well. Maybe the mergify bot can help ?

@ivanpauno
Copy link
Member Author

Tested with rolling, resolves the issue. Need to backport this to galactic (which the issue references) as well. Maybe the mergify bot can help ?

Yes, we should use the mergify bot after merging this PR to master.

@ivanpauno
Copy link
Member Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ivanpauno
Copy link
Member Author

  • Windows Build Status

@ivanpauno
Copy link
Member Author

Same failures can be seen in windows repeated nightly jobs (e.g. this one).
Going in!

@ivanpauno ivanpauno merged commit 4d97b4e into master Nov 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/fix-automatically-declared-parameters-type branch November 29, 2021 18:53
@ivanpauno
Copy link
Member Author

@Mergifyio backport galactic

mergify bot pushed a commit that referenced this pull request Nov 29, 2021
@ivanpauno
Fix automatically declared parameters descriptor type. (#853)
7b91899 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
(cherry picked from commit 4d97b4e)
@mergify mergify bot mentioned this pull request Nov 29, 2021
Merged
@mergify
Copy link
Contributor

mergify bot commented Nov 29, 2021

backport galactic

✅ Backports have been created

ivanpauno pushed a commit that referenced this pull request Nov 30, 2021
@mergify
Fix automatically declared parameters descriptor type. (backport #853) (
8daacdd 
#854)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
jmarsik added a commit to km-robotics/rclpy that referenced this pull request May 12, 2022
@jmarsik
fixed order of steps when handling parameters on node creation - auto…
cbf5da6 
…matic declaring from overrides should happen BEFORE creating the clock and time source (because those will try to declare use_sim_time parameter); otherwise "already declared" error will happen when use_sim_time is supplied; see commit 8daacdd and ros2#854 and ros2#853
jmarsik added a commit to km-robotics/rclpy that referenced this pull request May 12, 2022
@jmarsik
corrected fix for use_sim_time and automatic declaration of parameter…
83203a9 
…s from overrides (previous commit), order cannot be changed, we have to ignore already declared use_sim_time parameter; should finally fix "already declared" error that will happen when use_sim_time is supplied; see commit 8daacdd and ros2#854 and ros2#853
galou pushed a commit to galou/rclpy that referenced this pull request Jan 25, 2023
@jmarsik @galou
Fix handling parameters on node creation
99f0034 
Fix handling parameters on node creation. We have to ignore already
declared use_sim_time parameter; should finally fix "already declared"
error that will happen when use_sim_time is supplied; see commit
8daacdd and ros2#854 and ros2#853.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adityapande-1995 adityapande-1995 adityapande-1995 approved these changes

@jacobperron jacobperron Awaiting requested review from jacobperron

Assignees

@ivanpauno ivanpauno

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot set parameters declared in YAML file ROS2 Type Inference from YAML-Parameters
2 participants
@ivanpauno @adityapande-1995