Check if parameter is already declared to avoid re-declaring it. #227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A parameter can only be declared once. When implementing a LicecycleNode one would normally do all the initialization in `on_configure()` and the cleanup, deinitialization in the `on_cleanup` method. Furthermore, these transitions could potentially be called multiple times, depending on how the node is t ransitioned from the lifecycles are triggered. However, this approach doesn't currently work well with Diagnostics since, in the constructor, of `diagnostic_updater::Updater` it's declaring a static parameter without checking whether that parameter has already been declared. In subsequent calls to `on_configure` (and thus, if Diagnostics are initialized there, in subsequent calls to the `Updater` constructor, this results in the following message: ``` Original error: parameter 'diagnostic_updater.period' has already been declared ``` To avoid this, until the Foxy version, the wrapper code could explicitly call `undeclare_parameter` on the parameter that the `Updater` is declaring, but as of Galactic, undeclaring a static parameter is not allowed anymore. See e.g., ros2/rclcpp#1850. In this commit, I'm checking explicitly if the parameter has already been registered and if so, I'm just reading its value before attempting to re-declare it.
com2rng
approved these changes
Oct 19, 2022
|
lol, this PR is ~5 months old. I suppose that could also work. Feel free to verify and push a patch to this branch :) |
|
It is old, but gold. It's a very useful fix |
ralph-lange
reviewed
Nov 10, 2022
diagnostic_updater/include/diagnostic_updater/diagnostic_updater.hpp
Outdated
Show resolved
Hide resolved
ralph-lange
added a commit
to bergercookie/diagnostics
that referenced
this pull request
Nov 12, 2022
ralph-lange
added a commit
that referenced
this pull request
Nov 12, 2022
[BACKPORT - #227] Check if parameter is already declared to avoid re-declaring it
ralph-lange
added a commit
that referenced
this pull request
Nov 12, 2022
* Check if parameter is already declared to avoid re-declaring it. A parameter can only be declared once. When implementing a LicecycleNode one would normally do all the initialization in `on_configure()` and the cleanup, deinitialization in the `on_cleanup` method. Furthermore, these transitions could potentially be called multiple times, depending on how the node is t ransitioned from the lifecycles are triggered. However, this approach doesn't currently work well with Diagnostics since, in the constructor, of `diagnostic_updater::Updater` it's declaring a static parameter without checking whether that parameter has already been declared. In subsequent calls to `on_configure` (and thus, if Diagnostics are initialized there, in subsequent calls to the `Updater` constructor, this results in the following message: ``` Original error: parameter 'diagnostic_updater.period' has already been declared ``` To avoid this, until the Foxy version, the wrapper code could explicitly call `undeclare_parameter` on the parameter that the `Updater` is declaring, but as of Galactic, undeclaring a static parameter is not allowed anymore. See e.g., ros2/rclcpp#1850. In this commit, I'm checking explicitly if the parameter has already been registered and if so, I'm just reading its value before attempting to re-declare it. * Replaced try-catch with has_parameter check. Co-authored-by: Nikos Koukis <nikolaos@slamcore.com> Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A parameter can only be declared once. When implementing a LicecycleNode one
would normally do all the initialization in
on_configure()and the cleanup,deinitialization in the
on_cleanupmethod. Furthermore, these transitionscould potentially be called multiple times, depending on how the node is t
ransitioned from the lifecycles are triggered. However, this approach doesn't
currently work well with Diagnostics since, in the constructor, of
diagnostic_updater::Updaterit's declaring a static parameter without checkingwhether that parameter has already been declared. In subsequent calls to
on_configure(and thus, if Diagnostics are initialized there, in subsequentcalls to the
Updaterconstructor, this results in the following message:To avoid this, until the Foxy version, the wrapper code could explicitly call
undeclare_parameteron the parameter that theUpdateris declaring, but asof Galactic, undeclaring a static parameter is not allowed anymore. See e.g.,
ros2/rclcpp#1850.
In this commit, I'm checking explicitly if the parameter has already been
registered and if so, I'm just reading its value before attempting to re-declare
it.