Add neutral wifi command to list Tessel Wifi Information #282

MrNice · 2015-09-07T08:22:47Z

It does this:

~/s/t/bin ❯❯❯ /.nvm/versions/io.js/v2.2.1/bin/iojs tessel-2.js wifi
INFO Connecting to Tessel...
INFO Connected to pixel over USB
INFO Connected to "NETWORK"
INFO IP Address: 10.0.0.40
INFO Signal Strength: (61/70)
INFO Bitrate: 65mbps

Adds two commands, and a fairly painful method to wifi.js.

Fix JSHINT issues

johnnyman727 · 2015-09-07T17:08:36Z

bin/tessel-2.js

-function closeFailedCommand(err) {
-  // If the returned value is an error
+// Allow options to be partially applied
+function closeFailedCommand(opts, err) {


@MrNice what's the rationale modifying this function? You could previouslyt report an error if you pass it an error or you can have it warn the user if it's just a string

Undoing, sorry.

johnnyman727 · 2015-09-07T17:17:27Z

@MrNice thanks for the contribution! It works really well when Tessel is connected. I added a few implementation notes on the relevant lines.

In addition, the t2 wifi command should report that it's not connected to WiFI according to the spec. Currently, it throws an angry error:

➜  t2-cli git:(add-wifi-command) t2 wifi
INFO Connecting to Tessel...
INFO Connected to Harold over USB
Unhandled rejection TypeError: Reduce of empty array with no initial value
    at Array.reduce (native)
    at /Users/Jon/Work/technical/t2-cli/lib/controller.js:467:14
    at tryCatcher (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/promise.js:503:31)
    at Promise._settlePromiseAt (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/promise.js:577:18)
    at Promise._settlePromises (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/promise.js:693:14)
    at Async._drainQueue (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/async.js:123:16)
    at Async._drainQueues (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/async.js:133:10)
    at Immediate.Async.drainQueues [as _onImmediate] (/Users/Jon/Work/technical/t2-cli/node_modules/bluebird/js/main/async.js:15:14)
    at processImmediate [as _immediateCallback] (timers.js:367:17)

Lastly, you'll have to run grunt when you finish writing your code. That will run our JSHinter, check for any styling issues and run our unit tests. It looks like this commit caused one of the unit tests to fail.

Let me know if you have any questions!

johnnyman727 · 2015-09-07T17:22:39Z

Actually, I can't reproduce the failure Travis detected... I'm restarting the test to check if it's an intermittent type of issue.

MrNice · 2015-09-07T17:27:17Z

Re: Travis: last night it bounced this PR for jshint reasons, so I added 15
semicolons, but then it failed on a unit test that I couldn't reproduce
locally.

That specific error with the reduce will be fixed if its just passed an
array literal.

I'll work on this stuff when I get home.
On Sep 7, 2015 10:22 AM, "Jon" notifications@github.com wrote:

Actually, I can't reproduce the failure Travis detected... I'm restarting
the test to check if it's an intermittent type of issue.

—
Reply to this email directly or view it on GitHub
#282 (comment).

johnnyman727 · 2015-09-07T17:30:49Z

Ah the Travis issue is directly related to it being a build server with no USB peripherals. Somehow a path of code is running that attempts to get a USB Device List (not sure why this particular PR caused it though). I can look further into it later today.

rwaldron · 2015-09-08T16:41:59Z

lib/controller.js

+            .filter(function(addr) {
+              return addr.length;
+            });
+


I'd like to see this consolidated. The last three could be a single reduce

If the intent of the code is clear, why consolidate

I can replace the last three with the following

.reduce(function(list, ip) { if (/addr/.exec(item)) { var ip = chunk.split(':')[1]; if (ip.length) list.push[ip]; } return list; });

But it doesn't make it more clear (to me) what is happening.

:/

This is what reduce is for: consolidating map/filter operations.

I'll change it if that's wanted, but I don't think it's clearer.

On Tue, Sep 8, 2015 at 10:08 AM, Rick Waldron notifications@github.com
wrote:

In lib/controller.js
#282 (comment):

.map(function(line) {

return line.split(' ');

})

.reduce(function(a, b) {

return a.concat(b);

})

.filter(function(item) {

return /addr/.exec(item);

})

.map(function(chunk) {

return chunk.split(':')[1];

})

.filter(function(addr) {

return addr.length;

});

:/

This is what reduce is for: consolidating map/filter operations.

—
Reply to this email directly or view it on GitHub
https://github.com/tessel/t2-cli/pull/282/files#r38951868.

rwaldron · 2015-09-08T16:44:48Z

In addition to @johnnyman727's review points and fixing the existing failure, this needs new tests to cover the new behavior.

MrNice · 2015-09-08T17:58:36Z

@johnnyman727

Is this a reasonable error message?

~/s/t/bin ❯❯❯ iojs tessel-2.js wifi
INFO Connecting to Tessel...
INFO Connected to pixel over USB
ERR! This Tessel is not connected to Wi-Fi

johnnyman727 · 2015-09-08T18:10:20Z

@MrNice the issue with the spec suggests:

Not Connected (run 'tessel wifi -l' to see available networks)

I'd suggest one modification:

{TESSEL_NAME} is not connected to WiFi (run 'tessel wifi -l' to see available networks)

johnnyman727 · 2015-09-10T02:38:51Z

@MrNice did you need any more help with this or are you all good?

MrNice · 2015-09-10T02:39:52Z

Oh I'm rushing around busy. Got those other 2 pr's to wrap up first :(

On Wed, Sep 9, 2015 at 7:38 PM, Jon notifications@github.com wrote:

@MrNice https://github.com/MrNice did you need any more help with this
or are you all good?

—
Reply to this email directly or view it on GitHub
#282 (comment).

johnnyman727 · 2015-09-10T02:45:34Z

No worries - just wanted to make sure you weren't hung up on anything from me.

johnnyman727 · 2015-09-13T23:26:50Z

@MrNice can I help merge this?

MrNice · 2015-09-14T23:38:42Z

@johnnyman727 Wrapping this up and adding tests. Did we ever figure out travis?

johnnyman727 · 2015-10-04T00:25:13Z

Closing in favor of rebased and fixed #362.

Add neutral wifi command to list Tessel Wifi Information

9869ac3

Fix JSHINT issues

MrNice force-pushed the add-wifi-command branch from b14873a to 9869ac3 Compare September 7, 2015 08:26

MrNice mentioned this pull request Sep 7, 2015

t2 wifi needs filling out #82

Closed

7 tasks

johnnyman727 reviewed Sep 7, 2015
View reviewed changes

rwaldron reviewed Sep 8, 2015
View reviewed changes

MrNice added 2 commits September 14, 2015 20:09

Test wifi neutral, and revert some changes to the logger

c4c0b64

Remove console.log

f8723d0

johnnyman727 closed this Oct 4, 2015

Frijol deleted the add-wifi-command branch October 6, 2015 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add neutral wifi command to list Tessel Wifi Information #282

Add neutral wifi command to list Tessel Wifi Information #282

MrNice commented Sep 7, 2015

johnnyman727 Sep 7, 2015

MrNice Sep 8, 2015

johnnyman727 commented Sep 7, 2015

johnnyman727 commented Sep 7, 2015

MrNice commented Sep 7, 2015

johnnyman727 commented Sep 7, 2015

rwaldron Sep 8, 2015

MrNice Sep 8, 2015

rwaldron Sep 8, 2015

MrNice Sep 8, 2015

rwaldron commented Sep 8, 2015

MrNice commented Sep 8, 2015

johnnyman727 commented Sep 8, 2015

johnnyman727 commented Sep 10, 2015

MrNice commented Sep 10, 2015

johnnyman727 commented Sep 10, 2015

johnnyman727 commented Sep 13, 2015

MrNice commented Sep 14, 2015

johnnyman727 commented Oct 4, 2015

Add neutral wifi command to list Tessel Wifi Information #282

Add neutral wifi command to list Tessel Wifi Information #282

Conversation

MrNice commented Sep 7, 2015

johnnyman727 Sep 7, 2015

Choose a reason for hiding this comment

MrNice Sep 8, 2015

Choose a reason for hiding this comment

johnnyman727 commented Sep 7, 2015

johnnyman727 commented Sep 7, 2015

MrNice commented Sep 7, 2015

johnnyman727 commented Sep 7, 2015

rwaldron Sep 8, 2015

Choose a reason for hiding this comment

MrNice Sep 8, 2015

Choose a reason for hiding this comment

rwaldron Sep 8, 2015

Choose a reason for hiding this comment

MrNice Sep 8, 2015

Choose a reason for hiding this comment

rwaldron commented Sep 8, 2015

MrNice commented Sep 8, 2015

johnnyman727 commented Sep 8, 2015

johnnyman727 commented Sep 10, 2015

MrNice commented Sep 10, 2015

johnnyman727 commented Sep 10, 2015

johnnyman727 commented Sep 13, 2015

MrNice commented Sep 14, 2015

johnnyman727 commented Oct 4, 2015