Skip to content

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

Merged
jacobperron merged 1 commit intogalacticfrom
jacob/backport_1673
May 20, 2021
Merged

[galactic] Declare parameters uninitialized (#1673)#1681
jacobperron merged 1 commit intogalacticfrom
jacob/backport_1673

Conversation

@jacobperron
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

jacobperron commented May 20, 2021

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

@SteveMacenski
Copy link
Copy Markdown
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
Copy Markdown
Member Author

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

@jacobperron
Copy link
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants