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

deleting parameters doesn't actually remove them #487

Closed
dirk-thomas opened this issue Jun 1, 2018 · 1 comment
Closed

deleting parameters doesn't actually remove them #487

dirk-thomas opened this issue Jun 1, 2018 · 1 comment
Assignees
Labels
bug Something isn't working in review Waiting for review (Kanban column)

Comments

@dirk-thomas
Copy link
Member

When deleting a parameter from a node in C++ it is not being removed from the storage. Instead a parameter with the value type "not set" is being kept.

For future list requests that is confusing since the parameter was explicitly deleted. It should be removed from the entirely.

The line in question currently unconditionally sets the parameter in the map:

In the case where the type is "not set" it should instead remove the entry.

@dirk-thomas dirk-thomas added the bug Something isn't working label Jun 1, 2018
@dirk-thomas dirk-thomas self-assigned this Jun 1, 2018
@dirk-thomas
Copy link
Member Author

dirk-thomas commented Jun 1, 2018

The existing comment mentions that the parameter is intentionally kept in the map with a "not set" type:

// it is not necessary to erase the parameter from parameters_
// because the new value for this key (p.get_name()) will be a
// Parameter with type "NOT_SET"

Imo that is not a good idea:

  • First that makes it impossible to remove a once added parameter completely and the system can never get back into the previous state (before the parameter was added).
  • Second keeping a not set parameter around is not useful since the semantic is intended to be the same as if the parameter doesn't exist at all.
  • Third with the not set parameter being kept the caller has to actively filter not set parameter from a list response (which can't be done if only the name is transferred and the data has to be communicated unnecessarily).

@dirk-thomas dirk-thomas added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 1, 2018
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
* Increase MAX_STRING_SIZE

It's too short for string length.
It occurs the error when the string field in yaml files are too long....

Signed-off-by: Hyunseok Yang <hyunseok7.yang@lge.com>

* update test to match increased limit

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in review Waiting for review (Kanban column)
Projects
None yet
Development

No branches or pull requests

1 participant