Skip to content
This repository has been archived by the owner on Apr 17, 2020. It is now read-only.

Implement -k switch to use self signed certificates, cli parameters cleanup #7

Merged
merged 2 commits into from
Jan 1, 2018

Conversation

juzam
Copy link
Contributor

@juzam juzam commented Dec 30, 2017

This PR implements -k command line switch (-k inspired by cURL) in order to connect to a mqtt broker with TLS and self signed certificates (or certificate signed by a non trusted CA by default)

As per mqtt.Client(streamBuilder, options) it is done by passing rejectUnauthorized: false as an added mqtt connectio option. Defaults to false.

Also cleaned up a bit -l logging option adding proper .choices in config.js
-h and Readme changed accordingly

n: 'xiaomi'
})
.version()
.help('help')
.wrap(null)
Copy link
Owner

Choose a reason for hiding this comment

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

What does this row do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that disables yargs default usage output (which is 80 columns) basically demaning -h usage output wrapping to the terminal. (ref doc .wrap )[https://github.com/yargs/yargs/blob/HEAD/docs/api.md#wrapcolumns]. It's purely cosmetic, unfortunately I couldn't find a way to wrap properly by using something like .wrap(yargs.terminalWidth()) as suggested in the docs.

@svrooij
Copy link
Owner

svrooij commented Jan 1, 2018

@juzam I fixed the travis error, so you can ignore that but could you have a look a my comments?

@juzam
Copy link
Contributor Author

juzam commented Jan 1, 2018

@svrooij thanks for fixing the travis check (I didn't know it was broken prior to submitting the PR, sorry). See my comment to the .wrap line. That's not mandatory, but I thought that the usage output was clearer without the hardcoded wrapping at 80 columns by default.

src/bridge.js Outdated
@@ -27,7 +27,7 @@ function start () {
})
}

const mqttOptions = {
let mqttOptions = {
Copy link
Owner

Choose a reason for hiding this comment

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

I like short code, could you change this to the following?

  const mqttOptions = {
    will: {
      topic: config.name + '/connected',
      message: 0,
      qos: 0,
      retain: true
    },
    rejectUnauthorized = !config.insecure
  }

@svrooij
Copy link
Owner

svrooij commented Jan 1, 2018

@juzam
Copy link
Contributor Author

juzam commented Jan 1, 2018

@svrooij argh, I missed the comment. should be fixed now.

@svrooij svrooij merged commit c247fa3 into svrooij:master Jan 1, 2018
@svrooij
Copy link
Owner

svrooij commented Jan 1, 2018

It is now released in npm. I still think getting an official certificate isn’t very hard (like for free with lets encrypt), but that is up to you.

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