Skip to content
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

Add a placeholder implementation for non-mobile platforms #688

Merged
merged 5 commits into from
Sep 7, 2023

Conversation

swift-kim
Copy link
Contributor

Sets the default value of ReactiveBlePlatform.instance to an instance of _PlaceholderImplementation, rather than ReactiveBleMobilePlatform (current) which is for mobile platforms only. This is necessary so that federated implementations of this plugin (endorsed or not) can be added for non-mobile (desktop) platforms.

The same approach is being used by 1st-party plugin developers as they have decided to remove default method channel implementations from their platform interface packages. For example: https://github.com/flutter/plugins/blob/2ce625f1a87e4b738f63a6f1f7e3e040e7c0b87f/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart#L22

After this change, a new platform implementation can be added by providing a static registerWith method that is used to register the plugin for that platform on app startup (documentation). Here is the example:

I personally need this change to add a Tizen platform implementation of this plugin for our project. Any comments will be appreciated. Thank you!

@swift-kim
Copy link
Contributor Author

The following PRs will need an update after this change.

@swift-kim
Copy link
Contributor Author

Should I increase the version numbers in pubspec.yaml's and update the CHANGELOG files myself if I want to release this change, or is the release done on a regular basis?

@swift-kim
Copy link
Contributor Author

@Taym95 Hi. If you have some time, could you please take a look at this change and give me some advice?

@Taym95
Copy link
Collaborator

Taym95 commented Jul 5, 2023

@Taym95 Hi. If you have some time, could you please take a look at this change and give me some advice?

I will review it today, sorry for not responding quickly

@swift-kim
Copy link
Contributor Author

@Taym95 Hi. Could you review and give some comments?

Copy link
Collaborator

@Taym95 Taym95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution, this will be helpful for endorsed package indeed.

@Taym95
Copy link
Collaborator

Taym95 commented Sep 6, 2023

@Taym95 Hi. Could you review and give some comments?

please update your branch

@swift-kim
Copy link
Contributor Author

@Taym95 Sure. I hope there will be a version bump soon after merge so that this change can be released.

@Taym95
Copy link
Collaborator

Taym95 commented Sep 7, 2023

@Taym95 Sure. I hope there will be a version bump soon after merge so that this change can be released.

Yes

@Taym95 Taym95 merged commit d07b4cd into PhilipsHue:master Sep 7, 2023
1 check passed
cenumi pushed a commit to cenumi/flutter_reactive_ble that referenced this pull request Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants