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.

Sign up for GitHub

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

Jump to bottom

fix is_nice_character #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix is_nice_character #59

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dronecan/introspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _to_json_compatible_object_impl(obj):
t = dronecan.get_dronecan_data_type(obj)
if t.value_type.category == t.value_type.CATEGORY_PRIMITIVE:
def is_nice_character(ch):
if ch.is_printable() or ch.isspace():
if 32 <= ch <= 126:
Copy link
Member

Choose a reason for hiding this comment

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

why not the other way? I would prefer is_printable to the chatacter range

Copy link
Author

Choose a reason for hiding this comment

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

I would too, but actually the is_printable call triggered me an exception, and that's why I ended up patching this. I'll look for what triggered this to reproduce it, but I think the ch var sometimes came as an integer instead of a char, and then the call to is_printable method failed.

Copy link
Author

Choose a reason for hiding this comment

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

I did a quick test, with an ardupilot board (ArduCopter 4.5.1), and it fails when receiving dronecan messages with integer arrays. As an example, the message ardupilot.indication.NotifyState throws this exception:

Dronecan message received: Transfer(id=1, source_node_id=10, dest_node_id=None, transfer_priority=24, payload=ardupilot.indication.NotifyState(aux_data_type=0, aux_data=ArrayValue(type=saturated uint8[<=255], items=[4, 141]), vehicle_state=16))
File "venv\Lib\site-packages\dronecan\introspect.py", line 50, in _to_json_compatible_object_impl
if t.is_string_like and all(map(is_nice_character, obj)):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "venv\Lib\site-packages\dronecan\introspect.py", line 43, in is_nice_character
if ch.is_printable() or ch.isspace():
^^^^^^^^^^^^^^^
AttributeError: 'int' object has no attribute 'is_printable'

My understanding of this code is that is intended to get strings packed in arrays, however it fails if the array is made of integer elements, and that's why it was changed in the to an int range comparison in the _to_yaml_impl function, but that change was missed in the json part. Maybe a cleaner way is to test first for the element type, then test for printable and whitespace characters?

return True
if ch in b'\n\r\t':
return True
Expand Down