breaking: remove address from SMPClient; SMPTransport owns address

[JPHutchins]

I've long thought that passing the SMP server "address" to the SMPClient class instead of the SMPTransport implementation classes was a mistake. "address" means different things to the different transports. Notably, this change allows the UDP transport to clearly take an address and a port argument, rather than just an address.

Unfortunately this is a breaking change. While all existing address args should still be valid, they must be moved from "1st arg of SMPClient() to 1st arg of SMPTransport".

@M1cha this is what I have in mind as the breaking change required to continue implementing the unique requirements of each transport for smpmgr.

[M1cha]

This PR didn't change the behavior, but it got me wondering: What is the reasoning behind having a single parameter for both name and address? While this is probably fine for 99% of use-cases, you can easily make this connect to the wrong device by giving a device a name, that looks the same as the BLE address of another device 🙈 😅

[JPHutchins]

Yeah, I don't love it, and thought about creating a union type that's name or address, but honestly, if someone uses a MAC address as the device name, I WANT them to open a ticket - force them out into the wild! 🤣

[M1cha]

I didn't test this, yet but it looks good, thx.

[GHF]

I've long thought that passing the SMP server "address" to the SMPClient class instead of the SMPTransport implementation classes was a mistake.

I really like the direction this is going and I want to suggest that this could go even further. In this case, it's SMPClient dictating to SMPTransport what its connection parameters should be, when the transport-specific class would know better. I think the lifetime of a transport's connection session should be decoupled from SMPClient's session too.

We've been integrating smpclient into an application that has a standing BLE link and the semantics of SMPClient (the class and async context) owning the lifetime of the transport's session has been awkward. It wants to make a BLE connection, but the establishment procedure isn't idempotent (scanning fails if the peripheral stops advertising with an existing link), and it disconnects the link through bleak when the SMPClient context closes.

Instead, if the semantics were: an SMPTransport instance exists only after a session is established and SMPClient can assume that a transport is available to send and receive as long as it's open, then it becomes more flexible to reuse a BLE link.

Ideally it'd probably look something like:

async with SMPBLETransport(address) as transport:
	async with SMPClient(transport) as client:
        ...

[JPHutchins]

I've long thought that passing the SMP server "address" to the SMPClient class instead of the SMPTransport implementation classes was a mistake.

I really like the direction this is going and I want to suggest that this could go even further. In this case, it's SMPClient dictating to SMPTransport what its connection parameters should be, when the transport-specific class would know better. I think the lifetime of a transport's connection session should be decoupled from SMPClient's session too.

We've been integrating smpclient into an application that has a standing BLE link and the semantics of SMPClient (the class and async context) owning the lifetime of the transport's session has been awkward. It wants to make a BLE connection, but the establishment procedure isn't idempotent (scanning fails if the peripheral stops advertising with an existing link), and it disconnects the link through bleak when the SMPClient context closes.

Instead, if the semantics were: an SMPTransport instance exists only after a session is established and SMPClient can assume that a transport is available to send and receive as long as it's open, then it becomes more flexible to reuse a BLE link.

Ideally it'd probably look something like:

async with SMPBLETransport(address) as transport:
	async with SMPClient(transport) as client:
        ...

I think that I'm understanding that with this design the connected transport is an invariant of the SMPClient instance. This is nice, because it does away with SMPClient having a connect method and skirts around the general issue of python not having an __ainit__() constructor. It allows for client errors to occur without ending the transport lifetime, but semantically shows that a transport lifetime must live at least as long as a client.

Am I understanding correctly?