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

Feature/connect options notify #460

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

devVadimAlbul
Copy link

@devVadimAlbul devVadimAlbul commented May 7, 2020

Please you need to add this changed code to your branch for the possibility to set options to the device to notify the app when device connection/disconnect in background mode.

It works with changes in the library
MultiPlatformBleAdapter

@CLAassistant
Copy link

CLAassistant commented May 7, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@mikolak mikolak left a comment

Choose a reason for hiding this comment

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

There's one more issue to solve in the PR to MultiplatformBleAdapter, but afterwards we can go ahead and start merging! Thanks for both pull requests, tremendous work!

_manager.connectToPeripheral(identifier,
isAutoConnect: isAutoConnect,
requestMtu: requestMtu,
refreshGatt: refreshGatt,
timeout: timeout);
timeout: timeout,
isNotifyOnConnection: isNotifyOnNotification,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
isNotifyOnConnection: isNotifyOnNotification,
isNotifyOnConnection: isNotifyOnConnection,

Comment on lines 56 to 58
bool isNotifyOnConnection = false,
bool isNotifyOnDisconnection = false,
bool isNotifyOnNotification = false}) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add dart docs for the new arguments?

@@ -0,0 +1 @@
0c043f712767a79a690d42e9fd9f6d89
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file should be ignored, I think?

@@ -62,6 +62,7 @@ target 'Runner' do
end

# Keep pod path relative so it can be checked into Podfile.lock.
pod 'MultiplatformBleAdapter', :git => 'https://github.com/devVadimAlbul/MultiPlatformBleAdapter.git', :branch => 'feature/connect-options'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will have to be reverted before merging - marking to not forget

@okocsis
Copy link
Contributor

okocsis commented Apr 18, 2021

@devVadimAlbul is this PR still alive?
@mikolak will this also be part of 3.0 release, as it introduces API changes?

@mikolak
Copy link
Collaborator

mikolak commented Apr 18, 2021

@devVadimAlbul is this PR still alive?
@mikolak will this also be part of 3.0 release, as it introduces API changes?

As this is a new feature that does not introduce any breaking changes, it can go in any major release and I'd rather not postpone the release too much. I'm only one man with one or two evenings a week maximum.

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.

4 participants