Skip to content
This repository was archived by the owner on Dec 20, 2019. It is now read-only.

Fixes connection error handling#30

Open
jamuus wants to merge 2 commits into
sphero-inc:masterfrom
jamuus:master
Open

Fixes connection error handling#30
jamuus wants to merge 2 commits into
sphero-inc:masterfrom
jamuus:master

Conversation

@jamuus

@jamuus jamuus commented Oct 20, 2015

Copy link
Copy Markdown

Before, when initialising serial port, it was defaulting to open immediately, which was using its own callback that emitted an error when there was one. serialport.js looks like it was trying to give a callback of its own but the open event never passes a value.

I have set serialport to not open immediately, and corrected the open line such that an error will be passed to the callback. Then I added error handling to the sphero connect function such that an error is passed to whatever calls connect.

This now means that connect's callback isn't only called on a successful connection. This could be avoided by properly catching and emitting the error events such that a orb.on('error'... will catch them.

@edgarsilva

Copy link
Copy Markdown
Contributor

You are correct, the port should not be open instantly, but wait for sphero.js to open it. Could you pls fix the tests failing so we can merge this?

@jamuus

jamuus commented Oct 23, 2015

Copy link
Copy Markdown
Author

I'll fix the tests and cleanup these commits when I get a chance to learn how Mocha works.

@zoetrope69

Copy link
Copy Markdown
Contributor

Have fixed the linting but don't know what I'm doing with tests aha

@jamuus

jamuus commented Feb 6, 2016

Copy link
Copy Markdown
Author

I've removed the irrelevant commits.

@deadprogram

Copy link
Copy Markdown
Contributor

Thanks @jamuus I think it just needs the change I mentioned here #57 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants