Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.

Auto discovery#154

Open
scholvat wants to merge 31 commits into
masterfrom
auto-discovery
Open

Auto discovery#154
scholvat wants to merge 31 commits into
masterfrom
auto-discovery

Conversation

@scholvat
Copy link
Copy Markdown
Contributor

@scholvat scholvat commented Feb 1, 2017

No description provided.

@scholvat scholvat closed this Feb 1, 2017
@scholvat scholvat reopened this Feb 2, 2017
Comment thread app/connections/DataRelay.js Outdated
Logger.data(JSON.stringify(TelemetryData.getHeaders()), 'DATA_RELAY_HEADERS');
StatusManager.addStatus('Received headers from data_relay', 3, 3000);


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Get rid of the extra lines

Comment thread app/connections/DataRelay.js Outdated
TelemetryData.setCurrentStateFromString(data);
TelemetryData.emitPackets();
Logger.data(JSON.stringify(TelemetryData.getCurrentState()), 'DATA_RELAY_DATA');
StatusManager.setStatusCode('TIMEOUT_DATA_RELAY', false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dont add tabs. Set your editor to use 2 spaces

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are spaces...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Though I see what you mean, I will remove the 2 extra spaces

Comment thread app/connections/DataRelay.js Outdated
StatusManager.setStatusCode('CONNECTED_DATA_RELAY', true);
});
//run ipconfig command
var exec = require("child_process").exec;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Place your require calls at the begining of the file. Remember to document all the require calls in the file header as well

Comment thread app/connections/DataRelay.js Outdated
//run and parse ipconfig
exec("ipconfig", function(err, stdout, stderr) {
if (err) {
callback(err, stderr);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As discussed, log an error instead

Comment thread app/connections/DataRelay.js Outdated
parseHeaders(data);
do {
//search for any term in the form Bcast:#.#.#.# or broadcast:#.#.#.#
match = parsingStr.toString().match(/(?:Bcast|broadcast):([\d.]*)/);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I dont like having parsing logic in this file. I think you should make some kind of utility function in another module for parsing ifconfig and ipconfig outputs (if theres no module to do that). It'll also make this easier to unit test

Comment thread app/connections/DataRelay.js Outdated

var udp_open = false;

server.on('error', (err) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd like to keep the codebase ES5 consistent, so dont use arrow functions (I do use the ${variable} in my console logs however, since its an amazing feature)

Comment thread app/connections/DataRelay.js Outdated
//the message should include the port number
Logger.info('Data-relay at ' + rinfo.address.toString() + ':' + msg.toString());
connectTCP(rinfo.address.toString(), msg.toString());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no line

Comment thread app/connections/DataRelay.js Outdated
StatusManager.setStatusCode('TIMEOUT_UDP',true);
server.close();
}
}, 1000);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

make the timeout a config variable

Comment thread config/network-config.js
multiecho_timeout:5000, //in milliseconds, the amount of time a socket will wait idle before declaring itself as timed out and closing the connection
//legacy data relay
datarelay_legacy_host:'127.0.0.1',
datarelay_legacy_port:'1234'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tabs

var input = $('<input type="text">');

var setting_type = typeof(settings.default_settings[key]) //check input type
if( setting_type === "boolean"){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

space

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what do you mean? I added a line break in 56 for better readability

@scholvat
Copy link
Copy Markdown
Contributor Author

@sergei1152 can you take a look at this again? I redid some of the parsing stuff.

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.

2 participants