Add constructor boolean flag in Characteristic class to return raw db…

[garretthagen21]

…us value instead of always implicitly converting it before calling write callback.

The implicit conversion using dbus_tools.dbus_to_python() can result in unwanted effects for those looking to use raw byte arrays in their code. For example, I am using the python struct library to parse raw byte arrays that are transmitted over a BLE connection. Often times this implicit conversion to python will convert the raw dbus value to the wrong data type and then struct is unable to parse. My simple addition allows users, to disable this conversion which is still enabled by default.

[ukBaz]

To be clear, what we are talking about is that currently dbus_to_python converts a dbus.Array with signature y to a python list and not a bytearray so struct.unpack doesn't work? e.g.

from bluezero import dbus_tools
import dbus
import struct
x = dbus.Array([1,2,3,4,5,6,7,8], signature='y')
#  x = dbus.Array([1, 2, 3, 4, 5, 6, 7, 8], signature=dbus.Signature('y'))
y = dbus_tools.dbus_to_python(x)
#  y = [1, 2, 3, 4, 5, 6, 7, 8]
z = struct.unpack('<II', y)
Traceback (most recent call last):
  File "/usr/lib/python3.8/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<input>", line 1, in <module>
TypeError: a bytes-like object is required, not 'list'

I have some reluctance to go with your proposed solution because a documented design goal of Bluezero is that it should try to avoid presenting people with dbus information.

It would be more accurate to say that there is a bug in dbus_to_python and it is not doing the conversion correctly. There is probably some debate to be had if it should return bytes or bytearray. For reference:

python3's bytes and bytearray classes both hold arrays of bytes, where each byte can take on a value between 0 and 255. The primary difference is that a bytes object is immutable, meaning that oncce created, you cannot modify its elements. By contrast, a bytearray object allows you to modify its elements.

Initially I'm favouring bytearray but I'm interested in other opinions.

This would result in the code going from:

    elif isinstance(data, dbus.Array):
        data = [dbus_to_python(value) for value in data]

to

    elif isinstance(data, dbus.Array):
        if data.signature == dbus.Signature('y'):
            data = bytearray(data)
        else:
            data = [dbus_to_python(value) for value in data]

Rerunning the above example after making that change I get:

from bluezero import dbus_tools
import dbus
import struct
x = dbus.Array([1,2,3,4,5,6,7,8], signature='y')
#  x= dbus.Array([1, 2, 3, 4, 5, 6, 7, 8], signature=dbus.Signature('y'))
y = dbus_tools.dbus_to_python(x)
#  y = bytearray(b'\x01\x02\x03\x04\x05\x06\x07\x08')
z = struct.unpack('<II', y)
#  z = (67305985, 134678021)

Would that solution be workable for you?

[garretthagen21]

Yes, a bytearray would work for my usage. I understand the motivation to try and keep the user from having to deal with using the dbus array directly as it is messy, but I figured that there might be scenarios where other users want to perform their own customized parsing of the dbus array where in my case I would convert to a bytearray/bytes object and then continue my parsing in a custom message class where the byte array contains a mix of data types that are packed together. So it seemed like the best option to me to allow a scenario where the user could retreive the raw dbus array and then perform whatever conversions they see fit. Perhaps I am doing something incorrectly, but to give you an example, I am using this custom message class to parse the contents of the write callback data in deserialize_request(). The problem is that depending on the contents of the data, the dbus_to_python function would perform different conversions each time. (It would not let me attach .py file so you can view the contents in the .txt version that is attached) message_connection_info.txt

[ukBaz]

I'm not saying that what you did isn't better or, at least, would cover more scenarios. The main challenge with this library is keeping it to a manageable size as testing is so difficult/time consuming. The goal of the library is to help people get started with Bluetooth, once they know what they are doing it was expected that they migrate to using the BlueZ D-Bus API directly if they needed more.

However, I am interested in your statement:

The problem is that depending on the contents of the data, the dbus_to_python function would perform different conversions each time.

I would like to investigate that more. Do you have a testcase that I can run? I tried running your example but it has broken dependencies.

These are the tests that exist today so if there is something better I could do that would be great

https://github.com/ukBaz/python-bluezero/blob/392a115ace80a6d7257b76708ec24b99ec9e2e5d/tests/test_dbus_tools_mock.py#L40-L76

[ukBaz]

I am tidying up this repository by sorting out PR's that have stalled.

From the discussion above I think updating the dbus_to_python function is the best way forward and have created an new issue #372 to track this.