hoobs-org / hoobsd

Server component for the certified HOOBS smart home stack.
GNU General Public License v3.0
8 stars 4 forks source link

HOOBS v4 - Allowing Plugins to Write to the Bridge config.json #87

Closed mkormendy closed 3 years ago

mkormendy commented 3 years ago

With the release of HOOBS v4, the architecture has changed when it comes to the capability of plugins writing to the file system. This has been verified

The following block of code in my plugin gathers the platform section and adds specific objects and properties to it that are dynamically found by way of SSDP discovery and used for associating accessories with multiple and different hardware endpoints.

updateHomebridgeConfig.txt

Unfortunately HOOBS v4 doesn't seem to allow me to write to the bridge config file anymore, which is a convenient place to have this association, as the configuration UI then picks up where the discovery of the hardware endpoints left off and allows the user to assign accessories to those hardware endpoints nicely.

One of the roadblocks with trying to allow the user to write their own platform json for the config endpoints is that parts of the identifiers that maintain the provision of the hardware endpoints is not exposed to them in any sort of convenient fashion. The discovery process uses SSDP protocols and API responses with unique identifiers crafted from the device UUID addresses and this is overly complicated for the user to do – especially since this is abstracted and automated by my plugin for them in the first place.

Can we have some sort of ability to write to the config file again?

mkellsy commented 3 years ago

I did a quick review of your config code. I tightened up the code a bit and updated the features it uses. Also note that assigning an object variable to another doesn't clone it. You have to use a spread, assign or JSON methods. https://www.samanthaming.com/tidbits/70-3-ways-to-clone-objects/

Here's the same code updated.

/**
 * Update Homebridge config.json with discovered panel information.
 *
 * @param uuid string UUID for the panel as reported in the USN on discovery.
 * @param panel PanelObjectInterface  The status response object of the plugin from discovery.
 */
