-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add "adjust_to_caps" query parameter #134
Conversation
6751521
to
ddbe193
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'm sorry, I'm writing this on phone so please forgive if this is a bit messed up)
I'm really very confused about the relationship between the adjust_to_caps
property and the adjust_to_caps
query parameter. I'm not at all clear whether the property is a capability of the API, and the query parameter can only be specified when the property is true. The property could be adjust_to_caps_supported
to make that clear if so?
I'm not clear whether the API is permitted to ignore the query parameter... sounds like it is?
Or should there be some words about rejecting a PUT
request with the query parameter with 4xx
or 5xx
in some cases?
I think "adjust_to_caps": true
mean that adjustment is supported? I.e. not that it will always be performed? If so, is "adjust_to_caps": false
therefore the same as omitting the property?
Even if the property is true
and the query parameter is true
the API is permitted to not adjust the Base EDID?
Does adjusting the Base EDID at PUT
mean that the GET
returns the adjusted one? Or only the Effective EDID?
There is no Base EDID at the initial state. If the Base EDID for an Input changes, then all Senders associated with this Input MUST update their versions (in registered mode this MUST update the registered resources). | ||
|
||
### Effective EDID | ||
|
||
Effective EDID is such combination of Base EDID and Active Constraints of all Senders associated with this Input that the baseband signal from the Input can be transmitted by the Senders without breaking Active Constraints. | ||
If Active Constraints of all Senders associated with this Input are empty and `adjust_to_caps` is equal to `false` or not supported, the Effective EDID is equal to the Base EDID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume "the Effective EDID is equal to the Base EDID" is a shorthand for the actual behaviour, given further down "If the Base EDID is not set, the Effective EDID is built on the basis of a default EDID defined for the Input by the manufacturer."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, these two clauses seems not duplicating each other if you meant it. The first three paragraphs of the section describe how the Effective EDID should be built using the Base EDID and the fourth one describes what "If the Base EDID is not set".
If true, the Input is allowed to adjust Base EDID to internal capabilities. | ||
If omitted for an Input that supports it, the previous value is preserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't 100% understand either sentence.
If true, the Input is allowed to adjust... but doesn't have to?
If omitted (or false?), the previous value is preserved... Which previous value? The previous adjust_to_caps
value, or the previous Base EDID? Wouldn't that be an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it should be "the Input adjusts Base EDID to internal capabilities".
Maybe it makes sense to move the functional description to the JSON Schema's property and say here that this query parameter is a setter for that property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the functional description from the RAML spec. 49a62d3
If omitted for an Input that supports it, the previous value is preserved. | ||
type: boolean | ||
required: false | ||
example: adjust_to_caps=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example should probably just be true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks logical to me too, but I used https://github.com/AMWA-TV/is-10/blob/v1.0.x/APIs/AuthorizationAPI.raml#L65 as a reference. Though I checked it again and that file doesn't seem consistent in terms of such examples. https://raml.org/developers/raml-100-tutorial#enter-query-parameters says it should be rather true
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. 49a62d3
What is the expected behaviour before this PR if a client |
@garethsb, thank you for good questions.
Yes, it is. I don't see options to reject a PUT with the query parameter. If an implementation doesn't support this, then it just ignores query parameters. If there are no requirements in other NMOS specs to respond with an error on unexpected query params in a request, it makes sense to preserve such behaviour.
It means that the Effective EDID is the last Base EDID set + internal caps. If there is no the Base EDID, then it means that the next Base EDID set without query params will be combined with the internal caps to form the Effective EDID.
Maybe it makes this useless so I'll change the language.
Only the Effective EDID to let the user compare what they PUT and what they have after applying the internal caps.
It returns 400 "when the Base EDID is rejected due to validation failure". There's no room for other incompatibility because the Output's EDID may be really bad even in terms of the EDID specification but it may be what the baseband source expects for, so we have to provide the source with it. |
Resolves #132