Add device_model_id and software_version_id properties #20
Conversation
Also capitalize "Manufacturer" in the manufacturer_id description for consistency.
|
@mayanigrosh @sammysmallman Have a look at this and check for correctness and consistency? |
| "type": "integer", | ||
| "minimum": 0, | ||
| "maximum": 65535 | ||
| }, | ||
| "device_model_id": { | ||
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer. Users may expect to see these values in the UI as hexadecimal.", |
There was a problem hiding this comment.
I think the second sentence should be removed, because nothing specifies that.
There was a problem hiding this comment.
The spec says this, and that’s why I used “may”:
The recommended method for representing the UID in text is in hexadecimal format with a colon separating the Manufacturer ID and the Device ID.
An example of such would be: mmmm:dddddddd, where mmmm is the Manufacturer ID in hexadecimal and dddddddd is the Device ID in hexadecimal.
Would you rather see “it’s possible” instead of “may”?
There was a problem hiding this comment.
Or… is “Device ID” different from “Device Model ID”?
There was a problem hiding this comment.
"Device Model ID" and "Software Version ID" both appear as fields under DEVICE_INFO.
The Data Description for Device Model ID says
This field identifies the Device Model ID of the Root Device or the Sub-Device. The Manufacturer shall not use the same ID to represent more than one unique model type.
There was a problem hiding this comment.
Yes we want device_model_id. We're identifying a type of device and the software it is running, rather than a particular instance of a device. The use case is that we have a parameter definition from Device 1. If Device 2 is the same model made by the same manufacturer and running the same software, the get response would be the same thus we don't need to do a get request again, we can just used the previously retrieved parameter definition.
I'd just omit the part about displaying it as hex.
There was a problem hiding this comment.
Lets use GitHub's features and be explicit:
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer. Users may expect to see these values in the UI as hexadecimal.", | |
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer.", |
I'd like to argue that the two new keys be required. |
Are you also saying that you’d like for |
|
I'm still not sure I agree with this change. See my comment here: #13 (comment) In short, the data isn't necessary because each message doesn't need to be self-contained. |
|
ssilverman
force-pushed
the
master
branch
2 times, most recently
from
3e6d0c1
to
ac42033
Compare
There was a problem hiding this comment.
More generally, the description fields this adds are just the title with the word the prepended. Whereas the comment has potentially useful info.
From a UI perspective unless the description is mandatory, wouldn't we be better leaving it blank and then people don't need to show needlessly repetitive info. Or moving the $comment into description and then people can show that. I can't remember, is one for the developer and one for the user?
| "type": "integer", | ||
| "minimum": 0, | ||
| "maximum": 65535 | ||
| }, | ||
| "device_model_id": { | ||
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer. Users may expect to see these values in the UI as hexadecimal.", |
There was a problem hiding this comment.
Lets use GitHub's features and be explicit:
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer. Users may expect to see these values in the UI as hexadecimal.", | |
| "$comment": "The Device Model ID is a 16-bit value determined by the manufacturer.", |
| "maximum": 65535 | ||
| }, | ||
| "software_version_id": { | ||
| "$comment": "The Software Version ID is a 32-bit value determined by the manufacturer.", |
There was a problem hiding this comment.
It's not just a random 32-bit value they picked:
| "$comment": "The Software Version ID is a 32-bit value determined by the manufacturer.", | |
| "$comment": "The Software Version ID is a 32-bit value for a particular software release determined by the manufacturer.", |
|
This something i also need for my usecase. |
Also capitalize "Manufacturer" in the manufacturer_id description
for consistency.
Fixes #13