fix is_nice_character #59
Conversation
is_nice_character is a nested function defined in two places with different implementations. Fixed the one defined in _to_json_compatible_object to match the one in _to_yaml_impl
| @@ -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: | |||
There was a problem hiding this comment.
why not the other way? I would prefer is_printable to the chatacter range
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
The nested function is_nice_character https://github.com/dronecan/pydronecan/blob/master/dronecan/introspect.py#L42
is defined in two places with different implementations. Fixed the one defined in _to_json_compatible_object to match the one in _to_yaml_impl