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.

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

RFC: Improve device configuration format #1675

Open
AlCalzone opened this issue Feb 9, 2021 · 5 comments
Open

RFC: Improve device configuration format #1675

AlCalzone opened this issue Feb 9, 2021 · 5 comments
Labels
config ⚙ Configuration issues or updates

Comments

@AlCalzone
Copy link
Member

AlCalzone commented Feb 9, 2021

@LordMike I hope you don't mind that I hijack your suggestions, but there are a couple of configuration file weirdnesses that I want to discuss.

More concise definition of partial parameters (#1436, proposed by @LordMike)

The current format looks like this:

"9[0x01]": {
	"label": "Alarm reaction to Access Control trigger state",
	"description": "React to access control triggers from other Z-Wave devices.",
	"valueSize": 2,
	"minValue": 0,
	"maxValue": 1,
	"defaultValue": 0,
	"readOnly": false,
	"writeOnly": false,
	"allowManualEntry": true
},
"9[0x0100]": {
	"label": "Alarm reaction to Smoke Alarms",
	"description": "React to Smoke Alarms from other Z-Wave devices.",
	"valueSize": 2,
	"minValue": 0,
	"maxValue": 1,
	"defaultValue": 0,
	"readOnly": false,
	"writeOnly": false,
	"allowManualEntry": true
},
...

It's clunky, as we must repeat a number of values between settings - and come up with multiple labels and so on. I propose the following instead (edited by @AlCalzone):

"9": {
  "label": "Alarm reaction config",
  "description": "React to the following alarms from other Z-Wave devices.",
  // These stay out here and are shared by partial params:
  "valueSize": 2,
  "readOnly": false,
  "writeOnly": false,
  "allowManualEntry": true,
  "partials": {
    "0x01": {
      // Label is concatenated with the shared label of the param itself, e.g. "Alarm Reaction Config: Access Control Trigger State"
      "label": "CO2 Alarm",
      "minValue": 0,
      "maxValue": 1,
      "defaultValue": 0
    },
    "0x200": {
      "label": "CO Alarm",
      // Description overwrites the shared description
      "description": "Lalala",
      // if a bitmask only encompasses a single bit, min/max 0..1 could be implied
      "defaultValue": 0
    }
  }
}

Move manufacturer labels into devices array

We often have a situation where the same device is available under different brands. This leads to duplicated config files that might go out of sync because contributors only edit the one they know about. Or multiple brands share a manufacturer ID, because the same manufacturer made the device. I suggest we make it possible to move the manufacturer name (and potentially device label) into the devices array:

before:

// file 1:
{
	"manufacturer": "Brand A",
	"manufacturerId": "0x001f",
	"label": "Label A",
	"description": "Some Dimmer",
	"devices": [
		{
			"productType": "0x1234",
			"productId": "0x2345"
		}
	],
...
}

// file 2:
{
	"manufacturer": "Brand B",
	"manufacturerId": "0x001f",
	"label": "Label B",
	"description": "Some Dimmer",
	"devices": [
		{
			"productType": "0x0001",
			"productId": "0x0001"
		}
	],
... same as file 1
}

after

// single file:
{
	"label": "Label A",
	"description": "Some Dimmer",
	"devices": [
		{
			"manufacturer": "Brand A",
			"manufacturerId": "0x001a",
			"productType": "0x1234",
			"productId": "0x2345"
		},
		{
			"manufacturer": "Brand B",
			"manufacturerId": "0x001b",
			"productType": "0x0001",
			"productId": "0x0001"
		}
	],
...
}

In order to avoid repetition, having the manufacturer and label at the root level should still be allowed. The one in the devices array would have priority.

Conditional configuration parameters

==> Implemented in #1810

Templates/imports

=> Implemented in: #1687

@blhoward2
Copy link
Collaborator

blhoward2 commented Feb 9, 2021

The manufacturer one wouldn't work as described because the manufactuerid is still not per device. There are no cases to my knowledge where the same devices within a manufacturer have duplicate files.

That's what usually changes and not just the text label. (For example, Honeywell and GE devices are both made by Jasco.) If we moved it into the device tree then we'd need to figure out in which folder the file should live.

@LordMike
Copy link
Contributor

LordMike commented Feb 9, 2021

Reg. the same-device-different-manufacturer thing. How about a json reference?

Example:

{
        // Either a custom identifier of sorts, that you can parse into a path
        "$ref": "0x0103 ses_fs-zw",
        // .. or a unique resource identifier (URI) .. which may or may not be an actual available url.. XML schemas makes great use of URI's that don't actually work (namespaces).
        "$ref": "https://raw.githubusercontent.com/zwave-js/node-zwave-js/master/packages/config/config/devices/0x0103/ses_fs-zw.json",
	"label": "My Label",
	"description": "Plug Actuator",
	"devices": [
		{
			"productType": "0x0001",
			"productId": "0x0002"
		}
	]
}

Docs:

My example uses a url which isn't always what we want (not one that's used as an internet url anyways).. non-internet devices and whatnot.. But the url doesn't necessarily (as i understand) have to be an actual thing .. it just has to exist.

For another project I use, Nuget, they use something similar. Their API is all online, so all the references are to actual existing resources, but the interesting part is really that each document they have, has an "@id" property which the unique reference to this document (in their API, this is the actual url it can be found at).

Any other document can then reference that document, by linking to the "@id". Examples:

My thinking is that, much like the original "$ref", that some unique identifier be made for these configs. In places that need it, you could either support:

  • Linking to a whole document, like my example, and then overwrite specified values - in my example, the label becomes "My Label" and not whatever was referenced. This supports the reference only at the root level
  • Support a pre-processing of a document, like above, but at any level. So much like nuget's, you could allow linking just the associations, or just the parameters.. maybe even just one parameter..

Sidenote: In all cases, I think the actual model of the configs should be the same. Ie., no matter what links or stuff can be done in the files, whatever the users of zwave-js get, is the same as it is today.

@blhoward2
Copy link
Collaborator

That sounds overly complicated and unnecessary. The ID isn't the problem. It will undo a lot of work on the database website. It's not just changing it in a file, it changes the entire structure of how we store files as right now the manufacturerid comes from the folder name. If the files are no longer separated that way then you have to figure out where the file lives at has both GR, Jasco, and Honeywell devices in.

@marcus-j-davies
Copy link
Member

marcus-j-davies commented Feb 9, 2021

my 2c,

Once upon a time, browser was reliant on manufacturers.json, as everything was traversed from that seed.
Where-ever this ends up, as long as the index format doesn't change, and it still has the properties, with their designated keys/location within the index (including the manufacture id/brand name, file location), it shouldn't cause any issues.

Having just migrated browser to the index, id hate for that to be a lost cause.

The only piece of work, that will come of it (looking at the current proposal), is digging deeper into the partials, when parsing the cfg file, to display its info.

Edit:
The version of browser that uses the index will drop after 6.1.4

@AlCalzone
Copy link
Member Author

Updated the proposals after some discussion.

The only piece of work, that will come of it (looking at the current proposal), is digging deeper into the partials, when parsing the cfg file, to display its info.

You don't have to do that - the API does that for you. You might only have to be able to display the additional "parent" param.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config ⚙ Configuration issues or updates
Projects
None yet
Development

No branches or pull requests

4 participants