-
Notifications
You must be signed in to change notification settings - Fork 295
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
proposed breaking change: change handle argument type in notification callbacks #759
Comments
Having thought about this a bit more, instead of replacing the |
I like your second suggestion, passing the characteristic instead of the handle to the callback function. This also feels more natural from a general point of view, as notifications are about the characteristic anyway. |
+1 on the second option. Would be great to automatically get the Client + Characteristic in a callback by default. At the moment my code has it's own class to do this, but am happy for the breaking change and small refactor. |
Why not let the user provide an arbitrary reference? |
This is a breaking change that changes the first argument of the notification callback from the handle to the BleakGATTCharacteristic object. This has been a commonly reported problem by users (so much so it is in our troubleshooting guide which can now be removed) and #759 has received positive feedback for the breaking change. It is likely that most users don't use the first argument anyway, so in those cases, this won't actually be a breaking change. Fixes: #759
This is a breaking change that changes the first argument of the notification callback from the handle to the BleakGATTCharacteristic object. This has been a commonly reported problem by users (so much so it is in our troubleshooting guide which can now be removed) and #759 has received positive feedback for the breaking change. It is likely that most users don't use the first argument anyway, so in those cases, this won't actually be a breaking change. Fixes: #759
This is a breaking change that changes the first argument of the notification callback from the handle to the BleakGATTCharacteristic object. This has been a commonly reported problem by users (so much so it is in our troubleshooting guide which can now be removed) and #759 has received positive feedback for the breaking change. It is likely that most users don't use the first argument anyway, so in those cases, this won't actually be a breaking change. Fixes: #759
This is a commonly reported problem: #724, #467 (it feels like it has come up more than this, but that is all I could find).
The solutions is to use
functools.partial
to wrap the callback function to include the required extra info.But I'm wondering if it is worth making a breaking change to make it easier and more obvious. For example, we could replace the
handle
argument in the callback function with theBleakClient
object itself. This would only break programs that are actually using thehandle
argument. I suspect that is not many because unless you are attaching the same callback to multiple characteristics, this information is not needed.Originally posted by @dlech in #754 (comment)
The text was updated successfully, but these errors were encountered: