Skip to content

aioble/device.py: No default timeout for async disconnected() method. #497

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

Conversation

rknegjens
Copy link
Contributor

@rknegjens rknegjens commented Apr 6, 2022

The value for the timeout_ms optional argument to DeviceConnection.disconnected() async method is changed from 60000 to None. This way users awaiting a device disconnection using await connection.disconnected() won't be surprised by a 1 minute timeout. For instance, in the temp_sensor.py usage example I think it's reasonable to expect the connection to the client to stay open indefinitely.

@rknegjens rknegjens force-pushed the aioble-device-disconnected-default-timeout branch from e06c032 to 0b7f49e Compare April 6, 2022 20:46
@rknegjens rknegjens changed the title aioble/device.py: No default timeout for async disconnected() method. aioble/device.py: No default timeout for async disconnected() method. Apr 6, 2022
@rknegjens rknegjens force-pushed the aioble-device-disconnected-default-timeout branch from 0b7f49e to 924b5e5 Compare April 6, 2022 20:47
@andrewleech andrewleech requested a review from jimmo April 6, 2022 21:07
@andrewleech
Copy link
Contributor

This does make sense to me, I know a couple of other people this timeout has tripped up too.

I'm not sure if there was a specific intention behind having a timeout here, can you comment @jimmo ?

@rknegjens rknegjens force-pushed the aioble-device-disconnected-default-timeout branch from 924b5e5 to 05e84b5 Compare April 7, 2022 15:42
@peterson79
Copy link

This also tripped me up and caused a lot of debug time... can this get merged?

@dpgeorge
Copy link
Member

I agree, the default makes more sense as None, with no timeout.

In fact, the temp sensor example was just changed to explicitly specify this: d4362d5

The value for the `timeout_ms` optional argument to
`DeviceConnection.disconnected()` async method is changed from 60000 to
None.  This way users awaiting a device disconnection using `await
connection.disconnected()` won't be surprised by a 1 minute timeout.
@dpgeorge dpgeorge force-pushed the aioble-device-disconnected-default-timeout branch from 05e84b5 to db7f9a1 Compare May 24, 2024 09:15
@dpgeorge dpgeorge merged commit db7f9a1 into micropython:master May 24, 2024
2 of 3 checks passed
@dpgeorge
Copy link
Member

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants