DevEncoded/bytearray commands flawed in Python 3

[mguijarr]

(using PyTango 9.3.0, Python 3.7.3)

In a test Tango server, I have the following command:

@command(dtype_in=bytearray)
def DevSerWriteChar(self, chars):
    self.buf += chars

When calling it from a PyTango client like this:

>>> dev_proxy.DevSerWriteChar(bytearray(b"HELLO"))

I get the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/device_proxy.py", line 243, in f
    return dp.command_inout(name, *args, **kwds)
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/green.py", line 195, in greener
    return executor.run(fn, args, kwargs, wait=wait, timeout=timeout)
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/green.py", line 109, in run
    return fn(*args, **kwargs)
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/connection.py", line 108, in __Connection__command_inout
    r = Connection.command_inout_raw(self, name, *args, **kwds)
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/connection.py", line 136, in __Connection__command_inout_raw
    param = __get_command_inout_param(self, cmd_name, cmd_param)
  File "/home/matias/miniconda2/envs/bliss/lib/python3.7/site-packages/tango/connection.py", line 57, in __get_command_inout_param
    param.insert(info.in_type, cmd_param)
TypeError: No registered converter was able to extract a C++ pointer to type char from this Python object of type int

As far as I have seen, bytearray is converted to DevEncoded - but it seems something is missing to handle it properly ? Or maybe I am doing something wrong ?

[ajoubertza]

I tried this out and get the same error as you, @mguijarr. I see a few problems:

• When using DevEncoded, we need a tuple with two elements. First is the custom "encoding type" string, and then the actual data of interest. In your example, you are only using a single parameter instead of tuple.
• Even with a tuple, the bytearray still doesn't work (same "No registered converter" error). However, using bytes does work. Maybe because bytes are immutable, while bytearray is mutable?
• In the server, we shouldn't just append to self.buf, as the incoming parameter is tuple. The data is in the second element.

Server:

@command(dtype_in=bytearray)
def DevSerWriteChar(self, chars):
    print("User's encoding type: {}".format(chars[0]))
    print("User's data: {}".format(chars[1]))
    self.buf += chars[1]
    print("Buffer: {}".format(self.buf))

Client:

>>> dev_proxy.DevSerWriteChar(("my-type", bytes(b"HELLO")))

Output from server:

...
User's encoding type: my-type
User's data: HELLO
Buffer: ['H', 'E', 'L', 'L', 'O']
...

[mguijarr]

@ajoubertza thanks for having tried my code and thanks for your help.

Still, I do not understand if I am doing something wrong or if there is a bug in PyTango ?

I am asking because I can change the code to make the proper calls with tuples and with bytes however the documentation does not invite to do so...

[ajoubertza]

@mguijarr I think you are doing something wrong - DevEncoded types don't work the way you are using them. It also seem that the documentation is not clear for about using bytearray for commands and attributes.

Please propose an update to the documentation.

[mguijarr]

@ajoubertza Now I find it even more weird: did you check type(chars[1]) ? It is... <class 'str'> !

So, even using DevEncoded the proper way, it gets transformed to a string somewhere.

Edit: this is true for the server. On the client, second item of the tuple is of type 'bytes' as expected.

This is driving me crazy 🤒

[ajoubertza]

@mguijarr Yes, I see the type is str, which is strange. bytes would be better.

In issue #216, a similar issue was raised, although that was with attributes. After that fix, I can see that when writing to an attribute, the type on the server side is bytes. That is inconsistent with the command. The command also doesn't work with arbitrary bytes.

Note: under Python 2.7, the code below does not raise any errors, so looks like a Python 3 issue.

Adding an attribute:

from tango import AttrWriteType, DevEncoded
from tango.server import Device, attribute, command

class TestDevice(Device):
    value = ('uint8', b'')

    @attribute(
        dtype=bytearray,
        access=AttrWriteType.READ_WRITE)
    def encAttr(self):
        return self.value

    @encAttr.write
    def encAttr(self, value):
        print("type 0 {}, type 1 {}".format(type(value[0]), type(value[1])))
        print("chars[0]: {}".format(value[0]))
        print("chars[1]: {}".format(value[1]))
        self.value = value

    @command(dtype_in=bytearray)
    def DevSerWriteChar(self, chars):
        print("type 0 {}, type 1 {}".format(type(chars[0]), type(chars[1])))
        print("chars[0]: {}".format(chars[0]))
        print("chars[1]: {}".format(chars[1]))

Client:

>>> dev_proxy.encAttr = ("my-attr", b"hello-\x80\xff")
>>> dev_proxy.encAttr
('my-attr', b'hello-\x80\xff')

