update the EnableNotifications Method, add the parameter to specifing… #293
Conversation
… the notify or indicate mode of characters, just for windows os
|
sorry |
|
I has fixed the error of new argument |
| @@ -360,14 +361,29 @@ func (c DeviceCharacteristic) Read(data []byte) (int, error) { | |||
| return len(readBuffer), nil | |||
| } | |||
|
|
|||
| // EnableNotifications enables notifications in the Client Characteristic | |||
| // Configuration Descriptor (CCCD). Just the Notify mode is supported. | |||
| func (c DeviceCharacteristic) EnableNotifications(callback func(buf []byte)) error { | |||
There was a problem hiding this comment.
EnableNotifications enables notifications or indications, whatever is available. And it favors notifications over indications. This PR should not change that behaviour.
There was a problem hiding this comment.
Sorry， i will update to keep the original behavior
gattc_windows.go
Outdated
| if (c.properties&genericattributeprofile.GattCharacteristicPropertiesNotify == 0) && | ||
| (c.properties&genericattributeprofile.GattCharacteristicPropertiesIndicate == 0) { | ||
| return errNoNotify | ||
| func (c DeviceCharacteristic) EnableNotificationsWithMode(usingIndicate bool, callback func(buf []byte)) error { |
There was a problem hiding this comment.
I'm not a big fan of using boolean flags for this. It may make sense for methods that enable or disable something. But I think that in this case it is confusing as the user needs to know what true or false means.
An enum may be a better option. Maybe something like this?
type NotificationMode = genericattributeprofile.GattCharacteristicProperties
const (
NotificationModeNotify NotificationMode = genericattributeprofile.GattCharacteristicPropertiesNotify
NotificationModeIndicate NotificationMode = genericattributeprofile.GattCharacteristicPropertiesIndicate
)This also allows simplifying the branching inside the method:
func (c DeviceCharacteristic) EnableNotificationsWithMode(mode NotifyMode, callback func(buf []byte)) error {
if mode != NotificationModeIndicate && mode != NotificationModeNotify {
return errInvalidNotificationMode
}
if c.properties&mode == 0 {
return errNoNotify
}
...
}There was a problem hiding this comment.
Well , using enum makes code easier to understand.
Thanks for your advice.
I had changed my code and commit again.
If you have any other ideas, please let me know. Thank you
update the EnableNotifications Method,
add the parameter to specifing the notify or indicate mode of characters,
just for windows os