refactor: migrate to federated architecture

[tnc1997]

#969 (comment)

[chipweinberger]

  Stream<OnServicesResetEvent> get servicesChanges {
  Stream<OnDiscoveredServicesEvent> get servicesDiscovers {

yes I really dont like these in particular. onServicesReset, onDiscoveredServices is much better imo.

[chipweinberger]

  Future<List<int>> readCharacteristic(
    DeviceIdentifier remoteId,
    Guid serviceUuid,
    Guid characteristicUuid,
  ) {
    throw UnimplementedError();
  }

must support optional secondaryServiceUuid as well. same with the others.

[tnc1997]

I'd call them onCharacteristicReceived, onCharacteristicWritten. etc. And "on" help signals that it is a stream.

Yes I really don't like these in particular. onServicesReset, onDiscoveredServices is much better imo.

No problem, I can rename them accordingly, I was attempting to name them generically not Android specific 😄

[tnc1997]

must support optional secondaryServiceUuid as well.

Of course, I can add secondaryServiceUuid, but I noticed that it is null in most usages, is this intentional?

flutter_blue_plus/lib/src/bluetooth_characteristic.dart

Line 115 in dea3493

secondaryServiceUuid: null,

flutter_blue_plus/lib/src/bluetooth_characteristic.dart

Line 186 in dea3493

secondaryServiceUuid: null,

flutter_blue_plus/lib/src/bluetooth_characteristic.dart

Line 249 in dea3493

secondaryServiceUuid: null,

[chipweinberger]

theres an outstanding github issue to fix secondary services: #948

[tnc1997]

Thank you very much for your feedback, I have applied your suggestions from the code review.

If you are happy to proceed then I will create auxiliary classes in the platform interface that I will gladly maintain 😄

[chipweinberger]

looks good! just saw this comment

[chipweinberger]

  Future<void> setOptions({
    bool darwinShowPowerAlert = true,
  }) {
    throw UnimplementedError();
  }

this should probably take arbitrary keys and values. a Map<String,dynamic>

I just added a new option today (12c17a6) needs to be future proof.

[chipweinberger]

Also I wonder if we should have more future proofing

  Future<void> callAndroidFunction({
    String functionName, Map<String, Dynamic>
  }) {
    throw UnimplementedError();
  }

  Future<void> callAppleFunction({
    String functionName, Map<String, Dynamic>
  }) {
    throw UnimplementedError();
  }

  Future<void> callWindowsFunction({
    String functionName, Map<String, Dynamic>
  }) {
    throw UnimplementedError();
  }

or maybe just

  Future<void> callPlatformSpecificFunction({
    String functionName, Map<String, Dynamic>
  }) {
    throw UnimplementedError();
  }

[chipweinberger]

but im still not that knowledgable about designing these interfaces. I think you know more than me about this.

[tnc1997]

I just added a new option today (12c17a6) needs to be future proof.

In this scenario we should add that parameter to the method and give it a reasonable default for compatibility.

Future<void> setOptions({
  bool darwinShowPowerAlert = true,
  bool iosRestoreState = false, // or true
}) {
  throw UnimplementedError();
}

Also I wonder if we should have more future proofing.

Personally I would strongly avoid that because platform interfaces conventionally define properties and/or methods for the functionality. When functionality is added to flutter_blue_plus, then we should add an associated method to the platform interface, which should then be implemented by the platform specific implementations that support that functionality. Granted this is a slightly different way of thinking to the method channel which is used as an "interface".

[chipweinberger]

let’s say the Windows 12 comes out and adds a few new ble functions. will the platform interface have to change?

or let’s say we want to add more arguments to setOptions. will the platform implantations all break?

[tnc1997]

let’s say the Windows 12 comes out and adds a few new ble functions. will the platform interface have to change?

In this scenario we should add those new method(s) to the platform interface as a non-breaking change e.g.

Future<void> newMethod() {
  throw UnimplementedError();
}

or let’s say we want to add more arguments to setOptions. will the platform implantations all break?

If we make a breaking change to the platform interface then this should be published as a breaking change.

If we expect options (for example) to change often then we should probably consider something like this:

class Options {
  final bool darwinShowPowerAlert;
  final bool additionalParameter;

  Options({
    this.darwinShowPowerAlert = true,
    this.additionalParameter = false, // or true
  });
}

abstract base class FlutterBluePlusPlatform {
  Future<void> setOptions(
    Options options,
  ) {
    throw UnimplementedError();
  }
}

[chipweinberger]

let's make the platform interface as future proof as possible

options will evolve over time

i don't want to have to maintain windows, web, linux, as they evolve

so the platform interface should allow them enough flexibility to add new functions without my involvement

[tnc1997]

let's make the platform interface as future proof as possible

so the platform interface should allow them enough flexibility to add new functions without my involvement

Just to confirm that the platform interface may have methods added to it in the future to cover potential functionality.

One of the reasons for a platform interface is that it is the interface used by all platforms not just Android/iOS/macOS.

[tnc1997]

I would be more than happy to maintain the platform interface in collaboration with you to reduce your burden.

Just double checking that you're happy to continue with us creating a platform interface for flutter_blue_plus?

[chipweinberger]

what is the status of being able to do things like this?

FlutterBluePlus.android.clearGattCache()? or FlutterBluePlus.android.someOtherMethod()

did we come to a solution?

[tnc1997]

what is the status of being able to do things like this?

Researching other examples of platform interfaces that typically isn't how they are created.

• https://github.com/yanshouwang/bluetooth_low_energy/blob/master/bluetooth_low_energy_platform_interface/lib/src/bluetooth_low_energy_manager.dart
• https://github.com/PhilipsHue/flutter_reactive_ble/blob/master/packages/reactive_ble_platform_interface/lib/src/reactive_ble_platform_interface.dart

FlutterBluePlus.android.clearGattCache()

This implies that each platform has its own interface and so isn't a shared platform interface.

Platform implementations should extends their platform interface class rather than implements it, as newly added methods to platform interfaces are not considered breaking changes. Extending a platform interface ensures that subclasses will get the default implementations from the base class, while platform implementations that implements their platform interface will be broken by newly added methods.

[tnc1997]

FlutterBluePlus.android.clearGattCache()

That isn't to say that you couldn't do that in the existing flutter_blue_plus package, but looking at other examples of platform interfaces you wouldn't do that in a flutter_blue_plus_platform_interface package. For example you could have a method like that in flutter_blue_plus that calls FlutterBluePlusPlatform.instance.clearGattCache().

[chipweinberger]

what would the user facing API be? If not FlutterBluePlus.windows.someNewMethod()?

Maybe FlutterBluePlusWindows.someNewMethod()?

[tnc1997]

what would the user facing API be? If not FlutterBluePlus.windows.someNewMethod()?

Maybe FlutterBluePlusWindows.someNewMethod()?

The methods in the platform interface could be as per the changes in this pull request, whilst the methods in flutter_blue_plus would be as they are now, albeit with implementations that use the platform interface instead of a method channel.

The methods in the platform interface should then be implemented by each platform specific implementation where appropriate and possible, throwing an unsupported error if that particular method is not supported on the platform.

[chipweinberger]

Maybe im not clear. Does the platform interface need to be a superset of all platforms?

There's no way for each implementation to expose custom functions and options themselves?

If so, that's pretty annoying because every addition will require the platform interface to change.

android has like 50 difference custom functions and flags. iOS has at least 20. I imagine web, linux, windows, also have lots of custom features. FBP eventually needs to support all of these features.

each platform needs more autonomy.

[chipweinberger]

Have you seen this package? https://pub.dev/packages/flutter_blue_plus_windows

He added Windows support to FBP without any involvement from me.

He used a wrapper architecture:
https://github.com/chan150/flutter_blue_plus_windows/blob/master/lib/src/wrapper/flutter_blue_plus_wrapper.dart

Perhaps you can add linux & web in a similar fashion?

[tnc1997]

Maybe im not clear. Does the platform interface need to be a superset of all platforms?

Yes, the platform interface should be a superset of all platforms, otherwise it is not a "true" platform interface. I would encourage you to take a look at the platform interfaces that I previously linked to and those here for inspiration.

When adding a method to one or more platforms, it should be added to the platform interface, then implemented in the platforms that support that particular functionality.

[chipweinberger]

Yes, the platform interface should be a superset of all platforms, otherwise it is not a "true" platform interface.

Unfortunately, this is just not something I'm comfortable maintaining long term. I know I wont have enough enthusiasm make each platform a first class citizen.

I would be more than happy to maintain the platform interface in collaboration with you to reduce your burden.

I appreciate that offer! But I think then FBP for android/ios/mac would not be fully autonomous? That would make development of the main platforms harder.

---

I think you should consider that approach taken by https://pub.dev/packages/flutter_blue_plus_windows. It's much more autonomous, and the "wrapper" is not that difficult to write/maintain.

[tnc1997]

I think you should consider that approach taken by https://pub.dev/packages/flutter_blue_plus_windows. It's much more autonomous, and the "wrapper" is not that difficult to write/maintain.

Unfortunately as Stuart highlighted in this issue, that approach has a number of significant disadvantages including being very fragile and prone to breakage. This makes ongoing maintenance of the platform implementation a lot more complicated compared to simply implementing a platform interface. It is a shame that flutter_blue_plus will not create and publish a platform interface as per other popular plugins at this time.

[chipweinberger]

It is a shame that flutter_blue_plus will not create and publish a platform interface as per other popular plugins at this time.

I share your disappointment. But I just don't have the bandwidth to do it justice.

I welcome you to create flutter_blue_plus_universal though, if you have the energy. You can even wrap FBP, so that you don't need to maintain android/mac/iOS yourself. Then you can just "peg" your pubspec.yaml to depend on a specific FBP version, and it will never break. FBP is rather stable at this point.

Yes, there are tradeoffs for all the approaches, to be sure.

Thanks for your time in evaluate the federated approach. I'm sorry that I was ultimately not up for it.