aiohomekit icon indicating copy to clipboard operation
aiohomekit copied to clipboard

Share PDU code between CoAP and BLE?

Open Jc2k opened this issue 3 years ago • 4 comments
trafficstars

I optimistically put the BLE PDU code in aiohomekit/pdu.py. But how generic is it?

We can move the CoAP versions for OpCode and PDUStatus to aiohomekit.pdu for sure.

In terms of merging encode_pdu:

  • How does CoAP handle IP fragmentation? The spec says to assume a max payload size of 1024. An ipv6 packet has a minimum MTU of 1280. So i'm guessing we won't need do do fragmentation for CoAP? So i don't really want to inflict my version on you if thats the case.

In terms of merging decode_pdu:

  • It looks like your CoAP accessory always returns a body length, even if there is no body. That's not the case for BLE. 3 byte response PDU's are pretty common. It will be interesting to see if that is a Nanoleaf behaviour or not.
  • The request/response bit should be set of BLE PDU, so I could use your version there.
  • I can see why you'd return a ValueError, but the return type gets a bit messy. I think for mine i'll probably add a "proper" exception rather than just a ValueError. Would that work for you?

CC @roysjosh

Jc2k avatar Feb 06 '22 09:02 Jc2k

I've never seen a fragmented payload. The largest response is the accessory database and it came in at 1108 bytes. We might have to wait for a device with multiple accessories or more services and characteristics to see what happens when it crosses ~1500 bytes. The Thread network fragments to ~100 bytes and the Border Router reassembles so maybe we would just get a big payload split across a few UDP packets...

I don't mind coding defensively on the decode, starting with 3 bytes & only decoding the length if present. Yeah, I'm not super happy with it, I was trying to find a way to decode multiple good PDUs that have an "invalid request" PDU in the middle & communicate that error condition back up among the good data. I thought I saw that behavior at some point though I can't find an example in my current dev log. Both the Home app and Nanoleaf app send multiple requests in a single payload which might be a difference to BLE?

roysjosh avatar Feb 06 '22 15:02 roysjosh

Yeah, you can't batch requests payloads with BLE. You have to know the right characteristic GATT endpoint to write to AND know the iid as well, so each PDU goes to a different GATT endpoint.

I guess if you have multiple PDU's in a single UDP packet the body length is actually mandatory? Otherwise how do you know if you've found a body or the next PDU payload. And it wouldn't make sense to have different behaviour for the last PDU in a payload.

Jc2k avatar Feb 06 '22 15:02 Jc2k

I could just stick with bytes | PDUStatus and synthesize a PDUStatus for the tid mismatch and bad control conditions. That's slightly less gross.

roysjosh avatar Feb 06 '22 16:02 roysjosh

I was wondering the same thing, maybe make the value outside the range of a u8 so it doesn't clash in future?

Jc2k avatar Feb 06 '22 16:02 Jc2k