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

Fix README code #222

Merged
merged 4 commits into from Aug 31, 2020
Merged

Fix README code #222

merged 4 commits into from Aug 31, 2020

Conversation

ligi
Copy link
Contributor

@ligi ligi commented Aug 1, 2020

also this line seems to be wrong:

setNotificationCallback(firstCharacteristic, callback)

I guess it should just be

setNotificationCallback(firstCharacteristic)

but did not add to this PR as not 100% sure about this yet

PS: love the humor in the README!

@CLAassistant
Copy link

CLAassistant commented Aug 1, 2020

CLA assistant check
All committers have signed the CLA.

@philips77
Copy link
Member

These is a callback parameter in setNotificationCallback.

But could you also fix a typo in "Very, very long text that will NOT fit into MTU".

@ligi
Copy link
Contributor Author

ligi commented Aug 1, 2020

These is a callback parameter in setNotificationCallback.

I don't see the parameter there:

Selection_108

But could you also fix a typo in "Very, very long text that will NOT fit into MTU".

done - but thought this is slang ;-)

@philips77
Copy link
Member

Oh, true. I don't remember my own lib...

@ligi
Copy link
Contributor Author

ligi commented Aug 1, 2020

I know that feeling ,-)

@philips77
Copy link
Member

philips77 commented Aug 1, 2020

It should be setNotificationCallback(firstCharacteristic).with(callback);

@philips77
Copy link
Member

Hi, could you do the last change and I'll merge the PR?

@ligi
Copy link
Contributor Author

ligi commented Aug 3, 2020

sure - but not yet sure how the last change would look like - if you mean

setNotificationCallback(firstCharacteristic).with(callback);

callback is not defined anywhere

@philips77
Copy link
Member

OK, let's change it to:

setNotificationCallback(firstCharacteristic)
	.with((device, data) -> log(Log.INFO, "Notification received: " + data));

as this can just demonstrate use of the API.

@philips77 philips77 changed the base branch from master to develop August 31, 2020 07:50
@philips77 philips77 merged commit ab5c1a3 into NordicSemiconductor:develop Aug 31, 2020
@philips77
Copy link
Member

Thanks again for your contribution! I've just merged the PR.

@philips77 philips77 mentioned this pull request Jun 23, 2021
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.

None yet

3 participants