PhilipsHue / flutter_reactive_ble

Flutter library that handles BLE operations for multiple devices.
https://developers.meethue.com/
Other
662 stars 326 forks source link

Rename QualifiedCharacteristic -> CharacteristicInstance #788

Closed stargazing-dino closed 1 year ago

stargazing-dino commented 1 year ago

Describe the bug Hi ! In #776 QualifiedCharacteristic was semi renamed to CharacteristicInstance I think.

In flutter_reactive_ble.dart we also added this line:

export 'package:reactive_ble_platform_interface/reactive_ble_platform_interface.dart' hide CharacteristicInstance;

Which kinda tells me that CharacteristicInstance is not supposed to be a public facing type. That's fine but it's still exposed in some places including bluetooth.characteristicValueStream. I previously had extension methods on the QualifiedCharacteristic and can no longer replicate what I had because the instance type is hidden.

Mostly I'm confused about the rename. Is it a functional thing or was CharacteristicInstance meant to completely replace QualifiedCharateristic? It feels weird internally juggling two because I still see references to QualifiedCharateristic in the examples, tests and code.

CC @spkersten

spkersten commented 1 year ago

CharacteristicInstance is not supposed to be used if you're not doing anything within the plug-in itself. It has to be public (unless I overlooked something) because it is used to communicate between the several packages that make up the plugin.

QualifiedCharacteristic can still be used.

If you have an idea for how to make this better, I'd like to hear it 🙂

spkersten commented 1 year ago

I'm typing this on my phone, so I can check it at the moment, but I think bluetooth.characteristicValueStream isn't supposed to be public.

stargazing-dino commented 1 year ago

I'm totally fine with the rename, I think I'm just confused about the different types now and which ones should be user facing or public and which should be internal only.

I'm fairly sure characteristicValueStream has always been public and I personally use the stream to write values from my BLE devices to the cloud.

  /// A stream providing value updates for all the connected BLE devices.
  ///
  /// The updates include read responses as well as notifications.
  Stream<CharacteristicValue> get characteristicValueStream async* {
    await initialize();
    yield* _connectedDeviceOperator.characteristicValueStream;
  }

// characteristic_value.dart
// Value update for specific [CharacteristicInstance].
class CharacteristicValue {
  final CharacteristicInstance characteristic;
  final Result<List<int>, GenericFailure<CharacteristicValueUpdateError>?>
      result;

  const CharacteristicValue(
      {required this.characteristic, required this.result});

  @override
  String toString() =>
      "$runtimeType(characteristic: $characteristic, value: $result)";
}

Anyways, very happy with the new changes and I actually realized i probably shouldn't be creating extension traits like that on QualifiedCharacterstic regardless :p

Taym95 commented 1 year ago

since this is not issue anymore I will close it.