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

Pull out BLE protocol serializing into a series of helpers #84

Open
Jc2k opened this issue Jan 13, 2019 · 0 comments
Open

Pull out BLE protocol serializing into a series of helpers #84

Jc2k opened this issue Jan 13, 2019 · 0 comments
Labels
BLE Transport Issues related to BLE based Accessory

Comments

@Jc2k
Copy link
Collaborator

Jc2k commented Jan 13, 2019

Was implementing unpaired identify and wrote this for what felt like the 17th time:

body = len(value).to_bytes(length=2, byteorder='little') + value

So I think we need to define some serializer/deserializer helpers. I plan to do this after my backlog of PR's is cleared and test coverage is higher, as there is already a lot to review, so this ticket is just to collect ideas and remind me later.

Imagining something like:

def write_serialize(op, body=None):
    tid = random.rangrange(0, 255)
    request = bytearray([0x00, op, tid])
    if body:
        request.extend(body)
    return tid, request

def characteristic_write_request_serialize(iid, value):
    request = bytearray(iid.to_bytes(length=2, byteorder='little')

    request_tlv = TLV.encode_list([
        (1, value),
    ])
    request.extend(len(request_tlv).to_bytes(length=2, byteorder='little'))
    request.extend(request_tlv)
    
    return write_serialize(HapBleOpCodes.CHAR_WRITE, bytes(request))

These are quite 'pure' - no side effects - so very testable.

There will probably be a deserialize for each serialize to aid testing.

There will also be request and response variants for each protocol. For example:

def characteristic_write_response_deserialize(value):
    if not value:
        raise RequestRejected(0, 'No response was received')
    value = bytearray(value)
    if value[0] != 0x02:
        raise ProtocolError('Unexpected cf flags: %s' % value[0])
    tid = value[1]
    status = value[2]
    if status != HapBleStatusCodes.SUCCESS:
        raise RequestRejected(status, HapBleStatusCodes[status])
    payload = value[3:]

    # Stuff above this line would be factored out as shared with other response types

    # get body length
    length = int.from_bytes(data[:2], byteorder='little')
    # FIXME: Check length matches len of payload?

    # parse tlvs and analyse information
    tlv = TLV.decode_bytes(data[2:])
    return tid, dict(tlv)

    # FIXME: The actual wrapper probably wouldn't return the dict directly. E.g. for a read_response_deserialize it would probably return just the actual unpacked value directly.

I don't include the tid check inside that method - if the tid is wrong then its most likely a sign that we need to deal with out of order transactions, and that shouldn't be something this layer has to even know about.

My goal would be that the ble_implementation.py and controller.py modules don't have any .to_bytes/.from_bytes or TLV stuff in them and the new module would have 100% coverage.

@jlusiardi jlusiardi added the BLE Transport Issues related to BLE based Accessory label Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLE Transport Issues related to BLE based Accessory
Projects
None yet
Development

No branches or pull requests

2 participants