Manufacturer ID zero about rdm-schema

[samkearney]

I.e. the ESTA manufacturer ID for a dynamic UID 0x8001 and normal UID 0x0001 is 0x0001 in both cases.

+1 for this interpretation.

[peternewman]

Sorry this is a bit long...

So I'll state categorically that neither OLA nor the Open Lighting RDM website/PID store ( http://rdm.openlighting.org/ ) will be able to use this schema if this change is kept. We'd have to use a modified version of the schema for our usage and if we're doing that we're not really sticking to the schema/standard which somewhat defeats the point of going to the effort of moving to it.

I suspect the same is likely to be true of the other people that currently consume the OLP RDM data for the same reasons.

Personally I also think this is the wrong choice for the ecosystem (as I've said on previous public reviews). If you can represent all standardised and most manufacturer specific PIDs via a common system, then you can just make your interface use this one system for all PIDs it needs to show. This then automatically makes manufacturer specific PIDs first class citizens, as long as they either return their PID data as JSON, or it's been side-loaded into the system, they'll just work seamlessly. Likewise when a new standardised PID comes along, you can implement it with new data without needing additional software changes. OLA has been doing this successfully for a decade already (with our existing PID format). I accept in reality that for some PIDs like say DMX personality/description, you might need a bit more smarts on top to get a nicer UI and tie the descriptions up with the slots, but given it's JSON that sort of thing could be added in future, or stored separately. Certainly for more simple PIDs like say POWER_ON_SELF_TEST or the INVERT PIDs though, why bother writing specific code when you can write something generic (or use an existing JSON Schema library) to make a dynamic UI automatically.

In my opinion unless you can use the ESTA PIDs together with manufacturer specific ones, and ideally if the ESTA ones are offered pre-built, either by ESTA themselves or OLP will do this if/when we switch, then you've incentivised people further by minimising their effort keying in information (as well as reducing the risk of mistakes). A bit like the header files people have made, you could ship/link to a JSON file with each released standard containing the PIDs it has.

Also pedantically if ESTA/PLASA don't have a manufacturer ID of zero, why are they listed on the official list of manufacturer IDs under ID zero?
https://tsp.esta.org/tsp/working_groups/CP/mfctrIDs.php

---

As above, I really don't think you should, but I'd also point out you're being very inconsistent, you've restricted the standard so you can't represent the ESTA manufacturer ID, but haven't restricted it to only the appropriate range of manufacturer specific PIDs:

rdm-schema/rdm-schema.json

Lines 17 to 23 in 322f3c5

	"pid": {
	"title": "PID",
	"description": "The parameter ID.",
	"type": "integer",
	"minimum": 0,
	"maximum": 65535
	},

Which goes against the standard:

PID # Requested:
The manufacturer specific PID requested by the controller. Range 0x8000 to 0xFFDF.

You should do both, or ideally neither.

---

Also in the commit message, you said:

The range of 8000h and above is used in ANSI E1.33 for dynamic UIDs."
For now, keeping values 8000h and above in the range.

322f3c5

I know I asked the question and got a response during some of the review cycles (although I can't see anything in the standards), but my understanding is the MSBit is just a flag that it's a dynamic UID, the ESTA manufacturer ID is still the same as with a static UID without that bit set, and hence the range of manufacturer ID-PID pairs is still the same, we haven't doubled it and I can't have a different PID against a dynamic UID than a static one. So from my understanding that means the range of manufacturer IDs should still finish at 7FFFh. Being particularly pedantic, if you're saying the manufacturer ID zero doesn't exist (which as above I disagree with), then the manufacturer ID 8000h also can't exist, as that's the dynamic UID for the non-existent manufacturer ID zero.

[peternewman]

I also meant to say, on a more positive front, I think there would be some ways to provide both bits of functionality with subschemas etc, so there could be a schema to validate all PID definitions, and one that just validates ones which are valid to be returned by a manufacturer (i.e. with a restricted list of manufacturer PIDs), without needing to duplicate the schema elements, then people could just use whichever one is right for them.

[ssilverman]

I’m with you on this one. I think the range should be the entire 0-0xffff, especially since it’s defined as a 16-bit value only. I should probably change it back and then fight for it. This came about in the discussion.

It’s not just me deciding this.

[ssilverman]

I don’t think of those dynamic IDs as just a bit being set. I think of them as being >= 0x8000. That’s why I included them.

[ssilverman]

I also meant to say, on a more positive front, I think there would be some ways to provide both bits of functionality with subschemas etc, so there could be a schema to validate all PID definitions, and one that just validates ones which are valid to be returned by a manufacturer (i.e. with a restricted list of manufacturer PIDs), without needing to duplicate the schema elements, then people could just use whichever one is right for them.

Could you give some examples of what you mean?

[peternewman]

I’m with you on this one. I think the range should be the entire 0-0xffff, especially since it’s defined as a 16-bit value only. I should probably change it back and then fight for it. This came about in the discussion.

I guess this matches the scope in your readme:

This project intends to provide a machine-readable way to describe manufacturer-specific RDM messages.

I can't remember if that matches the scope on the most recent public review that came around. Although as I've hopefully indicated above, I personally think it's an unnecessarily narrow scope and by making it only slightly wider it could be far more useful.

It’s not just me deciding this.

Yeah I assumed it was coming out of the task group or similar.

[peternewman]

I don’t think of those dynamic IDs as just a bit being set. I think of them as being >= 0x8000. That’s why I included them.

That's not what's defined in E1.33 section 3.3.1 Static and Dynamic UIDs.
Dynamic Flag (1-bit)
ESTA Manufacturer ID (15-bit)
Device ID (32-bit)

I.e. the ESTA manufacturer ID for a dynamic UID 0x8001 and normal UID 0x0001 is 0x0001 in both cases.

Likewise when I asked a similar question at the PLASA Plugfest, (from my notes) people agreed with my interpretation, "Is a manufacturer specific PID from 8001 valid against 0001? - Just a flag, doesn’t exist".

[peternewman]

I think there would be some ways to provide both bits of functionality with subschemas etc

Could you give some examples of what you mean?

So I believe from https://json-schema.org/understanding-json-schema/structuring.html#id13 you could refer to one of multiple schemas, either in one or multiple files.

I think you could reference all the existing get|set_request|response[_subdevice_range], PID and version fields (i.e. without redefining all of those parameters) and just have different manufacturer fields.

[ssilverman]

Ok, per the comments from both of you, I stand corrected on the meaning of “dynamic”. (Thanks, @samkearney for piping in!)

@peternewman the reason we just added the manufacturer ID is because of your public review comment requesting it. Here’s what we heard:

1. Add a manufacturer ID field, and
2. Make it required.

The reasoning we heard in the comment was because of offline requirements and to reduce the traffic so there doesn’t need to be extra requests to obtain the manufacturer ID. (Note that this is from memory and the second thing may be just from the discussion and not from your comment; I don’t have a copy of them yet.)

Personally, I believe that if some data can point to where it’s from, that’s a reasonable thing to have, which is why I agree with adding the ID.

Did we get this right? (Both things above.)

Also, “valid manufacturer IDs” are indeed the full range of 0-65535 (per my reading of the spec). Do you advocate for changing the valid range to 0-65535 or keeping “1-32767, 32769-65535”? Or even changing to just 1-32767? Or 0-32767?

Per your second comment on restructuring, would you mind creating another issue? I think it sounds separate, if I’m understanding correctly.

[peternewman]

@peternewman the reason we just added the manufacturer ID is because of your public review comment requesting it. Here’s what we heard:

1. Add a manufacturer ID field, and

2. Make it required.

The reasoning we heard in the comment was because of offline requirements and to reduce the traffic so there doesn’t need to be extra requests to obtain the manufacturer ID. (Note that this is from memory and the second thing may be just from the discussion and not from your comment; I don’t have a copy of them yet.)

Personally, I believe that if some data can point to where it’s from, that’s a reasonable thing to have, which is why I agree with adding the ID.

Did we get this right? (Both things above.)

Yes, thanks. Consider if you fetched the following data from a fixture:

{
  "name": "FOO",
  "pid": 32769,
  "version": 1,
  "get_request_subdevice_range": [ "root", "subdevices" ],
  "get_request": [],
  "get_response": [
    { "name": "bar", "type": "uint32" }
  ],
  "set_request_subdevice_range": [ "root", "subdevices", "broadcast" ],
  "set_request": "get_response",
  "set_response": []
}

As it was (without manufacturer ID), I can't just take the chunk of JSON, either from the fixture or alternatively (if the standard allows it), downloaded from someone's website (either for a fixture which doesn't have the space to store/serve up the JSON or to give backwards compatibility to a fixture that's not having it's code worked on). Instead I'm forced to store/parse some additional metadata outside of that JSON to track the manufacturer ID, in the download case I either have to read it from the filename or put it in a special folder, neither of which are great.

It just needs to be required to ensure all manufacturers do it. Then as my first stage of validation I can throw an error if I fetched my JSON via RDM from a manufacturer with UID 0002:.... and it contains manufacturer 0x0001 in the JSON.

Also, “valid manufacturer IDs” are indeed the full range of 0-65535 (per my reading of the spec). Do you advocate for changing the valid range to 0-65535 or keeping “1-32767, 32769-65535”? Or even changing to just 1-32767? Or 0-32767?

The E1.20 spec says:

For use in this standard the value of the 16-bit Manufacturer ID shall be restricted to 0x0001 through 0x7FFF.

I don't see any mention of 65535 in it and it seems like we're in general agreement that 0x8000 up are just the dynamic flag of the normal ones (in E1.33 at least).

I advocate to change it to 0-32767, so zero can be used as the obvious choice (semi-confirmed on the TSP website, I've not gone digging in the standard itself) to (internally) represent an ESTA PID via the same schema. See e.g. http://rdm.openlighting.org/pid/manufacturer?manufacturer=0 for an existing example usage.

Per your second comment on restructuring, would you mind creating another issue? I think it sounds separate, if I’m understanding correctly.

I can do. I was only proposing it as a workaround if people were reluctant to allow the ESTA manufacturer ID zero to be in the main standard, without making the standard unworkable for other related usages.

Although sort of linked to this, I don't believe the schema currently allows for bundles of more than one PID in the same file, which would be nice for offline download and other indirect usage. Although it then means you're duplicating the manufacturer ID for each PID, unless you go for a more involved structure.

[
  {
    "pid": a,
    "manufacturer": 1
  },
  {
    "pid": b,
    "manufacturer": 1
  }
]

Obviously something like this is more versatile and allows one JSON file to have one (or more) manufacturers PIDs (consider badge engineering), but at the extent of a bit more complexity and overhead for a single PID via RDM if the same schema is used for both.

[
  {
    "manufacturer": 1,
    "pids": [
      {
        "pid": a,
      },
      {
        "pid": b,
      },
    ]
  }
]

Further to this, I've realised the latest draft I can look at accidentally implies one of the array options for 5.5 Get Metadata JSON URL (METADATA_JSON_URL) as unlike 5.4 there is no parameter ID field present! I assume that's just an oversight rather than intentional...

[ssilverman]

@peternewman I just pushed some changes:

1. Changed example manufacturer IDs to 32767 (0x7FFF).
2. Changed the upper manufacturer ID limit to 32767.
3. Added some discussion in the README, including adding one open question to the "Open questions" section on this subject.

[ssilverman]

@peternewman for the section 5.5 missing PID field, did you report an issue to the group?