updateHombridgeConfig(uuid: string, panel: PanelObjectInterface) {
  // sanitize panel name
  const name = (panel.model && panel.model !== '' ? panel.model : 'Konnected V1-V2').replace(/[^A-Za-z0-9\s/'":\-#.]/gi, '');

  // load config file
  const config = JSON.parse(fs.readFileSync(this.api.user.configPath()).toString());

  // clone config for alterations
  const modified = { ...config };

  // get index of my platform
  const platform = modified.platforms.findIndex((item: { [key: string]: any }) => item.platform === 'konnected');

  // only do this if the platform exists in the config
  if (platform >= 0) {
    modified.platforms[platform].panels = modified.platforms[platform].panels || [];

    // find existing definition of the panel
    const position = modified.platforms[platform].panels.findIndex((item: { [key: string]: any }) => item.uuid === uuid);

    // update if exists or push to panels array
    if (position >= 0) {
      modified.platforms[platform].panels[position] = { name, uuid, ipAddress: panel.ip, port: panel.port };
    } else {
      modified.platforms[platform].panels.push({ name, uuid, ipAddress: panel.ip, port: panel.port });
    }
  }

  // only write new config if there are changes
  if (JSON.stringify(modified) !== JSON.stringify(config)) {
    let backups = `${this.api.user.storagePath()}/backups/config-backups`;

    // if the backups folder doesn't exist use the main storage path
    if (!fs.existsSync(backups)) backups = this.api.user.storagePath();

    fs.writeFileSync(`${backups}/config.json.${new Date().getTime()}`, JSON.stringify(config, null, 4));
    fs.writeFileSync(this.api.user.configPath(), JSON.stringify(modified, null, 4));
  }
}

And I found a No-No. The plugin should never exit the process. There are other plugins that will need to be cleaned up. It's better to just throw an exception and let Homebridge/HOOBS handle it.

// restart/crash cleanup
const cleanup = () => {
  server.close();

  this.log.info(`Listening port ${this.listenerPort} closed and released`);
};

process.on('SIGINT', cleanup).on('SIGTERM', cleanup);

I'm still looking into why it isn't saving on my end.

mkellsy commented 3 years ago

As for the code that saves the config to the encrypted version, Here's that code block.

this.watcher = Watcher.watch(join(Paths.data(State.id), "config.json")).on("change", (path: string) => {
    const config = Paths.loadJson<any>(path, {});
    const existing = { accessories: this.config.accessories || [], platforms: this.config.platforms || [] };
    const modified = _.extend(existing, { accessories: config.accessories || [], platforms: config.platforms || [] });

    Config.filterConfig(modified.accessories);
    Config.filterConfig(modified.platforms);

    if (!jsonEquals(existing, modified)) Config.saveConfig(modified, State.id);
});

As for saving to the bridge config. This is tough. See HOOBS 4 sandboxes Homebridge, so it needs to store this information somewhere else. It is stored in /var/lib/hoobs/bridges.conf. Basically this file keeps track of the bridges and their settings. There are additional settings, like type, id ... It is best to keep any plugin settings inside the platform for your plugin for a few reasons.

  1. In HOOBS, the bridge settings are seperate.
  2. In Homebridge the keys and values outside accessories and platforms are governed by a TypeScript Interface and may throw type errors.

Lets look at the Homebridge config interfaces.

Here's the options interface, this is what is allowed inside the bridge config

export interface BridgeConfiguration {
  name: string;
  username: MacAddress;
  pin: string;
  advertiser: MDNSAdvertiser;
  port?: number;
  bind?: (InterfaceName | IPAddress) | (InterfaceName | IPAddress)[];
  setupID?: string[4];
  manufacturer?: string;
  model?: string;
  disableIpc?: boolean;
}

Here's the interface for platforms, notice that it inherits Record, which allows for pretty much anything.

export interface PlatformConfig extends Record<string, any> {
  platform: PlatformName | PlatformIdentifier;
  name?: string;
}

And skipping the accessory interface, since it is pretty much the same as platforms, here is the complete config interface.

export interface HomebridgeConfig {
  bridge: BridgeConfiguration;
  mdns?: any;
  accessories: AccessoryConfig[];
  platforms: PlatformConfig[];
  plugins?: PluginIdentifier[];
  disabledPlugins?: PluginIdentifier[];
  ports?: ExternalPortsConfiguration;
}

As you can see, there is no room for writing to the bridge section. It's not a good idea.

mkormendy commented 3 years ago

Is there a way that I can target HOOBS differently than Homebridge to just provide something that will add that to the config for the sadnboxed bridge?

mkormendy commented 3 years ago

As for saving to the bridge config. This is tough. See HOOBS 4 sandboxes Homebridge, so it needs to store this information somewhere else. It is stored in /var/lib/hoobs/bridges.conf. Basically this file keeps track of the bridges and their settings. There are additional settings, like type, id ... It is best to keep any plugin settings inside the platform for your plugin for a few reasons.

Having said that, and maybe I'm misunderstanding what you mean (which is a common fault of mine 🤣), but the intent of me writing to the config.json file isn't really to write to the bridge section of the config, but specifically to my platform block.

Ultimately, I appreciate and like the changes you were talking about in my code block I shared! I want to use that, but maybe have two sections, one for HOOBS and one for Homebridge. I don't mind writing something that satisfies both systems separately, but still have the convenience of one form.schema.json interface experience just the same.

mkellsy commented 3 years ago

I don't treat Homebridge differently in HOOBS. The only things I change are this.

  1. I define the storage path
  2. I define the config path
  3. I load plugins from a different directory (no global modules)
  4. Don't support Homebridge child bridges as our sandboxing solves this

Other then these 4 things HOOBS and Homebridge are the same. Allowing updates to the config.json file should be supported because Homebridge allows it. The plugins should not have to change. If I don't do this, plugin devs start to tell our users to ditch HOOBS and install Homebridge.

mkormendy commented 3 years ago

Okey doke! Well I'm keen to do some productive troubleshooting on Discord, and circle back here with the results and close the ticket when QA'd and pushed.

mkellsy commented 3 years ago

I have the latest pushed to our edge branch. I am testing with my production environment now. I found that saving and restarting was causing issues. The expected behavior is save and not restart. So I need to support this.

Ring does write to the config it updates the refresh token. I happen to use this plugin in my own setup. It seems to be working great, but I just want to be sure.

To set your 4.0 version to the edge branch run this command.

sudo hbs system edge

This will change the repo to https://dl.hoobs.org/deb/edge/

mkormendy commented 3 years ago

I did a quick review of your config code. I tightened up the code a bit and updated the features it uses. Also note that assigning an object variable to another doesn't clone it. You have to use a spread, assign or JSON methods. https://www.samanthaming.com/tidbits/70-3-ways-to-clone-objects/

I changed the {...original} spread to a JSON.parse{JSON.stringify(original)) since we're doing a deep copy here.

mkormendy commented 3 years ago

Okay so I am seeing that v4.0.91 now writes to the config properly. Thank you for your help with this issue!