bluetooth: macOS: Need to remove intermediate methods in BluetoothRemoteServiceMac |
|||
Issue descriptionThose methods don't make sense: void BluetoothRemoteGattServiceMac::DidUpdateValue(CBCharacteristic*, NSError*) void BluetoothRemoteGattServiceMac::DidWriteValue(CBCharacteristic*, NSError*) void BluetoothRemoteGattServiceMac::DidUpdateNotificationState(CBCharacteristic*, NSError*) They are used by BluetoothRemoteGattDeviceMac to send those events to the characteristics. To simplify the code, the device should directly call characteristics using those methods: void BluetoothRemoteGattCharacteristicMac::DidUpdateValue(NSError*) void BluetoothRemoteGattCharacteristicMac::DidWriteValue(NSError*) void BluetoothRemoteGattCharacteristicMac::DidUpdateNotificationState(NSError*)
,
May 29 2017
This would mean that the device would need to know of all characteristics and descriptors adding more logic and complexity to the device. I think just renaming the methods to CharacteristicDidUpdateValue() would be better.
,
May 29 2017
It looks more simple to me: https://codereview.chromium.org/2904363002/diff/40001/device/bluetooth/bluetooth_low_energy_device_mac.mm The device doesn't need to know anything more than it already knows. It just need to pass the message directly to the right object. The methods in BluetoothRemoteGattServiceMac don't gain anything to me. Those methods to remove are: https://codereview.chromium.org/2904363002/patch/40001/50004 I'm just trying to generalise what I implemented for: BluetoothLowEnergyDeviceMac::DidUpdateValueForDescriptor() BluetoothLowEnergyDeviceMac::DidWriteValueForDescriptor() I didn't implement those intermediate methods in BluetoothRemoteGattServiceMac and BluetoothRemoteGattCharacteristicMac to send the event from the device to the service, and from the service to the characteristic and then from the characteristic to the descriptor. Let me know what you think.
,
May 31 2017
I think it's a bit weird to have the device do all the routing but I agree that it is cleaner.
,
Jun 1 2017
Do you want me to go forward with my patch? Or should I close this bug as "won't fix" and make sure all events are implemented with intermediates methods?
,
Jun 1 2017
If you think it simplifies the code, go for it. +scheib in case he has opinions.
,
Jun 3 2017
This seems good to me.
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be89d3153d2c173742d473cc3e5b04e12770c507 commit be89d3153d2c173742d473cc3e5b04e12770c507 Author: jlebel <jlebel@chromium.org> Date: Wed Jun 14 10:57:16 2017 bluetooth: macOS: Removing intermediate methods in BluetoothRemoteServiceMac Making BluetoothRemoteGattDeviceMac call directly characteristic methods instead going through intermediate methods from BluetoothRemoteGattServiceMac. Removing: void BluetoothRemoteGattServiceMac::DidUpdateValue(CBCharacteristic*, NSError*) void BluetoothRemoteGattServiceMac::DidWriteValue(CBCharacteristic*, NSError*) void BluetoothRemoteGattServiceMac::DidUpdateNotificationState(CBCharacteristic*, NSError*) BUG= 727112 Review-Url: https://codereview.chromium.org/2904363002 Cr-Commit-Position: refs/heads/master@{#479346} [modify] https://crrev.com/be89d3153d2c173742d473cc3e5b04e12770c507/device/bluetooth/bluetooth_low_energy_device_mac.h [modify] https://crrev.com/be89d3153d2c173742d473cc3e5b04e12770c507/device/bluetooth/bluetooth_low_energy_device_mac.mm [modify] https://crrev.com/be89d3153d2c173742d473cc3e5b04e12770c507/device/bluetooth/bluetooth_remote_gatt_service_mac.h [modify] https://crrev.com/be89d3153d2c173742d473cc3e5b04e12770c507/device/bluetooth/bluetooth_remote_gatt_service_mac.mm
,
Jun 14 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by jlebel@chromium.org
, May 28 2017