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

More robust serial connection establishment & error handling #4

Merged
merged 8 commits into from
Oct 13, 2014

Conversation

tjanson
Copy link

@tjanson tjanson commented Oct 9, 2014

Instead of trying to establish the serial connection in the Board constructor, delegate this to a separate setup(). (API change!)
This allows the user to add listeners to the Board’s error event, which signifies that the connection could not be established. (This was already intended, but wasn't possible, as far as I can tell.)
Also displays a more intuitive error message.

Depends on #3. [Is this how you deal with Pull Request dependencies? I hope merging #3 will update this PR to only include the relevant, three most recent commits. This github stuff is more painful than I’d hoped.]

ni-c and others added 6 commits February 19, 2013 21:50
Conflicts:
	examples/rc.js
	index.js
	lib/board.js
	lib/rc.js
	package.json
	src/du.ino
Instead of having both `src/du.ino` (old, invalid path) and
`src/duino/duino.ino` (new), make the former a symlink to the latter.

(The Arduino IDE requires this folder format.)
Instead of trying to establish the serial connection in the Board constructor,
delegate this to a separate `setup()`. (**API change**!)
This allows the user to add listeners to the Board’s `error` event, which
signifies that the connection could not be established. (This was already
intended, but wasn't possible, as far as I can tell.)
Also displays a more intuitive error message.
`setup()` returns `this`, which allows for an easier transition from old code:

```
var board = new arduino.Board(); // old
var board = new arduino.Board().setup(); // new
```
@tjanson
Copy link
Author

tjanson commented Oct 13, 2014

I’ll rebase this or something, so the duplicate commits (or whatever those are) are removed.

@ni-c
Copy link
Owner

ni-c commented Oct 13, 2014

There is a confict in libs/board.js after merging the previous PR:

<<<<<<< HEAD
  this.debug = options && options.debug || false;
  this.device = options && options.device || 'tty(USB|ACM|S)[0-9]?';
=======
  this.debug    = options && options.debug    || false;
  this.devregex = options && options.devregex || 'usb|ttyACM*|ttyS0';
>>>>>>> 00a7728c1850b44d32fd200e1ec3fbd579c0cbf6

I have taken the version from this PR, i hope that was the right version?

@ni-c ni-c merged commit 00a7728 into ni-c:master Oct 13, 2014
@tjanson
Copy link
Author

tjanson commented Oct 13, 2014

Okay, cool. Yes, both work, but the bottom version matches false positives (e.g., /dev/usb). (See ecto#56)

@tjanson tjanson deleted the gracefulish-detectconnect branch October 13, 2014 13:07
@tjanson
Copy link
Author

tjanson commented Oct 13, 2014

Heads up: This was commited without being resolved. Like I said, the bottom should be used.

@ni-c
Copy link
Owner

ni-c commented Oct 13, 2014

Ups, thank you, fixed it. That's strange, my merge-commit is in the history.

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