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

Fix echo sometimes printing ..... #135

Merged
merged 1 commit into from
Aug 16, 2018
Merged

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Aug 16, 2018

Fixes #129

If truncate_length was less than 3 then the ... added to a list or sequence would be truncated. This fixes the issue by refactoring _convert_value to only have one block of conditionals.

I also changed bytes from being displayed as "b'somebytestring'" to a string where every byte has been converted with chr.

To test this PR manually

ros2 run image_tools cam2image -b

and

ros2 topic echo /image sensor_msgs/Image -l 2

CI

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

connects to #129

@sloretz sloretz added bug Something isn't working in review Waiting for review (Kanban column) labels Aug 16, 2018
Copy link
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @sloretz

@sloretz sloretz mentioned this pull request Aug 16, 2018
12 tasks
@sloretz sloretz merged commit 10c4759 into master Aug 16, 2018
@sloretz sloretz deleted the deduplicate_echo_ellipsis branch August 16, 2018 15:51
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Aug 16, 2018
# convert each key and value in the mapping
new_value = {} if isinstance(value, dict) else OrderedDict()
for k, v in value.items():
new_value[_convert_value(k)] = _convert_value(
new_value[_convert_value(k, truncate_length=truncate_length)] = _convert_value(
Copy link
Member

Choose a reason for hiding this comment

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

I do see a major problem with this change: the keys in dictionary are now also being truncated which was not the case before. This will potentially lead to different keys being reduce to the same shortened key and therefore "loose" data.

Copy link
Member

Choose a reason for hiding this comment

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

good point !

esteve pushed a commit to esteve/ros2cli that referenced this pull request Dec 16, 2022
* Support passing list of parameter file paths

* Support pathlib Paths

* WIP tests

* Move tests to a separate package

* Test parameters get resolved

* Use EnvironmentVariable instead of ThisLaunchFileDir

Doesn't work inside scripts

* Env var needs to be specified in calling context not node itself

* Add tests for invalid parameter arguments

* Update other tests for consistency

* Fixup

* Update wording when not a file

* typo
esteve pushed a commit to esteve/ros2cli that referenced this pull request Dec 16, 2022
* show error strings as part of the flake8 test

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* call new API which exposes the errors

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ros2topic] nested calls to convert_value results in inconsistent printing format
3 participants