>>> dev_proxy.DevSerWriteChar(('my-cmd', b'hello'))
>>> dev_proxy.DevSerWriteChar(('my-cmd', b'hello-\x80\xff'))
...
DevFailed: DevFailed[
DevError[
    desc = UnicodeDecodeError: 'utf-8' codec can't decode byte 0x80 in position 6: invalid start byte
           
  origin = 
  reason = PyDs_PythonError
severity = ERR]

DevError[
    desc = Failed to execute command_inout on device test/nodb/testdevice, command DevSerWriteChar
  origin = Connection::command_inout()
  reason = API_CommandFailed
severity = ERR]
]

Output from server:

...
type 0 <class 'str'>, type 1 <class 'bytes'>
chars[0]: my-attr
chars[1]: b'hello-\x80\xff'

type 0 <class 'str'>, type 1 <class 'str'>
chars[0]: my-cmd
chars[1]: hello

I'll investigate further.

[mguijarr]

Thanks @ajoubertza - tell me if you need some help from my side. I cannot devote much time to this, though ; as a background task I try to improve integration tests for BLISS our new beamline experiments control system at ESRF, through the use of simulation devices or communication channels (like serial lines).

[ajoubertza]

@sdebionne Could you maybe help with this, please? Similar to the issue you fixed with the attributes (PR #226). I don't know much about the C++ wrapper. See the PR linked above - I've just added some tests that show the problem. You could continue in that branch.

[sdebionne]

@ajoubertza Sure, I'll try to find some time to look into this. Thanks for the tests, it will make it easier to reproduce and fix.

[sdebionne]

A quick update on this! @tiagocoutinho I have identified the issue:

in command.cpp:225 we have:

    bopy::str encoded_format(data[0].encoded_format);
    bopy::str encoded_data((const char*)data[0].encoded_data.get_buffer(),
                           data[0].encoded_data.length());
    
    o = boost::python::make_tuple(encoded_format, encoded_data);

I believe the conversion of encoded_data to string is incorrect here (in Python 3).

[sdebionne]

And the fix is:

boost::python::object encoded_data(
	boost::python::handle<>(
		PyMemoryView_FromMemory(
			(char*)data[0].encoded_data.get_buffer(),
			data[0].encoded_data.length(),
			PyBUF_READ
		)
	)
);

[andygotz]

Thanks a lot!

[ajoubertza]

Thanks @sdebionne. Looking at that fix, I notice a similar function in to_py.h.

pytango/ext/to_py.h

Lines 20 to 31 in 19e4c5d

	struct DevEncoded_to_tuple
	{
	static inline PyObject* convert(Tango::DevEncoded const& a)
	{
	boost::python::str encoded_format(a.encoded_format);
	bopy::object encoded_data = bopy::object(
	bopy::handle<>(PyBytes_FromStringAndSize(
	(const char*)a.encoded_data.get_buffer(),
	(Py_ssize_t)a.encoded_data.length())));
	boost::python::object result = boost::python::make_tuple(encoded_format, encoded_data);
	return boost::python::incref(result.ptr());
	}

I wonder if we should be using PyBytes_FromStringAndSize or PyMemoryView_FromMemory? This fix would be the first usage of PyMemoryView_FromMemory.

(And I wonder why we have similar code in two places)

[sdebionne]

I saw the similar code in to_py.h but since it already uses PyBytes_FromStringAndSize I thought it was OK.

I wonder if we should be using PyBytes_FromStringAndSize or PyMemoryView_FromMemory?

PyBytes_FromStringAndSize works as well and the use of PyMemoryView_FromMemory is not really motivated so let's stay with PyBytes_FromStringAndSize.

Now I am confused because the fix works on my local machine with both PyBytes_FromStringAndSize and PyMemoryView_FromMemory but fails on Travis:

With PyBytes_FromStringAndSize, commit and CI.

With PyMemoryView_FromMemory, commit and CI.

I am building on Windows which is not covered by the CI yet, I need to investigate further...

[sdebionne]

I reverted tango to 9.2.5a from 9.3.2 on Linux and it works. On Windows I am using 9.3.3. does anyone have information about a bug fixed in 9.3.3 regarding DevEncoded -that would not be mentionned in the Changelog?

[ajoubertza]

Thanks for the change.
I see the all the Python 3 builds failed with:
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd4 in position 0: invalid continuation byte

That is strange. We shouldn't be trying to decode the bytestring to unicode - it is just some random bytes, not text.

[ajoubertza]

The same UnicodeDecodeError was also raised in the first test run, before any of your changes, so doesn't seem like the changes are in effect.

[ajoubertza]

Build issue on Travis has been resolved - see notes in the related PR.

[ajoubertza]

Fixed in #281.