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

Added detection for NW.js version 0.13+ #108

Merged
merged 3 commits into from Feb 4, 2016

Conversation

Projects
None yet
4 participants
@djipco
Contributor

djipco commented Jan 31, 2016

Since v0.13, NW.js fully supports the chrome.* APIs. This includes chrome.serial upon which browser-serialport is constructed. This pull request simply detects if the firmata module is being run inside NW.js in which case it uses the browser-serialport module instead of the serialport module.

Since native modules need to be specifically recompiled for NW.js, this makes it much easier to use the serial port in that environment.

cotejp added some commits Jan 31, 2016

Added support for NW.js v0.13+
When Firmata is running within NW.js version 13 or greater, it will use the browser-serialport module.
Converted single quotes to double quotes
I simply converted the single quotes to double quotes so it wouldn't trip the automated code style test.
Show outdated Hide outdated lib/firmata.js
@@ -22,7 +22,7 @@ var util = require("util"),
try {
if (process.browser) {
if (process.browser || parseFloat(process.versions["nw"]) >= 0.13) {

This comment has been minimized.

@rwaldron

rwaldron Feb 1, 2016

Collaborator

(process.versions.nw && parseFloat(process.versions.nw) >= 0.13)

@rwaldron

rwaldron Feb 1, 2016

Collaborator

(process.versions.nw && parseFloat(process.versions.nw) >= 0.13)

@djipco

This comment has been minimized.

Show comment
Hide comment
@djipco

djipco Feb 1, 2016

Contributor

Yep, that's a better syntax. By the way, Rick, if this pull request is merged in, I will submit a very similar one for Johnny-Five.

If both Firmata and Johnny-Five checked for the presence of NW.js v0.13+ and loaded browser-serialport accordingly, it would be awesome. It would then be trivial to create a GUI for any Johnny-Five project.

The current trouble with node-serialport within NW.js is that the module must be specifically recompiled for NW.js in order to run properly. This is prohibitively complicated for beginners. However, with version 0.13, NW.js added full support for chrome.* APIs. This includes chrome.serial but also other cool ones like chrome.bluetooth and chrome.tts (text-to-speech).

I tested browser-serialport + firmata + johnny-five + NW.js and it works flawlessly. To me, this is a kickass combination for physical computing and robotics.

Contributor

djipco commented Feb 1, 2016

Yep, that's a better syntax. By the way, Rick, if this pull request is merged in, I will submit a very similar one for Johnny-Five.

If both Firmata and Johnny-Five checked for the presence of NW.js v0.13+ and loaded browser-serialport accordingly, it would be awesome. It would then be trivial to create a GUI for any Johnny-Five project.

The current trouble with node-serialport within NW.js is that the module must be specifically recompiled for NW.js in order to run properly. This is prohibitively complicated for beginners. However, with version 0.13, NW.js added full support for chrome.* APIs. This includes chrome.serial but also other cool ones like chrome.bluetooth and chrome.tts (text-to-speech).

I tested browser-serialport + firmata + johnny-five + NW.js and it works flawlessly. To me, this is a kickass combination for physical computing and robotics.

@soundanalogous

This comment has been minimized.

Show comment
Hide comment
@soundanalogous

soundanalogous Feb 1, 2016

Member

With all of the transports I've been adding to Firmata (on the Arduino side), I wonder if it's worth it to rethink how node-firmata is initialized. I think it makes sense to default to node-serialport for backwards compatibility, but we should make it easy for people to use Ethernet, WiFi, BLE, and browser serial as well.

Member

soundanalogous commented Feb 1, 2016

With all of the transports I've been adding to Firmata (on the Arduino side), I wonder if it's worth it to rethink how node-firmata is initialized. I think it makes sense to default to node-serialport for backwards compatibility, but we should make it easy for people to use Ethernet, WiFi, BLE, and browser serial as well.

@djipco

This comment has been minimized.

Show comment
Hide comment
@djipco

djipco Feb 2, 2016

Contributor

I am not suggesting that the default behaviour be changed. In fact, the change I proposed is quite the opposite. The default module remains node-serialport and in the case where process.versions.nw is detected, browser-serialport kicks in. In the case where Firmata is being run inside an older NW.js version, it simply behaves as before. All other use cases remain unaffected.

It's a small check with a low-footprint that could bring huge benefits to those who want to build a GUI for their project.

Cheers!

Contributor

djipco commented Feb 2, 2016

I am not suggesting that the default behaviour be changed. In fact, the change I proposed is quite the opposite. The default module remains node-serialport and in the case where process.versions.nw is detected, browser-serialport kicks in. In the case where Firmata is being run inside an older NW.js version, it simply behaves as before. All other use cases remain unaffected.

It's a small check with a low-footprint that could bring huge benefits to those who want to build a GUI for their project.

Cheers!

@soundanalogous

This comment has been minimized.

Show comment
Hide comment
@soundanalogous

soundanalogous Feb 2, 2016

Member

I know you are not, but I am suggesting it to be changed so node-firmata doesn't seem to be serial only (whether that is node-serialport or browser serial). I'll open a separate issue to track.

Member

soundanalogous commented Feb 2, 2016

I know you are not, but I am suggesting it to be changed so node-firmata doesn't seem to be serial only (whether that is node-serialport or browser serial). I'll open a separate issue to track.

@djipco

This comment has been minimized.

Show comment
Hide comment
@djipco

djipco Feb 2, 2016

Contributor

Oh, I see. Sorry about the confusion. I actually agree with you that Firmata should slowly transition to a transport-independent model.

Contributor

djipco commented Feb 2, 2016

Oh, I see. Sorry about the confusion. I actually agree with you that Firmata should slowly transition to a transport-independent model.

rwaldron added a commit that referenced this pull request Feb 4, 2016

Merge pull request #108 from cotejp/master
Added detection for NW.js version 0.13+

@rwaldron rwaldron merged commit e368ef9 into firmata:master Feb 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment