The single subjob build failure is bogus: it's a job failure fetching dependencies.

I think packet_size needs to be asymmetric because WRITES can be as long as the attribute length while NOTIFY is limited by the MTU. Based on the doc here it needs to be the larger of the two: https://github.com/adafruit/circuitpython/blob/master/shared-bindings/_bleio/PacketBuffer.c#L100

We'll want to validate that outgoing data fits into the mtu if NOTIFY is being used.

@tannewt I am adding Connection.max_packet_length, which returns the maximum number of bytes that will fit in a single transmitted packet (MTU size - 3). It will throw an exception if you call it before the connection is established, since we don't know the MTU size.

Packet_buffer.packet_size could call this, but it will need to check if its characteristic is NOTIFY or not. And now we'll need to defer creating the local bytearray buffers in the various libraries (MIDI, etc.), until we have a connection. We can just set them to None in __init__ and create them on demand.

The logic for computing .packet_size is simple. Assuming we have Connection.max_packet_length as describe above, it becomes a convenience method only. It could be dropped and the user code could decide whether to use Characteristic.max_length or Connection.max_packet_length, based on the user code knowing about NOTIFY or not.

In either case we will need to update the libraries that use PacketBuffer.packet_size.

@tannewt I am adding Connection.max_packet_length, which returns the maximum number of bytes that will fit in a single transmitted packet (MTU size - 3). It will throw an exception if you call it before the connection is established, since we don't know the MTU size.

I don't think you will be able to raise an exception because a Connection object doesn't exist prior to it being established.

How will you know the maximum amount of bytes you can read out? If you are a server with a write and notify characteristic, then a client could write more than the MTU to you even though you can only write an MTU back.

I don't think you will be able to raise an exception because a Connection object doesn't exist prior to it being established.

Right, sorry, I was calling bleio_connection_ensure_connected(), which will throw an exception if the connection is no longer connected. So PacketBuffer.max_packet_length will throw an exception if it's not connected, because it doesn't know the MTU and so can't answer reliably.

How will you know the maximum amount of bytes you can read out? If you are a server with a write and notify characteristic, then a client could write more than the MTU to you even though you can only write an MTU back.

That's a bit pathological (WRITE/NOTIFY vs the typical READ/NOTIFY), but I"m thinking the point of .max_packet_length is how big a notify packet the server can send, and the point of PacketBuffer is to queue notify packets. Maybe we should just call it max_notify_packet_length or similar, to be more precise.

How will you know the maximum amount of bytes you can read out? If you are a server with a write and notify characteristic, then a client could write more than the MTU to you even though you can only write an MTU back.

That's a bit pathological (WRITE/NOTIFY vs the typical READ/NOTIFY), but I"m thinking the point of .max_packet_length is how big a notify packet the server can send, and the point of PacketBuffer is to queue notify packets. Maybe we should just call it max_notify_packet_length or similar, to be more precise.

It's not pathological. That's how USB MIDI works.

It's not pathological. That's how USB MIDI works.

All right, I stand corrected. So I'm thinking that what one wants .packet_size to reflect varies based on which direction you're interested in and what the flags are. Perhaps the best thing is to drop it, and have the user code figure out the packet length required based on the situation, the new Connection.max_packet_length, and on the Characteristic's max_length. Or do you think calling it .max_notify_length makes more sense?

How about renaming it incoming_packet_size but have it manage the notify vs write difference?

@tannewt OK, I think this is finally ready for review again. I decided not to rename .packet_size, but it would have create churn in the libraries, and I decided the alternate names were not really clearer. The doc is clearer.

This has been tested with BLE MIDI both out and in, with ble_uart_echo_test.py, and with some other libraries that use PacketBuffer.

Also, taking a cue from @jepler (thanks!), I changed a bunch of makefile ifndefs to use ?=, which makes them a lot shorter.

Whew, thanks! A lot of technical debt paid off, at the cost of too much time.