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] typeof selectDevice function #118

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

iharuya
Copy link

@iharuya iharuya commented Apr 23, 2024

According to the documentation, user can select device manually in this way:

// select via field/value
await neurosity.selectDevice(["deviceNickname", "Crown-A1B"]);

However this causes a typescript error so I fixed it.

Copy link

netlify bot commented Apr 23, 2024

👷 Deploy request for neurosity-sdk-js pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 640f238

@andrewjaykeller
Copy link
Contributor

Should is be passed as an array or object?

@iharuya
Copy link
Author

iharuya commented May 1, 2024

Should is be passed as an array or object?

Good point about using an object, but keeping it simple with direct key-value input might be easier. Moreover, I think splitting the selectDevice function into two clear methods would definitely make it less confusing.

@iharuya
Copy link
Author

iharuya commented May 1, 2024

I think splitting the selectDevice function into two clear methods would definitely make it less confusing.

I implemented this. Please check the change and I would like to get a feedback, @andrewjaykeller

@andrewjaykeller
Copy link
Contributor

Looks cool! Could you please add a comment block above the functions that define the params?

Many thanks!

@iharuya
Copy link
Author

iharuya commented May 22, 2024

OK. Comment blocks added for the new functions. @andrewjaykeller

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