Open linnik opened 2 years ago
All the timeout
values that you change here are not critical timeouts, are they? They are merely waiting for the event in question to be set to continue the program? For instance, it will wait for the device to disconnect, which wil not be improved by waiting longer...
The one I can see could be important to change is the get_services
-timeout in the BlueZ backend and maybe the read_characteristic
in the macOS, but if you haven't gotten a response in 5 seconds I don't think it is useful to wait any longer... The disconnect
does not need to be increased in any circumstance I think.
@hbldh As I read this PR code changes, it just tries to remove hard-coded timeouts with something that can be specified. If not provided it defaults to 10.0 seconds; perhaps you would rather see that default timeout be set to 5.0 seconds?
Then as I read the PR it would allow someone to specify the timeout if so desired, to a longer or a shorter timeframe.
it just tries to remove hard-coded timeouts with something that can be specified
I think the question is: is there an actual technical problem that is fixed with a different timeout or are we just doing this for the sake of making everything configurable?
Though I'm not the OP, for me this looks like something that could help me in testing. The use case being that I know something about the Device Under Test (DUT) such as a unique name or the MAC address from reading a NFC tag or scanning a QR code; after which I would like to connect to the DUT via BLE within a given amount of time, then discover its services within another given amount of time. The purpose being to figure out if the DUT is able to meet certain minimal criteria (which of course are parameters in a test script to be executed against the DUT when the test starts).
Thus for me the "make everything configurable" does seem interesting enough indeed; the alternative being of course that I would schedule time-outs differently (another thread running a clock and scheduling the time-out for each part of the test); and then if the DUT doesn't meet the test, I will "fail" it and terminate the DUT in some way... which may cause some side effects, so more graceful controlled time-outs sound interesting.
But my use case might be very specific and not fit the purpose of bleak, so it is just an input for your consideration of course.
This PR removes hardcoded timeout values in all backends in favor of default
BaseBleakClient._timeout
or kwargstimeout=...
argument.Otherwise it would be impossible to connect with slow devices even if timeout option is given: