New schema discussed by members of the E1.37-5 task group. #8

mayanigrosh · 2019-08-23T15:40:47Z

Modified spacing for ease in readability
Added schema "title" (which may need to have the word "informative" removed)
Added description values
Changed single-value enums to "const"
Separated "bits" from "bitField"
Created a simpleType enum for all basic types
Alphabetized definitions
Changed "label" to "labeledInteger"
- changed the "label" property to "name", for consistency
Changed "max_size" and "min_size" to "maxLength" and "minLength", for
consistency with JSON schema naming conventions.

Added '"uniqueItems": true' to the "bits" field of the "bitField" field type, the integer field type, the command type, and the range type
Created the subdeviceType with an enumeration of the possible values.
Added a "pdEnvelope" type to help deal with the proposed PACKED_PIDS parameter message
Added a new "list" type that represents a list of any valid non-list
field type.
Removed the requirement of having "name" for some of the types so they can
also be used in lists.
Suffixed all the types with "Type" so that naming is consistent. These aren't
the words that will be used when writing a schema and they don't have to be
the same as the constant names, eg. "uint8" or "string" or "list".
Added a "compoundType" type so that we don't have to make a special type for
all possibilities. For example, the list of 3-field items returned
in SLOT_INFO. These compound types are only supported in the list type,
"listType", because a field is only used in "commandType".
Factored all the types into a "oneOf" so that it can be referenced from
"compoundType", "listType", and "fieldType".
Changed "boundedIntegerType" to "integerType" and "boundedStringType"
to "stringType".
Removed "name" from the "required" list of "labeledIntegerType"
and "listType".
Changed "upper" and "lower" in "rangeType" to "maximum" and "minimum",
respectively, for consistency with JSON schema naming conventions.
Added an optional "length" property to "pdEnvelope".
Removed "fieldType" and instead made "commandType" be a collection
of "oneOfType".

- Modified spacing for ease in readability - Added schema "title" (which may need to have the word "informative" removed) - Added description values - Changed single-value enums to "const" - Separated "bits" from "bitField" - Created a simpleType enum for all basic types - Alphabetized definitions - Changed "label" to "labeledInteger" * changed the "label" property to "name", for consistency - Changed "max_size" and "min_size" to "maxLength" and "minLength", for consistency with JSON schema naming conventions. * Added '"uniqueItems": true' to the "bits" field of the "bitField" field type, the integer field type, the command type, and the range type * Created the subdeviceType with an enumeration of the possible values. * Added a "pdEnvelope" type to help deal with the proposed PACKED_PIDS parameter message * Added a new "list" type that represents a list of any valid non-list field type. * Removed the requirement of having "name" for some of the types so they can also be used in lists. * Suffixed all the types with "Type" so that naming is consistent. These aren't the words that will be used when writing a schema and they don't have to be the same as the constant names, eg. "uint8" or "string" or "list". * Added a "compoundType" type so that we don't have to make a special type for all possibilities. For example, the list of 3-field items returned in SLOT_INFO. These compound types are only supported in the list type, "listType", because a field is only used in "commandType". * Factored all the types into a "oneOf" so that it can be referenced from "compoundType", "listType", and "fieldType". * Changed "boundedIntegerType" to "integerType" and "boundedStringType" to "stringType". * Removed "name" from the "required" list of "labeledIntegerType" and "listType". * Changed "upper" and "lower" in "rangeType" to "maximum" and "minimum", respectively, for consistency with JSON schema naming conventions. * Added an optional "length" property to "pdEnvelope". * Removed "fieldType" and instead made "commandType" be a collection of "oneOfType".

…iled pull request. Added the MAC type, which had been forgotten until going over the public review comments.

mayanigrosh · 2019-08-23T17:31:09Z

Fixing this and updating with code that will pass CI and has a type we omitted in the earlier draft.

mayanigrosh · 2019-08-23T17:33:37Z

Have added updated schema with better line breaks and the "MAC" type.

peternewman

Thanks @mayanigrosh , some comments follow.

So the actual schema checker will fail currently as it's a draft-v4 one, I'm going to try and switch to https://ajv.js.org/ instead via https://www.npmjs.com/package/grunt-jsonschema-ajv to check draft-v7. Do you want to upload the new examples into the examples folder too?

I'd also suggest running it against https://www.json-schema-linter.com/ which picks up a lot of warnings and errors we ought to fix. It looks like it doesn't like refing something before it's been defined, which probably means dropping some of the alphabetical ordering.

I've started working on improving the CI here: #9

peternewman · 2019-08-24T13:17:17Z

schema.json

+      "description": "A bit field, a collection of 'bit' items.",
+      "properties": {
+        "name": { "$ref": "#/definitions/nameType" },
+        "type": { "const": "bitField" },


I don't think JSON Schema Linter likes us overriding type here, I think I agree, I'm not sure if we need this type for our use (to differentiate actual objects), but if we do we should give it a different name. We may also need to tell it that the type of our const is "string" in JS land.

"type" seems to validate okay in the online v7 validators?

peternewman · 2019-08-24T13:21:39Z

schema.json

-          },
-          "required": ["name", "type"]
+        "prefix": {
+          "description": "The unit prefix, defined in Table A-14 of E1.20.",


I'm not quite sure why @nomis52 went down this route with his original proposal, OLA stores the prefix as the actual exponent which to me seems a lot more sensible, given you have to do lots of mapping to turn other values here into their equivalent tables in E1.20, this one doesn't seem too bad, we could either just have it as a signed integer or restrict it to only the prefixes listed in A-14 if we wanted to be a tighter match, see:
https://github.com/OpenLightingProject/ola/blob/master/common/rdm/Pids.proto#L66

peternewman · 2019-08-24T13:22:40Z

schema.json

-          },
-          "required": ["name", "fields", "type"]
+        "unit": {
+          "description": "The unit type, defined in Table A-13 of E1.20.",


I think sadly units has to stay numeric to cover the manufacturer specific ones.

peternewman · 2019-08-24T13:23:27Z

schema.json

+        "name": { "$ref": "#/definitions/nameType" },
+        "value": { "type": "integer" }
+      },
+      "required": [ "value" ]


Name should be required too, this dropped out from our last discussion. Otherwise it's just a range with a single value.

peternewman · 2019-08-24T13:25:28Z

schema.json

+        { "$ref": "#/definitions/bitFieldType" },
+        { "$ref": "#/definitions/compoundType" },
+        { "$ref": "#/definitions/integerType" },
+        { "$ref": "#/definitions/listType" },


I think we need to drop listType otherwise you can have a list of lists and not know how to parse it (unless you enforce a fixed length on it), but then just doing a compoundType of e.g. three uint8's is easier and covers that edge case.

The list type is needed for things such as a list of compound types, see DEFAULT_SLOT_VALUE, for example.

peternewman · 2019-08-24T13:35:36Z

schema.json

+      "properties": {
+        "name": { "$ref": "#/definitions/nameType" },
+        "type": {
+          "enum": [


Should we add DS_NOT_DEFINED from Table A-15 in E1.20, I'm not sure what it's for and I don't think I've seen it in the wild.

We also have no way to represent the Manufacturer-Specific Data Types in this schema, although I'm not sure what they'd be apart from the fabled uint128 and indeed how the console would use them.

I added "int128" and "uint128". Can't hurt. Good suggestion.

There's a good chance that any of the manufacturer data types could be represented using the existing types in the schema, especially since there's string, list, and compound types. "list-of-bytes" or strings could be used to represent pretty much all data.

Agreed, although then you're back to the rather limited interface some controllers provide where manufacturer specific PID support is "please enter this list of bytes". For example if you were downloading a gobo preview, you really want to know what you're receiving is a BMP or a PNG or whatever, not just get sent a load of bytes.

peternewman · 2019-08-24T13:38:51Z

schema.json

+            "hostname",
+            "ipv4",
+            "ipv6",
+            "mac",


This should definitely be in Table A-15 in E1.20 given it's in E1.37-2.

peternewman · 2019-08-24T13:39:53Z

schema.json

+          "minimum": 0
+        }
+      },
+      "required": [ "type" ]


If these things don't have a name we can't give them a label and display them sensibly in our dynamic UI.

Done. Good point.

peternewman · 2019-08-24T13:40:36Z

schema.json

-    "set_subdevice_range": ["set_request", "set_response"]
+    "get_request": [ "get_response" ],
+    "get_response": [ "get_request" ],
+    "get_subdevice_range" : [ "get_request", "get_response" ],   


Nit: can we trim the trailing whitespace please.

peternewman · 2019-08-24T13:41:57Z

schema.json

-  "$schema": "http://json-schema.org/draft-04/schema#",
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "Parameter Metadata Language Schema, Informative",
+  "description": "The (informative) schema for the Parameter Metadata Language


I'm not sure this is valid JSON based on the discussion here, we might need to tweak our linter: https://stackoverflow.com/questions/2392766/multiline-strings-in-json

Indeed this validator doesn't like the whitespace:
https://jsonlint.com/

Fixed in 132c5ad in #11 .

peternewman · 2019-09-19T18:14:11Z

@ssilverman , do you fancy opening a new Pull Request with your changes in, then we can run them against the small battery of Continuous Integration I've started to put together?

ssilverman · 2019-09-19T19:32:35Z

@peternewman yes, I’d be happy to. Thanks for the suggestion. However, I will adhere to the 2019-09 spec and I’ve been hearing that the linter doesn’t work with long lines. Is this fixed?

peternewman · 2019-09-19T20:08:49Z

@ssilverman it's fixed in #11 but not merged. TBH for now you'll want to pull in all the changes/commits from there that don't touch JSON files (schema or examples).

ssilverman · 2019-09-19T21:49:28Z

@peternewman it sounds like you’re saying I should hold off on the JSON; that’s all I have that I’d be committing. It’ll be sent in an email off-list very soon.

peternewman · 2019-09-20T07:33:04Z

@peternewman it sounds like you’re saying I should hold off on the JSON; that’s all I have that I’d be committing. It’ll be sent in an email off-list very soon.

Sorry @ssilverman that wasn't too well worded. I meant pull in all my changes from that branch except the JSON based ones. I've made it simpler now, just grab everything in #12 and you'll be good. If you want to open a PR anyway and either grab those changes in #12, or just open a PR and I can do the magic to get them in.

mayanigroshsonos added 2 commits August 23, 2019 11:37

Fixed the line-wrapping issues that caused the failed CI build and fa…

1ec157b

…iled pull request. Added the MAC type, which had been forgotten until going over the public review comments.

mayanigrosh closed this Aug 23, 2019

mayanigrosh reopened this Aug 23, 2019

peternewman requested changes Aug 24, 2019

View reviewed changes

peternewman added 2 commits August 24, 2019 14:43

Merge branch 'master' into e137-5_json_schema

3f80bf3

Merge branch 'master' into e137-5_json_schema

a183633

peternewman mentioned this pull request Sep 7, 2019

E1.37-5 tweaks #11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New schema discussed by members of the E1.37-5 task group. #8

New schema discussed by members of the E1.37-5 task group. #8

mayanigrosh commented Aug 23, 2019

mayanigrosh commented Aug 23, 2019

mayanigrosh commented Aug 23, 2019

peternewman left a comment

peternewman Aug 24, 2019

ssilverman Sep 19, 2019 •

edited

Loading

peternewman Aug 24, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

ssilverman Sep 19, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

ssilverman Sep 19, 2019

ssilverman Sep 19, 2019

peternewman Sep 20, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

ssilverman Sep 19, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

peternewman Aug 24, 2019

peternewman Sep 18, 2019

peternewman commented Sep 19, 2019

ssilverman commented Sep 19, 2019

peternewman commented Sep 19, 2019

ssilverman commented Sep 19, 2019

peternewman commented Sep 20, 2019

New schema discussed by members of the E1.37-5 task group. #8

Are you sure you want to change the base?

New schema discussed by members of the E1.37-5 task group. #8

Conversation

mayanigrosh commented Aug 23, 2019

mayanigrosh commented Aug 23, 2019

mayanigrosh commented Aug 23, 2019

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssilverman Sep 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Sep 19, 2019

ssilverman commented Sep 19, 2019

peternewman commented Sep 19, 2019

ssilverman commented Sep 19, 2019

peternewman commented Sep 20, 2019

ssilverman Sep 19, 2019 •

edited

Loading