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.

Sign up for GitHub

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

Jump to bottom

Adding in port selection to serialport-term. #1448

Merged
merged 4 commits into from
Feb 3, 2018
Merged

Adding in port selection to serialport-term. #1448

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60c5c40
Adding in port selection to serialport-term.
robertmassaioli Jan 21, 2018
a54bab0
Fix the command line arguments.
robertmassaioli Jan 21, 2018
1e13043
Fixing eslint mistakes.
robertmassaioli Jan 21, 2018
125bb3b
Make all of the negative errors positive in serialport-terminal.
robertmassaioli Jan 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions bin/terminal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@
const SerialPort = require('../lib/');
const version = require('../package.json').version;
const args = require('commander');
const List = require('prompt-list');

function makeNumber(input) {
return Number(input);
}

args
.version(version)
.usage('-p <port> [options]')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since port is no longer a required argument we can remove references to it.

.usage('[options]')
.description('A basic terminal interface for communicating over a serial port. Pressing ctrl+c exits.')
.option('-l --list', 'List available ports then exit')
// TODO make the port not a flag as it's always required
.option('-p, --port <port>', 'Path or Name of serial port')
.option('-b, --baud <baudrate>', 'Baud rate default: 9600', makeNumber, 9600)
.option('--databits <databits>', 'Data bits default: 8', makeNumber, 8)
Expand All @@ -36,21 +36,53 @@ function listPorts() {
});
};

function createPort() {
if (!args.port) {
args.outputHelp();
args.missingArgument('port');
process.exit(-1);
function setupPort() {
if(args.port) {
createPort(args.port);
}

SerialPort.list((err, ports) => {
if (err) {
console.error('Error listing ports, and missing port argument.', err);
args.outputHelp();
args.missingArgument('port');
process.exit(-1);
} else {
if(ports.length > 0) {
var portSelection = new List({
name: 'serial-port-selection',
message: 'Select a serial port to open',
choices: ports.map((port, i) => `[${i + 1}]\t${port.comName}\t${port.pnpId || ''}\t${port.manufacturer || ''}`)
});

portSelection.run()
.then(answer => {
const choice = answer.split('\t')[1];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the hackiest part of the whole PR. Relying on the choices to be tab separated in order to extract the comName.

I would prefer it if the prompt-list library had a name and value fields so that what you display could be different to what gets returned as an 'answer' but, alas, it does not look like that is the case.

console.log(`Opening serial port: ${choice}`);
createPort(choice);
})
.catch(error => {
console.log(`Could not select a port: ${error}`);
process.exit(-2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what your policy is on the error code numbering that should be returned. Could use feedback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets make it a positive int for fewer surprises cross platform

});
} else {
args.outputHelp();
args.missingArgument('port');
process.exit(-1);
}
}
});
}

function createPort(selectedPort) {
const openOptions = {
baudRate: args.baud,
dataBits: args.databits,
parity: args.parity,
stopBits: args.stopbits
};

const port = new SerialPort(args.port, openOptions);
const port = new SerialPort(selectedPort, openOptions);

process.stdin.resume();
process.stdin.setRawMode(true);
Expand Down Expand Up @@ -82,5 +114,5 @@ function createPort() {
if (args.list) {
listPorts();
} else {
createPort();
setupPort();
}
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"nan": "^2.6.2",
"prebuild-install": "^2.4.1",
"promirepl": "^1.0.1",
"prompt-list": "^3.1.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does require a new dependency. I don't know if this will be contentious or not. Please let me know.

"safe-buffer": "^5.0.1"
},
"devDependencies": {
Expand Down Expand Up @@ -99,6 +100,7 @@
"lint": "eslint lib test bin examples",
"rebuild-all": "npm rebuild && node-gyp rebuild",
"repl": "node bin/repl.js",
"terminal": "node bin/terminal.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful for development because of: npm/npm#7137

"stress": "mocha --no-timeouts test/arduinoTest/stress.js",
"test": "istanbul cover ./node_modules/mocha/bin/_mocha",
"test:watch": "mocha -w",
Expand Down