NEEOInc / neeo-sdk

NEEO Brain SDK
https://neeoinc.github.io/neeo-sdk/
MIT License
49 stars 17 forks source link

feat(user-settings): Allow Custom Settings #80

Closed Shepless closed 6 years ago

Shepless commented 6 years ago

@neophob @bdauria here you go guys. Let me know what you think. I've npm link'd this locally on one of my drivers and its all working as expected. I've obviously included UTs too. An example usage of this (in a driver) would be:

device.js

const path = require('path');
const {initSettings, buildDevice} = require('neeo-sdk');
const controller = require('./controller');

initSettings('driver-settings', {
  schemaLocation: path.resolve(__dirname, 'settings-schema.json'),
  settingsFileLocation: path.resolve(process.cwd(), 'super-driver-settings.json')
});

module.exports = buildDevice('My Driver')
  .setManufacturer('NEEO')
  .enableDiscovery(
    DISCOVERY_INCTRUCTIONS,
    controller.findTheDevices
  )
  .setType('MEDIAPLAYER')
  .addButtonGroup('Power')
  .addButtonHander((data) => console.log(data));

controller.js

const {getSettings} = require('neeo-sdk');
const SomeApi = require('some-fake-api-lib');

const settings = getSettings('driver-settings'); // <---- loaded from the user provided super-driver-settings.json file
const someApi = new SomeApi({
  ipAddress: settings.ipAddress,
  username: settings.credentials.username,
  password: settings.credentials.password
});

module.exports = {
  findTheDevices() {
    return someApi.find();
  }
};

settings-schema.json

{
  "type": "object",
  "properties": {
    "ipAddress": {
      "description": "The address of your thing",
      "type": "string"
    },
    "credentials": {
      "type": "object",
      "properties": {
        "username": {
          "description": "Please provide your username",
          "type": "string"
        },
        "password": {
          "description": "Please provide your password",
          "type": "string"
        }
      },
      "required": [
        "username",
        "password"
      ]
    }
  }
}

super-driver-settings.json

This is the only thing the end user of the driver needs to do - create this file, with a valid schema, in the correct location (defined by the developer using the option settingsFileLocation). If it isn't in the right location, we provide a helpful error message. Also, it if doesn't match the required schema, we tell them why validation failed with another friendly error message.

{
  "ipAddress": "192.168.12.1",
  "credentials": {
    "username": "Mr Foo",
    "password": "SuperSecret"
  }
}
neophob commented 6 years ago

Thanks @Shepless for the writeup.

While this seems to simplify integration first, I'm not sure if we're heading to the right direction. Midterm plan is to run those drivers on the Brain - so there you won't have an option to edit those settings. The same if a non experienced user use your Driver Manager - that user will not be aware that he needs to update package.json file.

So all those runtime should be filled out as part of the setup on the Brain (then the Brain will send this data to your driver). Question is, what's missing on the Brain side?

pfiaux commented 6 years ago

I like the idea of a simple setup for shared settings. Indeed having hard coded settings in the code is not good both for reuse and for maintainability.

The password example highlights a less than ideal scenario, an evil driver could easily "steal" other driver's accounts.

Also based #82 I think this would be a better fit for a stand alone module or for the neeo-toolkit repository than the neeo-sdk core.

Shepless commented 6 years ago

@neophob no problem. It was fun to implement it 😄

Originally I had this working from package.json files but for the reasons you outlined above, the change actually just works from split out .json files. In this case - schema.json and settings.json.

If the mid-term goal is to have drivers/devices running on the brain itself I would envisage this working in a similar fashion to the PR implementation:

I'm not very familiar with the internal architecture of the NEEO Brain so I'm not sure what kind of storage mechanism is currently in use. If I had to guess it would be a SQLite DB or something similar to keep things as lightweight as possible? If something like that exists then the settings provided by the user would be saved against that driver. I would assume then that the SDK would need some kind of callback, e.g.

const {buildDevice, initSettings} = require('neeo-sdk');
const controller = require('./controller');

initSettings('driver-settings', {
  schemaLocation: path.resolve(__dirname, 'settings-schema.json')
  // NOTE THE OMISSION OF THE SETTINGS.JSON PATH (IT WOULDN'T BE NEEDED, AS THE BRAIN WILL SEND THEM)
});

const device = buildDevice('My Device')
   .registerDeviceSettingsHandler(controller.registerSettings);

For me, this is all theory as I'm not familiar with the Brain's inner-workings so please let me know if I'm not on the right lines lol

pfiaux commented 6 years ago

User account/security code is built in for the next release, and for other settings we decided to not include them schema based in the current architecture of the SDK, we think it fits better outside of the SDK.

Note: for simply storing data within one package (no shared/exposed config) https://www.npmjs.com/package/configstore is a good option.