Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[galactic] Declare parameters uninitialized (#1673) #1681

Merged
merged 1 commit into from
May 20, 2021

Conversation

jacobperron
Copy link
Member

Backport #1673 to Galactic.

I've released this change into Rolling and haven't noticed any regressions. The only code I found that this affects is in navigation2 and I've got a patch there: ros-navigation/navigation2#2347

* Declare parameters uninitialized

Fixes #1649

Allow declaring parameters without an initial value or override.
This was possible prior to Galactic, but was made impossible since we started enforcing the types of parameters in Galactic.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove assertion

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Throw NoParameterOverrideProvided exception if accessed before initialized

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test getting static parameter after it is set

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Do not throw on access of uninitialized dynamically typed parameter

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Rename exception type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unused exception type

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Uncrustify

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron changed the title Declare parameters uninitialized (#1673) [galactic] Declare parameters uninitialized (#1673) May 19, 2021
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

For the record: We discussed this change at the ROS 2 meeting, and decided to grant an exception to the core freeze for Galactic due in part to the limited scope of downstream usage and the return to previously established behavior that this change enables.

@jacobperron
Copy link
Member Author

jacobperron commented May 20, 2021

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

@SteveMacenski
Copy link
Collaborator

@jacobperron its my plan to release nav2 shortly. I'm going to assume from the 2 approvals here that this is on its way so that if I merge the complementary Nav2 PR, we should be good to go?

@jacobperron
Copy link
Member Author

@SteveMacenski That's right, we should go good to go.

@jacobperron
Copy link
Member Author

macOS warnings are unrelated.

@jacobperron jacobperron merged commit 5a09a46 into galactic May 20, 2021
@delete-merged-branch delete-merged-branch bot deleted the jacob/backport_1673 branch May 20, 2021 21:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants