Skip to content
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

Feature request: BleakClient.write_gatt_char response default should be dynamic #909

Closed
jackjansen opened this issue Jul 29, 2022 · 1 comment · Fixed by #1236
Closed
Labels
enhancement New feature or request

Comments

@jackjansen
Copy link
Contributor

The default value for the BleakClient.write_gatt_char argument response is False.

This is a nuisance for BLE devices that support write, but not write-without-response. And we could know this: the properties on the characteristics have this information.

Suggestion: we make response an Optional[bool] defaulting to None. And in the default case we examine the properties, use write-without-reponse if it is supported, otherwise we use write.

If this is a good idea I'll create a pull request.

@dlech dlech added the enhancement New feature or request label Jul 29, 2022
@dlech
Copy link
Collaborator

dlech commented Jul 29, 2022

I was thinking about this recently too. I was going to suggest to default to write with response in the case where both are supported since that is the more reliable write, but I suppose that could be a breaking change in some cases.

dlech added a commit that referenced this issue Feb 19, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
dlech added a commit that referenced this issue Feb 19, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
dlech added a commit that referenced this issue Feb 19, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
dlech added a commit that referenced this issue Feb 19, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
dlech added a commit that referenced this issue Jul 20, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
dlech added a commit that referenced this issue Jul 20, 2023
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: #909
@dlech dlech mentioned this issue Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants